Only cache compilation if we have the same set of analyzers (#79978)

The core issue hee is that we were caching the CompilationWithAnalyzers
for a compilation with *all* analyzers. Then, when being asked to
compute again with, say, a single analyzer, we'd reuse the
CompialtionWithAnalyzers.

This would then take ages to run *all* analyzers, even when running a
fix-all for a single analyzer.

Author: CyrusNajmabadi

src/Features/Core/Portable/Diagnostics/Service/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
 using System.Runtime.CompilerServices;
@@ -16,14 +17,54 @@ namespace Microsoft.CodeAnalysis.Diagnostics;
 
 internal sealed partial class DiagnosticAnalyzerService
 {
+    private sealed class ChecksumAndAnalyzersEqualityComparer
+        : IEqualityComparer<(Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers)>
+    {
+        public static readonly ChecksumAndAnalyzersEqualityComparer Instance = new();
+
+        public bool Equals((Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers) x, (Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers) y)
+        {
+            if (x.checksum != y.checksum)
+                return false;
+
+            // Fast path for when the analyzers are the same reference.
+            return x.analyzers == y.analyzers || x.analyzers.SetEquals(y.analyzers);
+        }
+
+        public int GetHashCode((Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers) obj)
+        {
+            var hashCode = obj.checksum.GetHashCode();
+
+            // Use addition so that we're resilient to any order for the analyzers.
+            foreach (var analyzer in obj.analyzers)
+                hashCode += analyzer.GetHashCode();
+
+            return hashCode;
+        }
+    }
+
+    /// <summary>
+    /// Cached data from a <see cref="ProjectState"/> to the <see cref="CompilationWithAnalyzersPair"/>s
+    /// 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 SmallDictionary 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.  A 
+    /// small dictionary is chosen as this will normally only have one item in it (the current project
+    /// and all its analyzers).  Occasionally it will have more, if (for example) a request to run
+    /// a single analyzer is performed.
+    /// </summary>
+    private static readonly ConditionalWeakTable<
+        ProjectState,
+        SmallDictionary<
+            (Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers),
+            AsyncLazy<CompilationWithAnalyzersPair?>>> s_projectToCompilationWithAnalyzers = new();
+
     /// <summary>
-    /// Cached data from a <see cref="ProjectState"/> to the last <see cref="CompilationWithAnalyzersPair"/> instance
-    /// created for it.  Note: the CompilationWithAnalyzersPair instance is dependent on the set of <see
-    /// cref="DiagnosticAnalyzer"/>s passed along with the project.  As such, we might not be able to use a prior cached
-    /// value if the set of analyzers changes.  In that case, a new instance will be created and will be cached for the
-    /// next caller.
+    /// Protection around the SmallDictionary in <see cref="s_projectToCompilationWithAnalyzers"/>.
     /// </summary>
-    private static readonly ConditionalWeakTable<ProjectState, StrongBox<(Checksum checksum, ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationWithAnalyzersPair? compilationWithAnalyzersPair)>> s_projectToCompilationWithAnalyzers = new();
+    private static readonly SemaphoreSlim s_gate = new(initialCount: 1);
 
     private static async Task<CompilationWithAnalyzersPair?> GetOrCreateCompilationWithAnalyzersAsync(
         Project project,
@@ -35,41 +76,42 @@ internal sealed partial class DiagnosticAnalyzerService
         if (!project.SupportsCompilation)
             return null;
 
-        var projectState = project.State;
         var checksum = await project.GetDiagnosticChecksumAsync(cancellationToken).ConfigureAwait(false);
 
-        // Make sure the cached pair was computed with at least the same state sets we're asking about.  if not,
+        // Make sure the cached pair was computed with the same state sets we're asking about.  if not,
         // recompute and cache with the new state sets.
-        if (!s_projectToCompilationWithAnalyzers.TryGetValue(projectState, out var tupleBox) ||
-            tupleBox.Value.checksum != checksum ||
-            !analyzers.IsSubsetOf(tupleBox.Value.analyzers))
+        var map = s_projectToCompilationWithAnalyzers.GetValue(
+            project.State, static _ => new(ChecksumAndAnalyzersEqualityComparer.Instance));
+
+        AsyncLazy<CompilationWithAnalyzersPair?>? lazy;
+        using (await s_gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
         {
-            var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
-            var compilationWithAnalyzersPair = CreateCompilationWithAnalyzers(projectState, compilation);
-            tupleBox = new((checksum, analyzers, compilationWithAnalyzersPair));
-
-#if NET
-            s_projectToCompilationWithAnalyzers.AddOrUpdate(projectState, tupleBox);
-#else
-            // Make a best effort attempt to store the latest computed value against these state sets. If this
-            // fails (because another thread interleaves with this), that's ok.  We still return the pair we 
-            // computed, so our caller will still see the right data
-            s_projectToCompilationWithAnalyzers.Remove(projectState);
-
-            // Intentionally ignore the result of this.  We still want to use the value we computed above, even if
-            // another thread interleaves and sets a different value.
-            s_projectToCompilationWithAnalyzers.GetValue(projectState, _ => tupleBox);
-#endif
+            var checksumAndAnalyzers = (checksum, analyzers);
+            if (!map.TryGetValue(checksumAndAnalyzers, out lazy))
+            {
+                lazy = AsyncLazy.Create(
+                    asynchronousComputeFunction: CreateCompilationWithAnalyzersAsync,
+                    arg: (project, analyzers, hostAnalyzerInfo, crashOnAnalyzerException));
+                map.Add(checksumAndAnalyzers, lazy);
+            }
         }
 
-        return tupleBox.Value.compilationWithAnalyzersPair;
+        return await lazy.GetValueAsync(cancellationToken).ConfigureAwait(false);
 
         // <summary>
         // Should only be called on a <see cref="Project"/> that <see cref="Project.SupportsCompilation"/>.
         // </summary>
-        CompilationWithAnalyzersPair? CreateCompilationWithAnalyzers(
-            ProjectState project, Compilation compilation)
+        static async Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync(
+            (Project project,
+             ImmutableArray<DiagnosticAnalyzer> analyzers,
+             HostAnalyzerInfo hostAnalyzerInfo,
+             bool crashOnAnalyzerException) tuple,
+            CancellationToken cancellationToken)
         {
+            var (project, analyzers, hostAnalyzerInfo, crashOnAnalyzerException) = tuple;
+
+            var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
+
             var projectAnalyzers = analyzers.WhereAsArray(static (s, info) => !info.IsHostAnalyzer(s), hostAnalyzerInfo);
             var hostAnalyzers = analyzers.WhereAsArray(static (s, info) => info.IsHostAnalyzer(s), hostAnalyzerInfo);
 
@@ -105,7 +147,7 @@ internal sealed partial class DiagnosticAnalyzerService
             var projectCompilation = !filteredProjectAnalyzers.Any()
                 ? null
                 : compilation.WithAnalyzers(filteredProjectAnalyzers, new CompilationWithAnalyzersOptions(
-                    options: project.ProjectAnalyzerOptions,
+                    options: project.State.ProjectAnalyzerOptions,
                     onAnalyzerException: null,
                     analyzerExceptionFilter: exceptionFilter,
                     concurrentAnalysis: false,

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems

@@ -23,6 +23,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\SharedStopwatch.cs" Link="InternalUtilities\SharedStopwatch.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\Syntax\SyntaxTreeExtensions.cs" Link="Syntax\SyntaxTreeExtensions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\Collections\BitVector.cs" Link="Collections\BitVector.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\Collections\SmallDictionary.cs" Link="Collections\SmallDictionary.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\Collections\IOrderedReadOnlySet.cs" Link="Collections\IOrderedReadOnlySet.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\SourceGeneration\GeneratedCodeUtilities.cs" Link="SourceGeneration\GeneratedCodeUtilities.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\SourceCodeKindExtensions.cs" Link="Utilities\Compiler\SourceCodeKindExtensions.cs" />