From 439121ff41695931a075ddee01228933af560560 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 7 Feb 2025 13:06:14 -0800 Subject: [PATCH 1/5] Cleanup how we map from projects to compilations --- ...osticIncrementalAnalyzer_GetDiagnostics.cs | 2 +- ...Span.ProjectAndCompilationWithAnalyzers.cs | 23 ---------- ...crementalAnalyzer_GetDiagnosticsForSpan.cs | 45 ++++++++++++------- ...IncrementalAnalyzer_IncrementalAnalyzer.cs | 2 +- 4 files changed, 30 insertions(+), 42 deletions(-) delete mode 100644 src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.ProjectAndCompilationWithAnalyzers.cs diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs index efd20ab728d24..a92baa2103b9c 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs @@ -105,7 +105,7 @@ private async Task ProduceDiagnosticsAsync( var stateSets = stateSetsForProject.Where(s => ShouldIncludeStateSet(project, s)).ToImmutableArrayOrEmpty(); // unlike the suppressed (disabled) analyzer, we will include hidden diagnostic only analyzers here. - 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.GetProjectAnalysisDataAsync(compilation, project, stateSets, cancellationToken).ConfigureAwait(false); diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.ProjectAndCompilationWithAnalyzers.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.ProjectAndCompilationWithAnalyzers.cs deleted file mode 100644 index be9d14efc42f9..0000000000000 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.ProjectAndCompilationWithAnalyzers.cs +++ /dev/null @@ -1,23 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -namespace Microsoft.CodeAnalysis.Diagnostics; - -internal partial class DiagnosticAnalyzerService -{ - private partial class DiagnosticIncrementalAnalyzer - { - private sealed class ProjectAndCompilationWithAnalyzers - { - public Project Project { get; } - public CompilationWithAnalyzersPair? CompilationWithAnalyzers { get; } - - public ProjectAndCompilationWithAnalyzers(Project project, CompilationWithAnalyzersPair? compilationWithAnalyzers) - { - Project = project; - CompilationWithAnalyzers = compilationWithAnalyzers; - } - } - } -} diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs index 88b41904c5222..16a2acfcc6a1a 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs +++ b/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,10 +48,7 @@ public async Task> GetDiagnosticsForSpanAsync( /// 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 s_lastProjectAndCompilationWithAnalyzers = new(null); + private static readonly ConditionalWeakTable s_projectToCompilationWithAnalyzers = new(); private readonly DiagnosticIncrementalAnalyzer _owner; private readonly TextDocument _document; @@ -110,25 +108,38 @@ public static async Task CreateAsync( bool crashOnAnalyzerException, CancellationToken cancellationToken) { - if (s_lastProjectAndCompilationWithAnalyzers.TryGetTarget(out var projectAndCompilationWithAnalyzers) && - projectAndCompilationWithAnalyzers?.Project == project) + if (!project.SupportsCompilation) + return null; + + if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair)) + compilationWithAnalyzersPair = await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); + + if (compilationWithAnalyzersPair is null) + return null; + + // Make sure the cached pair matches the state sets we're asking about. if not, recompute and cache + // with the new state sets. + if (HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) + return compilationWithAnalyzersPair; + + return await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); + + async Task ComputeAndCacheCompilationWithAnalyzersAsync() { - if (projectAndCompilationWithAnalyzers.CompilationWithAnalyzers == null) - { - return null; - } + var compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync(project, stateSets, crashOnAnalyzerException, cancellationToken).ConfigureAwait(false); - if (HasAllAnalyzers(stateSets, projectAndCompilationWithAnalyzers.CompilationWithAnalyzers)) - { - return projectAndCompilationWithAnalyzers.CompilationWithAnalyzers; - } - } + // 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); + s_projectToCompilationWithAnalyzers.GetValue(project, _ => compilationWithAnalyzersPair); - var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(project, stateSets, crashOnAnalyzerException, cancellationToken).ConfigureAwait(false); + return compilationWithAnalyzersPair; + } s_lastProjectAndCompilationWithAnalyzers.SetTarget(new ProjectAndCompilationWithAnalyzers(project, compilationWithAnalyzers)); return compilationWithAnalyzers; - static bool HasAllAnalyzers(IEnumerable stateSets, CompilationWithAnalyzersPair compilationWithAnalyzers) + static bool HasAllAnalyzers(ImmutableArray stateSets, CompilationWithAnalyzersPair compilationWithAnalyzers) { foreach (var stateSet in stateSets) { diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index 04d76624c5176..be858e36cc816 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -25,7 +25,7 @@ public async Task> ForceAnalyzeProjectAsync(Proje var stateSetsForProject = await _stateManager.GetOrCreateStateSetsAsync(project, cancellationToken).ConfigureAwait(false); var stateSets = GetStateSetsForFullSolutionAnalysis(stateSetsForProject, project); - var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync( + var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync( project, stateSets, AnalyzerService.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false); var result = await GetProjectAnalysisDataAsync(compilationWithAnalyzers, project, stateSets, cancellationToken).ConfigureAwait(false); From 29c8e3d35f9a2913234c9f7c910accbf4d117c5c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 7 Feb 2025 13:15:19 -0800 Subject: [PATCH 2/5] In progress --- ...cIncrementalAnalyzer.CompilationManager.cs | 52 ++++++++++++++++ .../DiagnosticIncrementalAnalyzer.StateSet.cs | 61 +++++++++---------- ...crementalAnalyzer_GetDiagnosticsForSpan.cs | 51 ---------------- 3 files changed, 81 insertions(+), 83 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs index b13fd1018622e..2a85007769f16 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; +using ICSharpCode.Decompiler.CSharp.Syntax; namespace Microsoft.CodeAnalysis.Diagnostics; @@ -20,4 +21,55 @@ private partial class DiagnosticIncrementalAnalyzer crashOnAnalyzerException, cancellationToken); } + + private static async Task GetOrCreateCompilationWithAnalyzersAsync( + Project project, + ImmutableArray stateSets, + bool crashOnAnalyzerException, + CancellationToken cancellationToken) + { + if (!project.SupportsCompilation) + return null; + + if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair)) + compilationWithAnalyzersPair = await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); + + if (compilationWithAnalyzersPair is null) + return null; + + // Make sure the cached pair matches the state sets we're asking about. if not, recompute and cache + // with the new state sets. + if (HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) + return compilationWithAnalyzersPair; + + return await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); + + async Task ComputeAndCacheCompilationWithAnalyzersAsync() + { + var compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync(project, stateSets, crashOnAnalyzerException, cancellationToken).ConfigureAwait(false); + + // 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); + s_projectToCompilationWithAnalyzers.GetValue(project, _ => compilationWithAnalyzersPair); + + return compilationWithAnalyzersPair; + } + s_lastProjectAndCompilationWithAnalyzers.SetTarget(new ProjectAndCompilationWithAnalyzers(project, compilationWithAnalyzers)); + return compilationWithAnalyzers; + + static bool HasAllAnalyzers(ImmutableArray 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; + } + } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateSet.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateSet.cs index d40dfcb20d6d0..ee0183d8c592f 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateSet.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateSet.cs @@ -16,47 +16,44 @@ namespace Microsoft.CodeAnalysis.Diagnostics; internal partial class DiagnosticAnalyzerService { - private partial class DiagnosticIncrementalAnalyzer + /// + /// this contains all states regarding a + /// + private sealed class StateSet { - /// - /// this contains all states regarding a - /// - private sealed class StateSet - { - public readonly DiagnosticAnalyzer Analyzer; - public readonly bool IsHostAnalyzer; + public readonly DiagnosticAnalyzer Analyzer; + public readonly bool IsHostAnalyzer; - private readonly ConcurrentSet _activeDocuments; - private readonly ConcurrentDictionary _projectStates; + private readonly ConcurrentSet _activeDocuments; + private readonly ConcurrentDictionary _projectStates; - public StateSet(DiagnosticAnalyzer analyzer, bool isHostAnalyzer) - { - Analyzer = analyzer; - IsHostAnalyzer = isHostAnalyzer; + public StateSet(DiagnosticAnalyzer analyzer, bool isHostAnalyzer) + { + Analyzer = analyzer; + IsHostAnalyzer = isHostAnalyzer; - _activeDocuments = []; - _projectStates = new ConcurrentDictionary(concurrencyLevel: 2, capacity: 1); - } + _activeDocuments = []; + _projectStates = new ConcurrentDictionary(concurrencyLevel: 2, capacity: 1); + } - public bool IsActiveFile(DocumentId documentId) - => _activeDocuments.Contains(documentId); + public bool IsActiveFile(DocumentId documentId) + => _activeDocuments.Contains(documentId); - public bool TryGetProjectState(ProjectId projectId, [NotNullWhen(true)] out ProjectState? state) - => _projectStates.TryGetValue(projectId, out state); + public bool TryGetProjectState(ProjectId projectId, [NotNullWhen(true)] out ProjectState? state) + => _projectStates.TryGetValue(projectId, out state); - public void AddActiveDocument(DocumentId documentId) - => _activeDocuments.Add(documentId); + public void AddActiveDocument(DocumentId documentId) + => _activeDocuments.Add(documentId); - public ProjectState GetOrCreateProjectState(ProjectId projectId) - => _projectStates.GetOrAdd(projectId, static (id, self) => new ProjectState(self, id), this); + public ProjectState GetOrCreateProjectState(ProjectId projectId) + => _projectStates.GetOrAdd(projectId, static (id, self) => new ProjectState(self, id), this); - public void OnRemoved() - { - // ths stateset is being removed. - // TODO: we do this since InMemoryCache is static type. we might consider making it instance object - // of something. - InMemoryStorage.DropCache(Analyzer); - } + public void OnRemoved() + { + // ths stateset is being removed. + // TODO: we do this since InMemoryCache is static type. we might consider making it instance object + // of something. + InMemoryStorage.DropCache(Analyzer); } } } diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs index 16a2acfcc6a1a..cd7e83d3b25f8 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs @@ -102,57 +102,6 @@ public static async Task CreateAsync( range, priorityProvider, isExplicit, logPerformanceInfo, incrementalAnalysis, diagnosticKinds); } - private static async Task GetOrCreateCompilationWithAnalyzersAsync( - Project project, - ImmutableArray stateSets, - bool crashOnAnalyzerException, - CancellationToken cancellationToken) - { - if (!project.SupportsCompilation) - return null; - - if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair)) - compilationWithAnalyzersPair = await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); - - if (compilationWithAnalyzersPair is null) - return null; - - // Make sure the cached pair matches the state sets we're asking about. if not, recompute and cache - // with the new state sets. - if (HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) - return compilationWithAnalyzersPair; - - return await ComputeAndCacheCompilationWithAnalyzersAsync().ConfigureAwait(false); - - async Task ComputeAndCacheCompilationWithAnalyzersAsync() - { - var compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync(project, stateSets, crashOnAnalyzerException, cancellationToken).ConfigureAwait(false); - - // 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); - s_projectToCompilationWithAnalyzers.GetValue(project, _ => compilationWithAnalyzersPair); - - return compilationWithAnalyzersPair; - } - s_lastProjectAndCompilationWithAnalyzers.SetTarget(new ProjectAndCompilationWithAnalyzers(project, compilationWithAnalyzers)); - return compilationWithAnalyzers; - - static bool HasAllAnalyzers(ImmutableArray 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, From bb364087813597a93d2bdf792441b1eb0cbc7584 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 7 Feb 2025 13:34:40 -0800 Subject: [PATCH 3/5] move method --- .../DocumentAnalysisExecutor_Helpers.cs | 76 ----------------- ...cIncrementalAnalyzer.CompilationManager.cs | 84 ++++++++++++++++++- 2 files changed, 83 insertions(+), 77 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs index 8c03ede203bd1..7de75d8b668ab 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs @@ -128,82 +128,6 @@ static string GetLanguageSpecificId(string? language, string noLanguageId, strin language: language); } - /// - /// Should only be called on a that . - /// - public static async Task CreateCompilationWithAnalyzersAsync( - Project project, - ImmutableArray projectAnalyzers, - ImmutableArray hostAnalyzers, - bool crashOnAnalyzerException, - CancellationToken cancellationToken) - { - 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 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); - } - /// /// Return true if the given is not suppressed for the given project. /// NOTE: This API is intended to be used only for performance optimization. diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs index 9ab16e4e7f775..7d135619a47cb 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs @@ -2,10 +2,16 @@ // 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; @@ -33,7 +39,7 @@ internal partial class DiagnosticAnalyzerService if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair) || !HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) { - compilationWithAnalyzersPair = await DocumentAnalysisExecutor.CreateCompilationWithAnalyzersAsync( + compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync( project, stateSets.SelectAsArray(s => !s.IsHostAnalyzer, s => s.Analyzer), stateSets.SelectAsArray(s => s.IsHostAnalyzer, s => s.Analyzer), @@ -67,5 +73,81 @@ static bool HasAllAnalyzers(ImmutableArray stateSets, CompilationWithA return true; } + + // + // Should only be called on a that . + // + static async Task CreateCompilationWithAnalyzersAsync( + Project project, + ImmutableArray projectAnalyzers, + ImmutableArray hostAnalyzers, + bool crashOnAnalyzerException, + CancellationToken cancellationToken) + { + 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 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); } } From 488adf09791cdb44caeaab67b7f76a5837b9321c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 7 Feb 2025 13:35:40 -0800 Subject: [PATCH 4/5] Simplify with captures --- ...ticIncrementalAnalyzer.CompilationManager.cs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs index 7d135619a47cb..02d8688f9ec2d 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs @@ -39,12 +39,7 @@ internal partial class DiagnosticAnalyzerService if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair) || !HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) { - compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync( - project, - stateSets.SelectAsArray(s => !s.IsHostAnalyzer, s => s.Analyzer), - stateSets.SelectAsArray(s => s.IsHostAnalyzer, s => s.Analyzer), - crashOnAnalyzerException, - cancellationToken).ConfigureAwait(false); + compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync().ConfigureAwait(false); // 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 @@ -77,13 +72,11 @@ static bool HasAllAnalyzers(ImmutableArray stateSets, CompilationWithA // // Should only be called on a that . // - static async Task CreateCompilationWithAnalyzersAsync( - Project project, - ImmutableArray projectAnalyzers, - ImmutableArray hostAnalyzers, - bool crashOnAnalyzerException, - CancellationToken cancellationToken) + async Task 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 From 71df88d8ab9cf76361f149a3a6e9d608dd14ede4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 8 Feb 2025 10:56:01 -0800 Subject: [PATCH 5/5] Simplify subset logic --- ...cIncrementalAnalyzer.CompilationManager.cs | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs index 02d8688f9ec2d..0795b3454a313 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.CompilationManager.cs @@ -23,7 +23,7 @@ internal partial class DiagnosticAnalyzerService /// 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. /// - private static readonly ConditionalWeakTable s_projectToCompilationWithAnalyzers = new(); + private static readonly ConditionalWeakTable stateSets, CompilationWithAnalyzersPair? compilationWithAnalyzersPair)>> s_projectToCompilationWithAnalyzers = new(); private static async Task GetOrCreateCompilationWithAnalyzersAsync( Project project, @@ -34,12 +34,13 @@ internal partial class DiagnosticAnalyzerService if (!project.SupportsCompilation) return null; - // Make sure the cached pair matches the state sets we're asking about. if not, recompute and cache with - // the new state sets. - if (!s_projectToCompilationWithAnalyzers.TryGetValue(project, out var compilationWithAnalyzersPair) || - !HasAllAnalyzers(stateSets, compilationWithAnalyzersPair)) + // 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)) { - compilationWithAnalyzersPair = await CreateCompilationWithAnalyzersAsync().ConfigureAwait(false); + 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 @@ -48,26 +49,10 @@ internal partial class DiagnosticAnalyzerService // 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, _ => compilationWithAnalyzersPair); + s_projectToCompilationWithAnalyzers.GetValue(project, _ => tupleBox); } - return compilationWithAnalyzersPair; - - static bool HasAllAnalyzers(ImmutableArray stateSets, CompilationWithAnalyzersPair? compilationWithAnalyzers) - { - if (compilationWithAnalyzers is null) - return false; - - 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; - } + return tupleBox.Value.compilationWithAnalyzersPair; // // Should only be called on a that .