Simplify the computation and caching of 'CompilationWithAnalyzers' fo…

…r a particular project in the diagnostics layer. (#77113)

Followup to #77111

src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs

@@ -11,6 +11,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
@@ -127,84 +128,6 @@ static string GetLanguageSpecificId(string? language, string noLanguageId, strin
                 language: language);
         }
 
-        public static async Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync(
-            Project project,
-            ImmutableArray<DiagnosticAnalyzer> projectAnalyzers,
-            ImmutableArray<DiagnosticAnalyzer> hostAnalyzers,
-            bool crashOnAnalyzerException,
-            CancellationToken cancellationToken)
-        {
-            var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
-            if (compilation == null)
-            {
-                // project doesn't support compilation
-                return null;
-            }
-
-            // Create driver that holds onto compilation and associated analyzers
-            var filteredProjectAnalyzers = projectAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
-            var filteredHostAnalyzers = hostAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
-            var filteredProjectSuppressors = filteredProjectAnalyzers.WhereAsArray(static a => a is DiagnosticSuppressor);
-            filteredHostAnalyzers = filteredHostAnalyzers.AddRange(filteredProjectSuppressors);
-
-            // PERF: there is no analyzers for this compilation.
-            //       compilationWithAnalyzer will throw if it is created with no analyzers which is perf optimization.
-            if (filteredProjectAnalyzers.IsEmpty && filteredHostAnalyzers.IsEmpty)
-            {
-                return null;
-            }
-
-            Contract.ThrowIfFalse(project.SupportsCompilation);
-            AssertCompilation(project, compilation);
-
-            // in IDE, we always set concurrentAnalysis == false otherwise, we can get into thread starvation due to
-            // async being used with synchronous blocking concurrency.
-            var projectAnalyzerOptions = new CompilationWithAnalyzersOptions(
-                options: project.AnalyzerOptions,
-                onAnalyzerException: null,
-                analyzerExceptionFilter: GetAnalyzerExceptionFilter(),
-                concurrentAnalysis: false,
-                logAnalyzerExecutionTime: true,
-                reportSuppressedDiagnostics: true);
-            var hostAnalyzerOptions = new CompilationWithAnalyzersOptions(
-                options: project.HostAnalyzerOptions,
-                onAnalyzerException: null,
-                analyzerExceptionFilter: GetAnalyzerExceptionFilter(),
-                concurrentAnalysis: false,
-                logAnalyzerExecutionTime: true,
-                reportSuppressedDiagnostics: true);
-
-            // Create driver that holds onto compilation and associated analyzers
-            return new CompilationWithAnalyzersPair(
-                filteredProjectAnalyzers.Any() ? compilation.WithAnalyzers(filteredProjectAnalyzers, projectAnalyzerOptions) : null,
-                filteredHostAnalyzers.Any() ? compilation.WithAnalyzers(filteredHostAnalyzers, hostAnalyzerOptions) : null);
-
-            Func<Exception, bool> GetAnalyzerExceptionFilter()
-            {
-                return ex =>
-                {
-                    if (ex is not OperationCanceledException && crashOnAnalyzerException)
-                    {
-                        // report telemetry
-                        FatalError.ReportAndPropagate(ex);
-
-                        // force fail fast (the host might not crash when reporting telemetry):
-                        FailFast.OnFatalException(ex);
-                    }
-
-                    return true;
-                };
-            }
-        }
-
-        [Conditional("DEBUG")]
-        private static void AssertCompilation(Project project, Compilation compilation1)
-        {
-            // given compilation must be from given project.
-            Contract.ThrowIfFalse(project.TryGetCompilation(out var compilation2));
-            Contract.ThrowIfFalse(compilation1 == compilation2);
-        }
-
         /// <summary>
         /// Return true if the given <paramref name="analyzer"/> is not suppressed for the given project.
         /// NOTE: This API is intended to be used only for performance optimization.

src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs

@@ -2,22 +2,130 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.ErrorReporting;
+using Microsoft.CodeAnalysis.Shared.Extensions;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Diagnostics;
 
 internal partial class DiagnosticAnalyzerService
 {
-    private partial class DiagnosticIncrementalAnalyzer
+    /// <summary>
+    /// Cached data from a <see cref="Project"/> to the last <see cref="CompilationWithAnalyzersPair"/> instance created
+    /// for it.  Note: the CompilationWithAnalyzersPair instance is dependent on the set of <see cref="StateSet"/>s
+    /// passed along with the project.  As such, we might not be able to use a prior cached value if the set of state
+    /// sets changes.  In that case, a new instance will be created and will be cached for the next caller.
+    /// </summary>
+    private static readonly ConditionalWeakTable<Project, StrongBox<(ImmutableArray<StateSet> stateSets, CompilationWithAnalyzersPair? compilationWithAnalyzersPair)>> s_projectToCompilationWithAnalyzers = new();
+
+    private static async Task<CompilationWithAnalyzersPair?> GetOrCreateCompilationWithAnalyzersAsync(
+        Project project,
+        ImmutableArray<StateSet> stateSets,
+        bool crashOnAnalyzerException,
+        CancellationToken cancellationToken)
+    {
+        if (!project.SupportsCompilation)
+            return null;
+
+        // Make sure the cached pair was computed with at least the same state sets we're asking about.  if not,
+        // recompute and cache with the new state sets.
+        if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var tupleBox) ||
+            !stateSets.IsSubsetOf(tupleBox.Value.stateSets))
+        {
+            var compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync().ConfigureAwait(false);
+            tupleBox = new((stateSets, compilationWithAnalyzersPair));
+
+            // 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(project);
+
+            // 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(project, _ => tupleBox);
+        }
+
+        return tupleBox.Value.compilationWithAnalyzersPair;
+
+        // <summary>
+        // Should only be called on a <see cref="Project"/> that <see cref="Project.SupportsCompilation"/>.
+        // </summary>
+        async Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync()
+        {
+            var projectAnalyzers = stateSets.SelectAsArray(s => !s.IsHostAnalyzer, s => s.Analyzer);
+            var hostAnalyzers = stateSets.SelectAsArray(s => s.IsHostAnalyzer, s => s.Analyzer);
+
+            var compilation = await project.GetRequiredCompilationAsync(cancellationToken).ConfigureAwait(false);
+
+            // Create driver that holds onto compilation and associated analyzers
+            var filteredProjectAnalyzers = projectAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
+            var filteredHostAnalyzers = hostAnalyzers.WhereAsArray(static a => !a.IsWorkspaceDiagnosticAnalyzer());
+            var filteredProjectSuppressors = filteredProjectAnalyzers.WhereAsArray(static a => a is DiagnosticSuppressor);
+            filteredHostAnalyzers = filteredHostAnalyzers.AddRange(filteredProjectSuppressors);
+
+            // PERF: there is no analyzers for this compilation.
+            //       compilationWithAnalyzer will throw if it is created with no analyzers which is perf optimization.
+            if (filteredProjectAnalyzers.IsEmpty && filteredHostAnalyzers.IsEmpty)
+            {
+                return null;
+            }
+
+            Contract.ThrowIfFalse(project.SupportsCompilation);
+            AssertCompilation(project, compilation);
+
+            // in IDE, we always set concurrentAnalysis == false otherwise, we can get into thread starvation due to
+            // async being used with synchronous blocking concurrency.
+            var projectAnalyzerOptions = new CompilationWithAnalyzersOptions(
+                options: project.AnalyzerOptions,
+                onAnalyzerException: null,
+                analyzerExceptionFilter: GetAnalyzerExceptionFilter(),
+                concurrentAnalysis: false,
+                logAnalyzerExecutionTime: true,
+                reportSuppressedDiagnostics: true);
+            var hostAnalyzerOptions = new CompilationWithAnalyzersOptions(
+                options: project.HostAnalyzerOptions,
+                onAnalyzerException: null,
+                analyzerExceptionFilter: GetAnalyzerExceptionFilter(),
+                concurrentAnalysis: false,
+                logAnalyzerExecutionTime: true,
+                reportSuppressedDiagnostics: true);
+
+            // Create driver that holds onto compilation and associated analyzers
+            return new CompilationWithAnalyzersPair(
+                filteredProjectAnalyzers.Any() ? compilation.WithAnalyzers(filteredProjectAnalyzers, projectAnalyzerOptions) : null,
+                filteredHostAnalyzers.Any() ? compilation.WithAnalyzers(filteredHostAnalyzers, hostAnalyzerOptions) : null);
+
+            Func<Exception, bool> GetAnalyzerExceptionFilter()
+            {
+                return ex =>
+                {
+                    if (ex is not OperationCanceledException && crashOnAnalyzerException)
+                    {
+                        // report telemetry
+                        FatalError.ReportAndPropagate(ex);
+
+                        // force fail fast (the host might not crash when reporting telemetry):
+                        FailFast.OnFatalException(ex);
+                    }
+
+                    return true;
+                };
+            }
+        }
+    }
+
+    [Conditional("DEBUG")]
+    private static void AssertCompilation(Project project, Compilation compilation1)
     {
-        private static Task<CompilationWithAnalyzersPair?> CreateCompilationWithAnalyzersAsync(Project project, ImmutableArray<StateSet> stateSets, bool crashOnAnalyzerException, CancellationToken cancellationToken)
-            => DocumentAnalysisExecutor.CreateCompilationWithAnalyzersAsync(
-                project,
-                stateSets.SelectAsArray(s => !s.IsHostAnalyzer, s => s.Analyzer),
-                stateSets.SelectAsArray(s => s.IsHostAnalyzer, s => s.Analyzer),
-                crashOnAnalyzerException,
-                cancellationToken);
+        // given compilation must be from given project.
+        Contract.ThrowIfFalse(project.TryGetCompilation(out var compilation2));
+        Contract.ThrowIfFalse(compilation1 == compilation2);
     }
 }

src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateSet.cs

@@ -6,21 +6,18 @@ namespace Microsoft.CodeAnalysis.Diagnostics;
 
 internal partial class DiagnosticAnalyzerService
 {
-    private partial class DiagnosticIncrementalAnalyzer
+    /// <summary>
+    /// this contains all states regarding a <see cref="DiagnosticAnalyzer"/>
+    /// </summary>
+    private sealed class StateSet
     {
-        /// <summary>
-        /// this contains all states regarding a <see cref="DiagnosticAnalyzer"/>
-        /// </summary>
-        private sealed class StateSet
-        {
-            public readonly DiagnosticAnalyzer Analyzer;
-            public readonly bool IsHostAnalyzer;
+        public readonly DiagnosticAnalyzer Analyzer;
+        public readonly bool IsHostAnalyzer;
 
-            public StateSet(DiagnosticAnalyzer analyzer, bool isHostAnalyzer)
-            {
-                Analyzer = analyzer;
-                IsHostAnalyzer = isHostAnalyzer;
-            }
+        public StateSet(DiagnosticAnalyzer analyzer, bool isHostAnalyzer)
+        {
+            Analyzer = analyzer;
+            IsHostAnalyzer = isHostAnalyzer;
         }
     }
 }

src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs

@@ -149,7 +149,7 @@ async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnalysisResult>> Ge
                     }
 
                     // Otherwise, just compute for the state sets we care about.
-                    var compilation = await CreateCompilationWithAnalyzersAsync(project, stateSets, Owner.AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
+                    var compilation = await GetOrCreateCompilationWithAnalyzersAsync(project, stateSets, Owner.AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
 
                     var result = await Owner.ComputeDiagnosticAnalysisResultsAsync(compilation, project, stateSets, cancellationToken).ConfigureAwait(false);
                     return result;

src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs

@@ -7,6 +7,7 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeActions;
@@ -47,11 +48,6 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsForSpanAsync(
         /// </summary>
         private sealed class LatestDiagnosticsForSpanGetter
         {
-            // PERF: Cache the last Project and corresponding CompilationWithAnalyzers used to compute analyzer diagnostics for span.
-            //       This is now required as async lightbulb will query and execute different priority buckets of analyzers with multiple
-            //       calls, and we want to reuse CompilationWithAnalyzers instance if possible. 
-            private static readonly WeakReference<ProjectAndCompilationWithAnalyzers?> s_lastProjectAndCompilationWithAnalyzers = new(null);
-
             private readonly DiagnosticIncrementalAnalyzer _owner;
             private readonly TextDocument _document;
             private readonly SourceText _text;
@@ -104,44 +100,6 @@ public static async Task<LatestDiagnosticsForSpanGetter> CreateAsync(
                     range, priorityProvider, isExplicit, logPerformanceInfo, incrementalAnalysis, diagnosticKinds);
             }
 
-            private static async Task<CompilationWithAnalyzersPair?> GetOrCreateCompilationWithAnalyzersAsync(
-                Project project,
-                ImmutableArray<StateSet> stateSets,
-                bool crashOnAnalyzerException,
-                CancellationToken cancellationToken)
-            {
-                if (s_lastProjectAndCompilationWithAnalyzers.TryGetTarget(out var projectAndCompilationWithAnalyzers) &&
-                    projectAndCompilationWithAnalyzers?.Project == project)
-                {
-                    if (projectAndCompilationWithAnalyzers.CompilationWithAnalyzers == null)
-                    {
-                        return null;
-                    }
-
-                    if (HasAllAnalyzers(stateSets, projectAndCompilationWithAnalyzers.CompilationWithAnalyzers))
-                    {
-                        return projectAndCompilationWithAnalyzers.CompilationWithAnalyzers;
-                    }
-                }
-
-                var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(project, stateSets, crashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
-                s_lastProjectAndCompilationWithAnalyzers.SetTarget(new ProjectAndCompilationWithAnalyzers(project, compilationWithAnalyzers));
-                return compilationWithAnalyzers;
-
-                static bool HasAllAnalyzers(IEnumerable<StateSet> stateSets, CompilationWithAnalyzersPair compilationWithAnalyzers)
-                {
-                    foreach (var stateSet in stateSets)
-                    {
-                        if (stateSet.IsHostAnalyzer && !compilationWithAnalyzers.HostAnalyzers.Contains(stateSet.Analyzer))
-                            return false;
-                        else if (!stateSet.IsHostAnalyzer && !compilationWithAnalyzers.ProjectAnalyzers.Contains(stateSet.Analyzer))
-                            return false;
-                    }
-
-                    return true;
-                }
-            }
-
             private LatestDiagnosticsForSpanGetter(
                 DiagnosticIncrementalAnalyzer owner,
                 CompilationWithAnalyzersPair? compilationWithAnalyzers,

src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs

@@ -60,7 +60,7 @@ public async Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Proje
                     static (stateSet, arg) => arg.self.IsCandidateForFullSolutionAnalysis(stateSet.Analyzer, stateSet.IsHostAnalyzer, arg.project),
                     (self: this, project));
 
-                var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(
+                var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync(
                     project, fullSolutionAnalysisStateSets, AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
 
                 var projectAnalysisData = await ComputeDiagnosticAnalysisResultsAsync(compilationWithAnalyzers, project, fullSolutionAnalysisStateSets, cancellationToken).ConfigureAwait(false);