Improve DocumentBasedFixAllProvider performance for multiple documents #1983
Conversation
Current coverage is
|
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.
sharwell
force-pushed
the
fixall-performance
branch
from
a532710 to
6524035
Compare
Also updated FixAllContextHelper to support code fixes for standard C# compiler warnings.
sharwell
force-pushed
the
fixall-performance
branch
from
6524035 to
04c27a3
Compare
| if (analyzers.Any()) | ||
| { | ||
| var compilationWithAnalyzers = compilation.WithAnalyzers(analyzers, project.AnalyzerOptions, fixAllContext.CancellationToken); | ||
| diagnostics = await compilationWithAnalyzers.GetAnalyzerDiagnosticsAsync().ConfigureAwait(false); |
There was a problem hiding this comment.
@mavasani I'm finding that when I use CompilationWithAnalyzers here along with #1979, even though all diagnostics are reported in the errors pane in Visual Studio not all diagnostics end up corrected by the Fix All in Project and Fix All in Solution operations. Is there some way I can update this method so we have the performance of CompilationWithAnalyzers.GetAnalyzerDiagnosticsAsync, but also get all diagnostics?
There was a problem hiding this comment.
GetAnalyzerDiagnostics seems to be affected by the same bug you pointed out in your churn testing. You may try to replicate what IDE diagnostic service does sequentially - for each document invoke GetAnalyzerSyntaxDiagnosticsAsync and GetAnalyzerSemanticDiagnosticsAsync. If you also have any compilation end analyzers, then also invoke GetAnalyzerCompilationDiagnosticsAsync at the end. This won't be as fast as GetAnalyzerDiagnosticsAsync() but should be faster then current performance of FixAllContext.GetAllDiagnosticsAsync.
sharwell
force-pushed
the
fixall-performance
branch
from
cf7422a to
39948d9
Compare
This commit also moves back to using FixAllContext.GetAllDiagnosticsAsync for non-1.1 versions of Roslyn, where performance of the operation wasn't causing a major problem.
|
With the latest change, the churn tests indicate that aside from true duplicates in 1.0.0, all three tested versions of Roslyn produce identical outputs. |
Closes DotNetAnalyzers#1991 Closes DotNetAnalyzers#1992 Closes DotNetAnalyzers#1993 Closes DotNetAnalyzers#1994 Closes DotNetAnalyzers#1995 Closes DotNetAnalyzers#1996 Closes DotNetAnalyzers#1997
|
|
||
| // Note that the following loop to obtain syntax and semantic diagnostics for each document cannot operate | ||
| // on parallel due to our use of a single CompilationWithAnalyzers instance. | ||
| var diagnostics = ImmutableArray<Diagnostic>.Empty; |
There was a problem hiding this comment.
💭 using an ImmutableArray<Diagnostic>.Builder or a List<Diagnostic> would improve the performance when adding diagnostics. There is no need to have the diagnostics in the ImmutableArray<Diagnostic> type yet, as they are never passed to another method.
|
👍 Looks good to me, with two small remarks. |
|
👍 |
Improve DocumentBasedFixAllProvider performance for multiple documents
Previously, custom fix all providers called
FixAllContext.GetDocumentDiagnosticsAsyncfor every document in the fix all scope. This change first replaces those calls with a single call toFixAllContextHelpers.GetDocumentDiagnosticsToFixAsync, which was extracted fromCustomBatchFixAllProvider. It goes on to instead operate on aCompilationWithAnalyzersfollowing the commentary in #1979. The result is improved performance when fixing violations in large projects, especially when only a few documents in the project contained violations.Fixes #1974.