Skip to content

Commit efd47fd

Browse files
sharwellsharwell
committed
Improve DocumentBasedFixAllProvider performance for multiple documents
Previously, custom fix all providers called FixAllContext.GetDocumentDiagnosticsAsync for every document in the fix all scope. This change replaces those calls with a single call to FixAllContextHelpers.GetDocumentDiagnosticsToFixAsync, which was extracted from CustomBatchFixAllProvider. The result is improved performance when fixing violations in large projects, especially when only a few documents in the project contained violations.
1 parent d391039 commit efd47fd

33 files changed

Lines changed: 212 additions & 195 deletions

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1626CodeFixProvider.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ private class FixAll : DocumentBasedFixAllProvider
6868
protected override string CodeActionTitle =>
6969
DocumentationResources.SA1626CodeFix;
7070

71-
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document)
71+
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
7272
{
73-
var diagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
7473
if (diagnostics.IsEmpty)
7574
{
7675
return null;

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/CustomBatchFixAllProvider.cs‎

Lines changed: 4 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -210,93 +210,14 @@ public virtual string GetFixAllTitle(FixAllContext fixAllContext)
210210
}
211211
}
212212

213-
public virtual async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(FixAllContext fixAllContext)
213+
public virtual Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(FixAllContext fixAllContext)
214214
{
215-
var allDiagnostics = ImmutableArray<Diagnostic>.Empty;
216-
var projectsToFix = ImmutableArray<Project>.Empty;
217-
218-
var document = fixAllContext.Document;
219-
var project = fixAllContext.Project;
220-
221-
switch (fixAllContext.Scope)
222-
{
223-
case FixAllScope.Document:
224-
if (document != null)
225-
{
226-
var documentDiagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
227-
return ImmutableDictionary<Document, ImmutableArray<Diagnostic>>.Empty.SetItem(document, documentDiagnostics);
228-
}
229-
230-
break;
231-
232-
case FixAllScope.Project:
233-
projectsToFix = ImmutableArray.Create(project);
234-
allDiagnostics = await fixAllContext.GetAllDiagnosticsAsync(project).ConfigureAwait(false);
235-
break;
236-
237-
case FixAllScope.Solution:
238-
projectsToFix = project.Solution.Projects
239-
.Where(p => p.Language == project.Language)
240-
.ToImmutableArray();
241-
242-
var diagnostics = new ConcurrentDictionary<ProjectId, ImmutableArray<Diagnostic>>();
243-
var tasks = new Task[projectsToFix.Length];
244-
for (int i = 0; i < projectsToFix.Length; i++)
245-
{
246-
fixAllContext.CancellationToken.ThrowIfCancellationRequested();
247-
var projectToFix = projectsToFix[i];
248-
tasks[i] = Task.Run(
249-
async () =>
250-
{
251-
var projectDiagnostics = await fixAllContext.GetAllDiagnosticsAsync(projectToFix).ConfigureAwait(false);
252-
diagnostics.TryAdd(projectToFix.Id, projectDiagnostics);
253-
}, fixAllContext.CancellationToken);
254-
}
255-
256-
await Task.WhenAll(tasks).ConfigureAwait(false);
257-
allDiagnostics = allDiagnostics.AddRange(diagnostics.SelectMany(i => i.Value));
258-
break;
259-
}
260-
261-
if (allDiagnostics.IsEmpty)
262-
{
263-
return ImmutableDictionary<Document, ImmutableArray<Diagnostic>>.Empty;
264-
}
265-
266-
return await GetDocumentDiagnosticsToFixAsync(allDiagnostics, projectsToFix, fixAllContext.CancellationToken).ConfigureAwait(false);
215+
return FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(fixAllContext);
267216
}
268217

269-
public virtual async Task<ImmutableDictionary<Project, ImmutableArray<Diagnostic>>> GetProjectDiagnosticsToFixAsync(FixAllContext fixAllContext)
218+
public virtual Task<ImmutableDictionary<Project, ImmutableArray<Diagnostic>>> GetProjectDiagnosticsToFixAsync(FixAllContext fixAllContext)
270219
{
271-
var project = fixAllContext.Project;
272-
if (project != null)
273-
{
274-
switch (fixAllContext.Scope)
275-
{
276-
case FixAllScope.Project:
277-
var diagnostics = await fixAllContext.GetProjectDiagnosticsAsync(project).ConfigureAwait(false);
278-
return ImmutableDictionary<Project, ImmutableArray<Diagnostic>>.Empty.SetItem(project, diagnostics);
279-
280-
case FixAllScope.Solution:
281-
var projectsAndDiagnostics = new ConcurrentDictionary<Project, ImmutableArray<Diagnostic>>();
282-
var options = new ParallelOptions() { CancellationToken = fixAllContext.CancellationToken };
283-
Parallel.ForEach(project.Solution.Projects, options, proj =>
284-
{
285-
fixAllContext.CancellationToken.ThrowIfCancellationRequested();
286-
var projectDiagnosticsTask = fixAllContext.GetProjectDiagnosticsAsync(proj);
287-
projectDiagnosticsTask.Wait(fixAllContext.CancellationToken);
288-
var projectDiagnostics = projectDiagnosticsTask.Result;
289-
if (projectDiagnostics.Any())
290-
{
291-
projectsAndDiagnostics.TryAdd(proj, projectDiagnostics);
292-
}
293-
});
294-
295-
return projectsAndDiagnostics.ToImmutableDictionary();
296-
}
297-
}
298-
299-
return ImmutableDictionary<Project, ImmutableArray<Diagnostic>>.Empty;
220+
return FixAllContextHelper.GetProjectDiagnosticsToFixAsync(fixAllContext);
300221
}
301222

302223
public virtual async Task<Solution> TryMergeFixesAsync(Solution oldSolution, IEnumerable<CodeAction> codeActions, CancellationToken cancellationToken)
@@ -429,57 +350,6 @@ public virtual async Task<Solution> TryMergeFixesAsync(Solution oldSolution, IEn
429350
return currentSolution;
430351
}
431352

432-
private static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(
433-
ImmutableArray<Diagnostic> diagnostics,
434-
ImmutableArray<Project> projects,
435-
CancellationToken cancellationToken)
436-
{
437-
var treeToDocumentMap = await GetTreeToDocumentMapAsync(projects, cancellationToken).ConfigureAwait(false);
438-
439-
var builder = ImmutableDictionary.CreateBuilder<Document, ImmutableArray<Diagnostic>>();
440-
foreach (var documentAndDiagnostics in diagnostics.GroupBy(d => GetReportedDocument(d, treeToDocumentMap)))
441-
{
442-
cancellationToken.ThrowIfCancellationRequested();
443-
var document = documentAndDiagnostics.Key;
444-
var diagnosticsForDocument = documentAndDiagnostics.ToImmutableArray();
445-
builder.Add(document, diagnosticsForDocument);
446-
}
447-
448-
return builder.ToImmutable();
449-
}
450-
451-
private static async Task<ImmutableDictionary<SyntaxTree, Document>> GetTreeToDocumentMapAsync(ImmutableArray<Project> projects, CancellationToken cancellationToken)
452-
{
453-
var builder = ImmutableDictionary.CreateBuilder<SyntaxTree, Document>();
454-
foreach (var project in projects)
455-
{
456-
cancellationToken.ThrowIfCancellationRequested();
457-
foreach (var document in project.Documents)
458-
{
459-
cancellationToken.ThrowIfCancellationRequested();
460-
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
461-
builder.Add(tree, document);
462-
}
463-
}
464-
465-
return builder.ToImmutable();
466-
}
467-
468-
private static Document GetReportedDocument(Diagnostic diagnostic, ImmutableDictionary<SyntaxTree, Document> treeToDocumentsMap)
469-
{
470-
var tree = diagnostic.Location.SourceTree;
471-
if (tree != null)
472-
{
473-
Document document;
474-
if (treeToDocumentsMap.TryGetValue(tree, out document))
475-
{
476-
return document;
477-
}
478-
}
479-
480-
return null;
481-
}
482-
483353
/// <summary>
484354
/// Try to merge the changes between <paramref name="newDocument"/> and <paramref name="oldDocument"/> into <paramref name="cumulativeChanges"/>.
485355
/// If there is any conflicting change in <paramref name="newDocument"/> with existing <paramref name="cumulativeChanges"/>, then the original <paramref name="cumulativeChanges"/> are returned.

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/DocumentBasedFixAllProvider.cs‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,24 @@ public override Task<CodeAction> GetFixAsync(FixAllContext fixAllContext)
5858
/// </summary>
5959
/// <param name="fixAllContext">The context for the Fix All operation.</param>
6060
/// <param name="document">The document to fix.</param>
61+
/// <param name="diagnostics">The diagnostics to fix in the document.</param>
6162
/// <returns>
6263
/// <para>The new <see cref="SyntaxNode"/> representing the root of the fixed document.</para>
6364
/// <para>-or-</para>
6465
/// <para><see langword="null"/>, if no changes were made to the document.</para>
6566
/// </returns>
66-
protected abstract Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document);
67+
protected abstract Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics);
6768

6869
private async Task<Document> GetDocumentFixesAsync(FixAllContext fixAllContext)
6970
{
70-
var newRoot = await this.FixAllInDocumentAsync(fixAllContext, fixAllContext.Document).ConfigureAwait(false);
71+
var documentDiagnosticsToFix = await FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(fixAllContext).ConfigureAwait(false);
72+
ImmutableArray<Diagnostic> diagnostics;
73+
if (!documentDiagnosticsToFix.TryGetValue(fixAllContext.Document, out diagnostics))
74+
{
75+
return fixAllContext.Document;
76+
}
77+
78+
var newRoot = await this.FixAllInDocumentAsync(fixAllContext, fixAllContext.Document, diagnostics).ConfigureAwait(false);
7179
if (newRoot == null)
7280
{
7381
return fixAllContext.Document;
@@ -78,11 +86,20 @@ private async Task<Document> GetDocumentFixesAsync(FixAllContext fixAllContext)
7886

7987
private async Task<Solution> GetSolutionFixesAsync(FixAllContext fixAllContext, ImmutableArray<Document> documents)
8088
{
89+
var documentDiagnosticsToFix = await FixAllContextHelper.GetDocumentDiagnosticsToFixAsync(fixAllContext).ConfigureAwait(false);
90+
8191
Solution solution = fixAllContext.Solution;
8292
List<Task<SyntaxNode>> newDocuments = new List<Task<SyntaxNode>>(documents.Length);
8393
foreach (var document in documents)
8494
{
85-
newDocuments.Add(this.FixAllInDocumentAsync(fixAllContext, document));
95+
ImmutableArray<Diagnostic> diagnostics;
96+
if (!documentDiagnosticsToFix.TryGetValue(document, out diagnostics))
97+
{
98+
newDocuments.Add(document.GetSyntaxRootAsync(fixAllContext.CancellationToken));
99+
continue;
100+
}
101+
102+
newDocuments.Add(this.FixAllInDocumentAsync(fixAllContext, document, diagnostics));
86103
}
87104

88105
for (int i = 0; i < documents.Length; i++)

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/FixAllContextHelper.cs‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Helpers
5+
{
6+
using System.Collections.Concurrent;
7+
using System.Collections.Immutable;
8+
using System.Linq;
9+
using System.Threading;
10+
using System.Threading.Tasks;
11+
using Microsoft.CodeAnalysis;
12+
using Microsoft.CodeAnalysis.CodeFixes;
13+
14+
internal static class FixAllContextHelper
15+
{
16+
public static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(FixAllContext fixAllContext)
17+
{
18+
var allDiagnostics = ImmutableArray<Diagnostic>.Empty;
19+
var projectsToFix = ImmutableArray<Project>.Empty;
20+
21+
var document = fixAllContext.Document;
22+
var project = fixAllContext.Project;
23+
24+
switch (fixAllContext.Scope)
25+
{
26+
case FixAllScope.Document:
27+
if (document != null)
28+
{
29+
var documentDiagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
30+
return ImmutableDictionary<Document, ImmutableArray<Diagnostic>>.Empty.SetItem(document, documentDiagnostics);
31+
}
32+
33+
break;
34+
35+
case FixAllScope.Project:
36+
projectsToFix = ImmutableArray.Create(project);
37+
allDiagnostics = await fixAllContext.GetAllDiagnosticsAsync(project).ConfigureAwait(false);
38+
break;
39+
40+
case FixAllScope.Solution:
41+
projectsToFix = project.Solution.Projects
42+
.Where(p => p.Language == project.Language)
43+
.ToImmutableArray();
44+
45+
var diagnostics = new ConcurrentDictionary<ProjectId, ImmutableArray<Diagnostic>>();
46+
var tasks = new Task[projectsToFix.Length];
47+
for (int i = 0; i < projectsToFix.Length; i++)
48+
{
49+
fixAllContext.CancellationToken.ThrowIfCancellationRequested();
50+
var projectToFix = projectsToFix[i];
51+
tasks[i] = Task.Run(
52+
async () =>
53+
{
54+
var projectDiagnostics = await fixAllContext.GetAllDiagnosticsAsync(projectToFix).ConfigureAwait(false);
55+
diagnostics.TryAdd(projectToFix.Id, projectDiagnostics);
56+
}, fixAllContext.CancellationToken);
57+
}
58+
59+
await Task.WhenAll(tasks).ConfigureAwait(false);
60+
allDiagnostics = allDiagnostics.AddRange(diagnostics.SelectMany(i => i.Value));
61+
break;
62+
}
63+
64+
if (allDiagnostics.IsEmpty)
65+
{
66+
return ImmutableDictionary<Document, ImmutableArray<Diagnostic>>.Empty;
67+
}
68+
69+
return await GetDocumentDiagnosticsToFixAsync(allDiagnostics, projectsToFix, fixAllContext.CancellationToken).ConfigureAwait(false);
70+
}
71+
72+
public static async Task<ImmutableDictionary<Project, ImmutableArray<Diagnostic>>> GetProjectDiagnosticsToFixAsync(FixAllContext fixAllContext)
73+
{
74+
var project = fixAllContext.Project;
75+
if (project != null)
76+
{
77+
switch (fixAllContext.Scope)
78+
{
79+
case FixAllScope.Project:
80+
var diagnostics = await fixAllContext.GetProjectDiagnosticsAsync(project).ConfigureAwait(false);
81+
return ImmutableDictionary<Project, ImmutableArray<Diagnostic>>.Empty.SetItem(project, diagnostics);
82+
83+
case FixAllScope.Solution:
84+
var projectsAndDiagnostics = new ConcurrentDictionary<Project, ImmutableArray<Diagnostic>>();
85+
var options = new ParallelOptions() { CancellationToken = fixAllContext.CancellationToken };
86+
Parallel.ForEach(project.Solution.Projects, options, proj =>
87+
{
88+
fixAllContext.CancellationToken.ThrowIfCancellationRequested();
89+
var projectDiagnosticsTask = fixAllContext.GetProjectDiagnosticsAsync(proj);
90+
projectDiagnosticsTask.Wait(fixAllContext.CancellationToken);
91+
var projectDiagnostics = projectDiagnosticsTask.Result;
92+
if (projectDiagnostics.Any())
93+
{
94+
projectsAndDiagnostics.TryAdd(proj, projectDiagnostics);
95+
}
96+
});
97+
98+
return projectsAndDiagnostics.ToImmutableDictionary();
99+
}
100+
}
101+
102+
return ImmutableDictionary<Project, ImmutableArray<Diagnostic>>.Empty;
103+
}
104+
105+
private static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(
106+
ImmutableArray<Diagnostic> diagnostics,
107+
ImmutableArray<Project> projects,
108+
CancellationToken cancellationToken)
109+
{
110+
var treeToDocumentMap = await GetTreeToDocumentMapAsync(projects, cancellationToken).ConfigureAwait(false);
111+
112+
var builder = ImmutableDictionary.CreateBuilder<Document, ImmutableArray<Diagnostic>>();
113+
foreach (var documentAndDiagnostics in diagnostics.GroupBy(d => GetReportedDocument(d, treeToDocumentMap)))
114+
{
115+
cancellationToken.ThrowIfCancellationRequested();
116+
var document = documentAndDiagnostics.Key;
117+
var diagnosticsForDocument = documentAndDiagnostics.ToImmutableArray();
118+
builder.Add(document, diagnosticsForDocument);
119+
}
120+
121+
return builder.ToImmutable();
122+
}
123+
124+
private static async Task<ImmutableDictionary<SyntaxTree, Document>> GetTreeToDocumentMapAsync(ImmutableArray<Project> projects, CancellationToken cancellationToken)
125+
{
126+
var builder = ImmutableDictionary.CreateBuilder<SyntaxTree, Document>();
127+
foreach (var project in projects)
128+
{
129+
cancellationToken.ThrowIfCancellationRequested();
130+
foreach (var document in project.Documents)
131+
{
132+
cancellationToken.ThrowIfCancellationRequested();
133+
var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
134+
builder.Add(tree, document);
135+
}
136+
}
137+
138+
return builder.ToImmutable();
139+
}
140+
141+
private static Document GetReportedDocument(Diagnostic diagnostic, ImmutableDictionary<SyntaxTree, Document> treeToDocumentsMap)
142+
{
143+
var tree = diagnostic.Location.SourceTree;
144+
if (tree != null)
145+
{
146+
Document document;
147+
if (treeToDocumentsMap.TryGetValue(tree, out document))
148+
{
149+
return document;
150+
}
151+
}
152+
153+
return null;
154+
}
155+
}
156+
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1500CodeFixProvider.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,8 @@ private class FixAll : DocumentBasedFixAllProvider
257257
protected override string CodeActionTitle =>
258258
LayoutResources.SA1500CodeFix;
259259

260-
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document)
260+
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
261261
{
262-
var diagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
263262
if (diagnostics.IsEmpty)
264263
{
265264
return null;

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1503CodeFixProvider.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,8 @@ private class FixAll : DocumentBasedFixAllProvider
9494
protected override string CodeActionTitle =>
9595
LayoutResources.SA1503CodeFix;
9696

97-
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document)
97+
protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
9898
{
99-
var diagnostics = await fixAllContext.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
10099
if (diagnostics.IsEmpty)
101100
{
102101
return null;

0 commit comments

Comments
 (0)