Parallelize loading solution-level analyzers#82447
Parallelize loading solution-level analyzers#82447RikkiGibson wants to merge 3 commits intodotnet:mainfrom
Conversation
| // 'CreateSolutionLevelAnalyzerReferences' needs to be broken out into its own service for us to be able to move this. | ||
| var miscellaneousFilesWorkspace = new LanguageServerWorkspace(hostServicesProvider.HostServices, WorkspaceKind.MiscellaneousFiles); | ||
| miscellaneousFilesWorkspace.SetCurrentSolution(s => s.WithAnalyzerReferences(CreateSolutionLevelAnalyzerReferencesForWorkspace(miscellaneousFilesWorkspace)), WorkspaceChangeKind.SolutionChanged); | ||
| Contract.ThrowIfFalse( |
There was a problem hiding this comment.
This is a basis for the assumption that reusing the same analyzer references array is fine.
There was a problem hiding this comment.
It looks likes the AnalyzerFileReference may reference the actual DiagnosticAnalyzer / ISourceGenerator instances? I am not the most familiar here, but at a glance it seems dangerous to share them between two different workspaces?https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerFileReference.cs#L39
Maybe @jasonmalinowski is more familiar here?
There was a problem hiding this comment.
Neither of those types are aware of the workspaces layer, and, are expected to be able to be used with a variety of compilations, etc. in parallel, so, it feels unexpected for the sharing to be problematic. Let's definitely get confirmation though.
There was a problem hiding this comment.
Note that if we went back to essentially doing this work twice, like before, we would probably add 0.5s to the wall clock time.
There was a problem hiding this comment.
yeah if these are shareable, then this definitely makes sense to do. Just not sure on the contract here for that type.
2 participants
I found this while investigating slow test perf in
FileBasedProgramsWorkspaceTests.I used test case
TestSemanticDiagnosticsNotEnabledWhenCoarseGrainedFlagDisabled(False)as a reference. This test is not supposed to perform any design time builds, etc., so is expected to be "low cost" compared to many tests in this class.trueargument.)I was able to get some info using "Profile Test" in VS, and measuring "Async Activities". Unfortunately, it seems like the profiler doesn't have full ability to 'trace cause and effect', when, what we are essentially doing is sending a request into a stream, and another thread is reading it out. So, the information I got from the profiler, hinted at CreateTestLspServerAsync, and the Initialize request taking a long time specifically.
However, I wasn't able to get more details than that from the profiler data. Eventually I simply resorted to stepping thru the work the Initialize handler was doing, and literally just noticing when things 'felt slow', and finally sticking in
Log()calls with timestamps to verify where the time was going. It would be fantastic to work out how to do this better in the future.This is not great, and possibly represents me misusing the tools. But, in this case I was at least able to identify a major cause of the slowness. Creating an
AnalyzerFileReferencewill cause us to crack the assembly file to get its name and stuff like that. Sometimes the wait time from that is significant. In practice there were ~45 analyzer DLLs to do this on. We were also doing essentially the exact same work back-to-back, so, sharing the resulting references array between host and misc workspaces, takes off ~0.5s from the test runtime.