Skip to content

Commit 4610f5a

Browse files
Move computation of deprioritized analyzers to oop (#79989)
2 parents 4ecc7b3 + 1a2b5d6 commit 4610f5a

File tree

7 files changed

+189
-46
lines changed

7 files changed

+189
-46
lines changed

src/Features/Core/Portable/Diagnostics/IDiagnosticAnalyzerService.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ internal interface IDiagnosticAnalyzerService : IWorkspaceService
2525
/// <inheritdoc cref="IRemoteDiagnosticAnalyzerService.ForceAnalyzeProjectAsync"/>
2626
Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Project project, CancellationToken cancellationToken);
2727

28+
/// <inheritdoc cref="IRemoteDiagnosticAnalyzerService.GetDeprioritizationCandidatesAsync"/>
29+
Task<ImmutableArray<DiagnosticAnalyzer>> GetDeprioritizationCandidatesAsync(
30+
Project project, ImmutableArray<DiagnosticAnalyzer> analyzers, CancellationToken cancellationToken);
31+
2832
/// <summary>
2933
/// Get diagnostics of the given diagnostic ids and/or analyzers from the given solution. all diagnostics returned
3034
/// should be up-to-date with respect to the given solution. Note that for project case, this method returns

src/Features/Core/Portable/Diagnostics/Service/DiagnosticAnalyzerService.cs

Lines changed: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@
99
using System.IO;
1010
using System.Linq;
1111
using System.Reflection;
12+
using System.Reflection.Metadata;
1213
using System.Threading;
1314
using System.Threading.Tasks;
1415
using Microsoft.CodeAnalysis.CodeActions;
1516
using Microsoft.CodeAnalysis.CodeStyle;
1617
using Microsoft.CodeAnalysis.Host;
1718
using Microsoft.CodeAnalysis.Host.Mef;
1819
using Microsoft.CodeAnalysis.Options;
20+
using Microsoft.CodeAnalysis.PooledObjects;
1921
using Microsoft.CodeAnalysis.Remote;
2022
using Microsoft.CodeAnalysis.Shared.TestHooks;
2123
using Microsoft.CodeAnalysis.SolutionCrawler;
@@ -176,22 +178,47 @@ public async Task<ImmutableArray<DiagnosticData>> GetProjectDiagnosticsForIdsAsy
176178
cancellationToken).ConfigureAwait(false);
177179
}
178180

179-
private Task<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync(
181+
internal async Task<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync(
180182
Project project,
181183
ImmutableArray<DiagnosticAnalyzer> analyzers,
182184
ImmutableHashSet<string>? diagnosticIds,
183-
IReadOnlyList<DocumentId> documentIds,
185+
ImmutableArray<DocumentId> documentIds,
184186
bool includeLocalDocumentDiagnostics,
185187
bool includeNonLocalDocumentDiagnostics,
186188
bool includeProjectNonLocalResult,
187189
CancellationToken cancellationToken)
188190
{
189-
return _incrementalAnalyzer.ProduceProjectDiagnosticsAsync(
191+
var client = await RemoteHostClient.TryGetClientAsync(project, cancellationToken).ConfigureAwait(false);
192+
if (client is not null)
193+
{
194+
var analyzerIds = analyzers.Select(a => a.GetAnalyzerId()).ToImmutableHashSet();
195+
var result = await client.TryInvokeAsync<IRemoteDiagnosticAnalyzerService, ImmutableArray<DiagnosticData>>(
196+
project,
197+
(service, solution, cancellationToken) => service.ProduceProjectDiagnosticsAsync(
198+
solution, project.Id, analyzerIds, diagnosticIds, documentIds,
199+
includeLocalDocumentDiagnostics, includeNonLocalDocumentDiagnostics, includeProjectNonLocalResult,
200+
cancellationToken),
201+
cancellationToken).ConfigureAwait(false);
202+
if (!result.HasValue)
203+
return [];
204+
205+
return result.Value;
206+
}
207+
208+
// Fallback to proccessing in proc.
209+
return await _incrementalAnalyzer.ProduceProjectDiagnosticsAsync(
190210
project, analyzers, diagnosticIds, documentIds,
191211
includeLocalDocumentDiagnostics,
192212
includeNonLocalDocumentDiagnostics,
193213
includeProjectNonLocalResult,
194-
cancellationToken);
214+
cancellationToken).ConfigureAwait(false);
215+
}
216+
217+
internal Task<ImmutableArray<DiagnosticAnalyzer>> GetProjectAnalyzersAsync(
218+
Project project, CancellationToken cancellationToken)
219+
{
220+
return _stateManager.GetOrCreateAnalyzersAsync(
221+
project.Solution.SolutionState, project.State, cancellationToken);
195222
}
196223

197224
private async Task<ImmutableArray<DiagnosticAnalyzer>> GetDiagnosticAnalyzersAsync(
@@ -200,9 +227,7 @@ private async Task<ImmutableArray<DiagnosticAnalyzer>> GetDiagnosticAnalyzersAsy
200227
Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer,
201228
CancellationToken cancellationToken)
202229
{
203-
var analyzersForProject = await _stateManager.GetOrCreateAnalyzersAsync(
204-
project.Solution.SolutionState, project.State, cancellationToken).ConfigureAwait(false);
205-
230+
var analyzersForProject = await GetProjectAnalyzersAsync(project, cancellationToken).ConfigureAwait(false);
206231
var analyzers = analyzersForProject.WhereAsArray(a => ShouldIncludeAnalyzer(project, a));
207232

208233
return analyzers;
@@ -355,6 +380,59 @@ public async Task<ImmutableDictionary<string, ImmutableArray<DiagnosticDescripto
355380
return project.Solution.SolutionState.Analyzers.GetDiagnosticDescriptorsPerReference(this._analyzerInfoCache, project);
356381
}
357382

383+
public async Task<ImmutableArray<DiagnosticAnalyzer>> GetDeprioritizationCandidatesAsync(
384+
Project project, ImmutableArray<DiagnosticAnalyzer> analyzers, CancellationToken cancellationToken)
385+
{
386+
var client = await RemoteHostClient.TryGetClientAsync(project, cancellationToken).ConfigureAwait(false);
387+
if (client is not null)
388+
{
389+
var analyzerIds = analyzers.Select(a => a.GetAnalyzerId()).ToImmutableHashSet();
390+
var result = await client.TryInvokeAsync<IRemoteDiagnosticAnalyzerService, ImmutableHashSet<string>>(
391+
project,
392+
(service, solution, cancellationToken) => service.GetDeprioritizationCandidatesAsync(
393+
solution, project.Id, analyzerIds, cancellationToken),
394+
cancellationToken).ConfigureAwait(false);
395+
if (!result.HasValue)
396+
return [];
397+
398+
return analyzers.FilterAnalyzers(result.Value);
399+
}
400+
401+
using var _ = ArrayBuilder<DiagnosticAnalyzer>.GetInstance(out var builder);
402+
403+
var hostAnalyzerInfo = await _stateManager.GetOrCreateHostAnalyzerInfoAsync(
404+
project.Solution.SolutionState, project.State, cancellationToken).ConfigureAwait(false);
405+
var compilationWithAnalyzers = await GetOrCreateCompilationWithAnalyzersAsync(
406+
project, analyzers, hostAnalyzerInfo, this.CrashOnAnalyzerException, cancellationToken).ConfigureAwait(false);
407+
408+
foreach (var analyzer in analyzers)
409+
{
410+
if (await IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(analyzer).ConfigureAwait(false))
411+
builder.Add(analyzer);
412+
}
413+
414+
return builder.ToImmutableAndClear();
415+
416+
async Task<bool> IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(DiagnosticAnalyzer analyzer)
417+
{
418+
// We deprioritize SymbolStart/End and SemanticModel analyzers from 'Normal' to 'Low' priority bucket,
419+
// as these are computationally more expensive.
420+
// Note that we never de-prioritize compiler analyzer, even though it registers a SemanticModel action.
421+
if (compilationWithAnalyzers == null ||
422+
analyzer.IsWorkspaceDiagnosticAnalyzer() ||
423+
analyzer.IsCompilerAnalyzer())
424+
{
425+
return false;
426+
}
427+
428+
var telemetryInfo = await compilationWithAnalyzers.GetAnalyzerTelemetryInfoAsync(analyzer, cancellationToken).ConfigureAwait(false);
429+
if (telemetryInfo == null)
430+
return false;
431+
432+
return telemetryInfo.SymbolStartActionsCount > 0 || telemetryInfo.SemanticModelActionsCount > 0;
433+
}
434+
}
435+
358436
private sealed class DiagnosticAnalyzerComparer : IEqualityComparer<DiagnosticAnalyzer>
359437
{
360438
public static readonly DiagnosticAnalyzerComparer Instance = new();

src/Features/Core/Portable/Diagnostics/Service/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public async Task<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync
2222
Project project,
2323
ImmutableArray<DiagnosticAnalyzer> analyzers,
2424
ImmutableHashSet<string>? diagnosticIds,
25-
IReadOnlyList<DocumentId> documentIds,
25+
ImmutableArray<DocumentId> documentIds,
2626
bool includeLocalDocumentDiagnostics,
2727
bool includeNonLocalDocumentDiagnostics,
2828
bool includeProjectNonLocalResult,

src/Features/Core/Portable/Diagnostics/Service/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnosticsForSpan.cs

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6+
using System.Collections.Generic;
67
using System.Collections.Immutable;
78
using System.Diagnostics;
89
using System.Linq;
@@ -203,18 +204,21 @@ async Task ComputeDocumentDiagnosticsAsync(
203204
Debug.Assert(!incrementalAnalysis || kind == AnalysisKind.Semantic);
204205
Debug.Assert(!incrementalAnalysis || analyzers.All(analyzer => analyzer.SupportsSpanBasedSemanticDiagnosticAnalysis()));
205206

206-
using var _ = ArrayBuilder<DiagnosticAnalyzer>.GetInstance(analyzers.Length, out var filteredAnalyzers);
207+
using var _1 = ArrayBuilder<DiagnosticAnalyzer>.GetInstance(analyzers.Length, out var filteredAnalyzers);
208+
using var _2 = PooledHashSet<DiagnosticAnalyzer>.GetInstance(out var deprioritizationCandidates);
209+
210+
deprioritizationCandidates.AddRange(await this.AnalyzerService.GetDeprioritizationCandidatesAsync(
211+
project, analyzers, cancellationToken).ConfigureAwait(false));
212+
207213
foreach (var analyzer in analyzers)
208214
{
209215
Debug.Assert(priorityProvider.MatchesPriority(analyzer));
210216

211217
// Check if this is an expensive analyzer that needs to be de-prioritized to a lower priority bucket.
212218
// If so, we skip this analyzer from execution in the current priority bucket.
213219
// We will subsequently execute this analyzer in the lower priority bucket.
214-
if (await TryDeprioritizeAnalyzerAsync(analyzer, kind, span).ConfigureAwait(false))
215-
{
220+
if (TryDeprioritizeAnalyzer(analyzer, kind, span, deprioritizationCandidates))
216221
continue;
217-
}
218222

219223
filteredAnalyzers.Add(analyzer);
220224
}
@@ -224,8 +228,6 @@ async Task ComputeDocumentDiagnosticsAsync(
224228

225229
analyzers = filteredAnalyzers.ToImmutable();
226230

227-
var hostAnalyzerInfo = await StateManager.GetOrCreateHostAnalyzerInfoAsync(solutionState, project.State, cancellationToken).ConfigureAwait(false);
228-
229231
var projectAnalyzers = analyzers.WhereAsArray(static (a, info) => !info.IsHostAnalyzer(a), hostAnalyzerInfo);
230232
var hostAnalyzers = analyzers.WhereAsArray(static (a, info) => info.IsHostAnalyzer(a), hostAnalyzerInfo);
231233
var analysisScope = new DocumentAnalysisScope(document, span, projectAnalyzers, hostAnalyzers, kind);
@@ -235,7 +237,7 @@ async Task ComputeDocumentDiagnosticsAsync(
235237
ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<DiagnosticData>> diagnosticsMap;
236238
if (incrementalAnalysis)
237239
{
238-
using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Incremental");
240+
using var _3 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Incremental");
239241

240242
diagnosticsMap = await _incrementalMemberEditAnalyzer.ComputeDiagnosticsAsync(
241243
executor,
@@ -245,7 +247,7 @@ async Task ComputeDocumentDiagnosticsAsync(
245247
}
246248
else
247249
{
248-
using var _2 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Document");
250+
using var _3 = TelemetryLogging.LogBlockTimeAggregatedHistogram(FunctionId.RequestDiagnostics_Summary, $"Pri{priorityProvider.Priority.GetPriorityInt()}.Document");
249251

250252
diagnosticsMap = await ComputeDocumentDiagnosticsCoreAsync(executor, cancellationToken).ConfigureAwait(false);
251253
}
@@ -260,8 +262,9 @@ async Task ComputeDocumentDiagnosticsAsync(
260262
_incrementalMemberEditAnalyzer.UpdateDocumentWithCachedDiagnostics((Document)document);
261263
}
262264

263-
async Task<bool> TryDeprioritizeAnalyzerAsync(
264-
DiagnosticAnalyzer analyzer, AnalysisKind kind, TextSpan? span)
265+
bool TryDeprioritizeAnalyzer(
266+
DiagnosticAnalyzer analyzer, AnalysisKind kind, TextSpan? span,
267+
HashSet<DiagnosticAnalyzer> deprioritizationCandidates)
265268
{
266269
// PERF: In order to improve lightbulb performance, we perform de-prioritization optimization for certain analyzers
267270
// that moves the analyzer to a lower priority bucket. However, to ensure that de-prioritization happens for very rare cases,
@@ -284,10 +287,8 @@ async Task<bool> TryDeprioritizeAnalyzerAsync(
284287

285288
// Condition 3.
286289
// Check if this is a candidate analyzer that can be de-prioritized into a lower priority bucket based on registered actions.
287-
if (!await IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(analyzer).ConfigureAwait(false))
288-
{
290+
if (!deprioritizationCandidates.Contains(analyzer))
289291
return false;
290-
}
291292

292293
// 'LightbulbSkipExecutingDeprioritizedAnalyzers' option determines if we want to execute this analyzer
293294
// in low priority bucket or skip it completely. If the option is not set, track the de-prioritized
@@ -301,31 +302,6 @@ async Task<bool> TryDeprioritizeAnalyzerAsync(
301302
return true;
302303
}
303304

304-
// Returns true if this is an analyzer that is a candidate to be de-prioritized to
305-
// 'CodeActionRequestPriority.Low' priority for improvement in analyzer
306-
// execution performance for priority buckets above 'Low' priority.
307-
// Based on performance measurements, currently only analyzers which register SymbolStart/End actions
308-
// or SemanticModel actions are considered candidates to be de-prioritized. However, these semantics
309-
// could be changed in future based on performance measurements.
310-
async Task<bool> IsCandidateForDeprioritizationBasedOnRegisteredActionsAsync(DiagnosticAnalyzer analyzer)
311-
{
312-
// We deprioritize SymbolStart/End and SemanticModel analyzers from 'Normal' to 'Low' priority bucket,
313-
// as these are computationally more expensive.
314-
// Note that we never de-prioritize compiler analyzer, even though it registers a SemanticModel action.
315-
if (compilationWithAnalyzers == null ||
316-
analyzer.IsWorkspaceDiagnosticAnalyzer() ||
317-
analyzer.IsCompilerAnalyzer())
318-
{
319-
return false;
320-
}
321-
322-
var telemetryInfo = await compilationWithAnalyzers.GetAnalyzerTelemetryInfoAsync(analyzer, cancellationToken).ConfigureAwait(false);
323-
if (telemetryInfo == null)
324-
return false;
325-
326-
return telemetryInfo.SymbolStartActionsCount > 0 || telemetryInfo.SemanticModelActionsCount > 0;
327-
}
328-
329305
bool ShouldInclude(DiagnosticData diagnostic)
330306
{
331307
return diagnostic.DocumentId == document.Id &&

src/Workspaces/Core/Portable/Diagnostics/Extensions.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,4 +568,20 @@ public static bool IsReportedInDocument(Diagnostic diagnostic, TextDocument targ
568568

569569
return false;
570570
}
571+
572+
public static ImmutableArray<DiagnosticAnalyzer> FilterAnalyzers(
573+
this ImmutableArray<DiagnosticAnalyzer> analyzers,
574+
ImmutableHashSet<string> analyzerIds)
575+
{
576+
using var _ = PooledDictionary<string, DiagnosticAnalyzer>.GetInstance(out var analyzerMap);
577+
foreach (var analyzer in analyzers)
578+
{
579+
// In the case of multiple analyzers with the same ID, we keep the last one.
580+
var analyzerId = analyzer.GetAnalyzerId();
581+
if (analyzerIds.Contains(analyzerId))
582+
analyzerMap[analyzerId] = analyzer;
583+
}
584+
585+
return [.. analyzerMap.Values];
586+
}
571587
}

src/Workspaces/Core/Portable/Diagnostics/IRemoteDiagnosticAnalyzerService.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Runtime.Serialization;
88
using System.Threading;
99
using System.Threading.Tasks;
10+
using Microsoft.CodeAnalysis.CodeActions;
1011

1112
namespace Microsoft.CodeAnalysis.Diagnostics;
1213

@@ -17,7 +18,27 @@ internal interface IRemoteDiagnosticAnalyzerService
1718
/// </summary>
1819
ValueTask<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Checksum solutionChecksum, ProjectId projectId, CancellationToken cancellationToken);
1920

21+
/// <summary>
22+
/// Returns the analyzers that are candidates to be de-prioritized to
23+
/// <see cref="CodeActionRequestPriority.Low"/> priority for improvement in analyzer
24+
/// execution performance for priority buckets above 'Low' priority.
25+
/// Based on performance measurements, currently only analyzers which register SymbolStart/End actions
26+
/// or SemanticModel actions are considered candidates to be de-prioritized. However, these semantics
27+
/// could be changed in future based on performance measurements.
28+
/// </summary>
29+
ValueTask<ImmutableHashSet<string>> GetDeprioritizationCandidatesAsync(
30+
Checksum solutionChecksum, ProjectId projectId, ImmutableHashSet<string> analyzerIds, CancellationToken cancellationToken);
31+
2032
ValueTask<SerializableDiagnosticAnalysisResults> CalculateDiagnosticsAsync(Checksum solutionChecksum, DiagnosticArguments arguments, CancellationToken cancellationToken);
33+
ValueTask<ImmutableArray<DiagnosticData>> ProduceProjectDiagnosticsAsync(
34+
Checksum solutionChecksum, ProjectId projectId,
35+
ImmutableHashSet<string> analyzerIds,
36+
ImmutableHashSet<string>? diagnosticIds,
37+
ImmutableArray<DocumentId> documentIds,
38+
bool includeLocalDocumentDiagnostics,
39+
bool includeNonLocalDocumentDiagnostics,
40+
bool includeProjectNonLocalResult,
41+
CancellationToken cancellationToken);
2142

2243
ValueTask<ImmutableArray<DiagnosticData>> GetSourceGeneratorDiagnosticsAsync(Checksum solutionChecksum, ProjectId projectId, CancellationToken cancellationToken);
2344
ValueTask ReportAnalyzerPerformanceAsync(ImmutableArray<AnalyzerPerformanceInfo> snapshot, int unitCount, bool forSpanAnalysis, CancellationToken cancellationToken);

0 commit comments

Comments
 (0)