Adds parallel execution of background Roslyn analyzers #2312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
there is now a new option in the roslyn-extensions-option: ThreadsToUseForAnalyzers - which defaults to half the number of processors on the machine. there are two workers started, one for background and one for foreground. so this *might* actually use all the cores? in my usage it seems not to.
Co-authored-by: David Zidar <[email protected]>
same logic as for the diagnostic worker with analyzers.
since it's used for diagnostic with and without analyzers the name should reflect this
i should never try to do this stuff from github
- Parallelize at the document level instead of only at the project level (so user benefits also with few projects but many files) - Since currently analyzing project no longer has any real meaning, add new status events to convey "analyzing n remaining files", updates every 50 documents - Retain old status events for backward compat with clients - Use 75% of available cores instead of half
This was referenced Dec 19, 2021
hard-coding to a number of documents (e.g. 50 or 24) will either send updates very rarely giving large jumps, or very frequently (i.e. many times within one percentage-point). this change calculates how many documents correspond to 1%, and send an event every time that has been reached. if that is less than every 10 documents, it events on every tenth document. this avoids unnecessary updates and updates as often as relevant.
|
The improvements in this PR have been merged into the original PR #2285 - closing this one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2241
This PR builds upon the work started by @TomasEkeli in #2285 and adds some further improvements.
The original PR:
This PR addresses the above points by:
BackgroundDiagnosticStatusevent (and associated model types) in addition to the existingProjectDiagnosticStatusevent. The new event is modeled around total and remaining number of documents currently being analyzed, instead of the notion of current project. An update event is emitted every 24 documents (results in reasonable frequency on most machines, could be made time-based as an improvement) allowing clients to display meaningful progress. Code for handling the new status event can eventually be added toomnisharp-vscodeand other clients.Analyzing (23%)This PR also parallelizes slightly more aggressively by defaulting to 75% of available processors (rather than 50%).