Optimize OOP compilation of HasSourceGenerators. #72978
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
Going to to a test insertion to see if this has any perf issues. |
| var result = await client.TryInvokeAsync<IRemoteSourceGenerationService, bool>( | ||
| solution, | ||
| projectId, | ||
| (service, solution, cancellationToken) => service.HasGeneratorsAsync(solution, projectId, cancellationToken), | ||
| (service, solution, cancellationToken) => service.HasGeneratorsAsync(solution, projectId, analyzerReferences, projectState.Language, cancellationToken), |
There was a problem hiding this comment.
new approach passes along the analyzer-reference checksums and language, as we can answer on the oop side with just that info.
| private static readonly Dictionary<string, AnalyzerReferenceMap> s_languageToAnalyzerReferenceMap = new() | ||
| { | ||
| { LanguageNames.CSharp, new() }, | ||
| { LanguageNames.VisualBasic, new() }, |
There was a problem hiding this comment.
it's fine to have these be keyed just by VB/C#. OOP is only supported for those two languages alone. This makes it easy to initialize and look into this without locking, or complex types like ConcurrentDict.
| { | ||
| return RunServiceAsync(solutionChecksum, async solution => | ||
| { LanguageNames.CSharp, (new(), static analyzerReference => HasSourceGenerators(analyzerReference, LanguageNames.CSharp)) }, | ||
| { LanguageNames.VisualBasic, (new(), static analyzerReference => HasSourceGenerators(analyzerReference, LanguageNames.VisualBasic)) }, |
There was a problem hiding this comment.
same here. we can hardcode in C#/vb here to simplify thigns.
There was a problem hiding this comment.
note: we cache thsi per analyzer reference so that multiple projects with the same analyzer references (common), and lang, will cache the same result.
| string language, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (analyzerReferenceChecksums.Length == 0) |
There was a problem hiding this comment.
technically not necessary. the host side already checked this. d
|
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/543307 shows no regressions. (There is one regression, but it was caused by #72965 not this PR). |
| return false; | ||
|
|
||
| var analyzerReferenceMap = s_languageToAnalyzerReferenceMap[projectState.Language]; | ||
| if (!analyzerReferenceMap.TryGetValue(projectState.AnalyzerReferences, out var lazy)) | ||
| { | ||
| // Extracted into local function to prevent allocations in the case where we find a value in the cache. |
There was a problem hiding this comment.
my guess is no. there are needed captures in these paths. but i will try.
There was a problem hiding this comment.
yeah, it's all sorts of captury. :) No go on static here. We need to capture enough data to be able to call to oop effectively to answer the question. note that once this happens once, and is cached, that captured data will be dropped. So we only need this alloc when looking at a fresh (non-identity) list of analyzer-references. Which should be very rare as the solution forks forwards.
src/Workspaces/Remote/ServiceHub/Services/SourceGeneration/RemoteSourceGenerationService.cs
Show resolved
Hide resolved
|
@jasonmalinowski For review when you get back. |
We had two problems with the prior approach.
The new approach is much more lightweight.
First, On the host side, we now cache the information along with the list-of-analyzer-references a project has. That list almost never changes when projects fork (unless hte user literally adds/removes references). So the information is reused virtually all the time on the host side.
Second, when we do make the call to oop, oop only needs to sync over the analyzer references, which it can do in in a much more lightweight fashion. Oop also caches the information on a per-analyzer-reference, so that different projects which reference the same analyzer references can call to oop and have the question answered the first time without a call back to the host at all.
--
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=9398165&view=results
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=9398506&view=results