-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Only cache compilation if we have the same set of analyzers #79978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
3b0f2bc
cbd9c74
948a505
9c9fdce
e2e3763
e5e01e3
37dfc63
135e1b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,15 +49,18 @@ public int GetHashCode((Checksum checksum, ImmutableArray<DiagnosticAnalyzer> an | |
| /// we've created for it. Note: the CompilationWithAnalyzersPair instance is dependent on the set of <see | ||
| /// cref="DiagnosticAnalyzer"/>s passed along with the project. | ||
| /// <para/> | ||
| /// The value of the table is a <see cref="ConcurrentDictionary{TKey, TValue}"/> that maps from the | ||
| /// The value of the table is a <see cref="Dictionary{TKey, TValue}"/> that maps from the | ||
| /// <see cref="Project"/> checksum the set of <see cref="DiagnosticAnalyzer"/>s being requested. | ||
| /// Note: this dictionary must be locked with <see cref="s_gate"/> before accessing it. | ||
| /// </summary> | ||
| private static readonly ConditionalWeakTable< | ||
| ProjectState, | ||
| ConcurrentDictionary< | ||
| SmallDictionary< | ||
| (Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers), | ||
| AsyncLazy<CompilationWithAnalyzersPair?>>> s_projectToCompilationWithAnalyzers = new(); | ||
|
|
||
| private static readonly SemaphoreSlim s_gate = new(initialCount: 1); | ||
|
|
||
| private static async Task<CompilationWithAnalyzersPair?> GetOrCreateCompilationWithAnalyzersAsync( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: i dont' like this design at all. it means on the client side we end up makign the compilatino, then create the compilationWithAnalyzersPair only to eventually call over to oop with the set of analyzers we want, where it does the same thing over ther eas well. we should make the call to oop much higher in stack. |
||
| Project project, | ||
| ImmutableArray<DiagnosticAnalyzer> analyzers, | ||
|
|
@@ -75,16 +78,24 @@ private static readonly ConditionalWeakTable< | |
| // recompute and cache with the new state sets. | ||
| var map = s_projectToCompilationWithAnalyzers.GetValue( | ||
| projectState, | ||
| // We will almost always have one item in this dict | ||
| static _ => new(ChecksumAndAnalyzersEqualityComparer.Instance)); | ||
|
|
||
| var lazy = map.GetOrAdd( | ||
| (checksum, analyzers), | ||
| checksumAndAnalyzers => AsyncLazy.Create(async cancellationToken => | ||
| AsyncLazy<CompilationWithAnalyzersPair?>? lazy; | ||
| using (await s_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false)) | ||
| { | ||
| var checksumAndAnalyzers = (checksum, analyzers); | ||
| if (!map.TryGetValue(checksumAndAnalyzers, out lazy)) | ||
| { | ||
| var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); | ||
| var compilationWithAnalyzersPair = CreateCompilationWithAnalyzers(projectState, compilation); | ||
| return compilationWithAnalyzersPair; | ||
| })); | ||
| lazy = AsyncLazy.Create(async cancellationToken => | ||
| { | ||
| var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false); | ||
| var compilationWithAnalyzersPair = CreateCompilationWithAnalyzers(projectState, compilation); | ||
| return compilationWithAnalyzersPair; | ||
| }); | ||
| map.Add(checksumAndAnalyzers, lazy); | ||
| } | ||
| } | ||
|
|
||
| return await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOte: i designed it this way so that we dont' toss out a very good 'full compilation with analyzers' and replace it with a 'compilation with analyzers with a single analyzer in it'.