Skip to content

AnalyzerDriver performance improvements#45537

Merged
mavasani merged 3 commits intodotnet:masterfrom
mavasani:SymbolStartPerf2
Jul 9, 2020
Merged

AnalyzerDriver performance improvements#45537
mavasani merged 3 commits intodotnet:masterfrom
mavasani:SymbolStartPerf2

Conversation

@mavasani
Copy link
Contributor

@mavasani mavasani commented Jun 29, 2020

  1. Avoid creating and querying dictionaries from analyzers to actions. Instead maintain a tuple of (analyzer, actions) and special case IDE partial analysis (open files only and/or subset of analyzers) to skip analyzers not in analysis scope.
  2. Cache grouped analyzer actions for per-symbol actions registered within SymbolStart actions instead of re-computing them for every member symbol.

Performance numbers

Around 10-15% improvement in analyzer execution time on following test sets:

1. Compiler benchmark

Benchmark: https://github.com/dotnet/roslyn/blob/master/docs/wiki/Measuring-Compiler-Performance.md

Using compiler built from current master branch

  1. Default benchmark:
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
GetDiagnosticsWithAnalyzers 14.79 s 0.147 s 0.304 s 14.87 s 14.17 s 15.45 s 218000.0000 64000.0000 3000.0000 1.28 GB
  1. Default benchmark + one SymbolStart analyzer:
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
GetDiagnosticsWithAnalyzers 16.96 s 0.079 s 0.088 s 16.95 s 16.85 s 17.14 s 225000.0000 66000.0000 2000.0000 1.32 GB

Using compiler built with this PR branch

  1. Default benchmark:
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
GetDiagnosticsWithAnalyzers 12.76 s 0.077 s 0.085 s 12.75 s 12.62 s 12.92 s 218000.0000 60000.0000 2000.0000 1.28 GB
  1. Default benchmark + one SymbolStart analyzer:
Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Gen 2 Allocated
GetDiagnosticsWithAnalyzers 14.87 s 0.121 s 0.101 s 14.90 s 14.72 s 15.08 s 220000.0000 65000.0000 2000.0000 1.29 GB

2. Building real world projects

  1. Microsoft.CodeAnalysis.CSharp.csproj: msbuild /v:m /m /t:rebuild /p:UseRoslynAnalyzers=true: Improvement from ~60 seconds to ~51 seconds
  2. RoslynAnalyzers.sln: Improvement from ~55 seconds to ~48 seconds

@mavasani mavasani added this to the 16.8 milestone Jun 29, 2020
mavasani
mavasani commented Jun 29, 2020
View reviewed changes
...ompilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.GroupedAnalyzerActionsByAnalyzer.cs Outdated
Copy link
Contributor Author

@mavasani mavasani Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the code here is just simplified and moved from GroupedAnalyzerActions, but avoids the dictionaries keyed on analyzers.

mavasani
mavasani commented Jun 29, 2020
View reviewed changes
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs Outdated
Comment on lines 72 to 87
Copy link
Contributor Author

@mavasani mavasani Jun 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the key changes in this PR mentioned in the description:

  1. Replace dictionaries from analyzer to actions with an array of tuples (analyzer, actions) and avoid dictionary TryGetValue calls, which showed up quite a bit on performance profiles.
  2. Change _perSymbolAnalyzerActionsCache to cache IGroupedAnalyzerActions instead of AnalyzerActions. Previously, we used to recompute GroupedAnalyzerActions for the same AnalyzerActions shared between all member symbols of the INamespaceOrTypeSymbol which is part of the key for the dictionary _perSymbolAnalyzerActionsCache.

mavasani
mavasani commented Jun 29, 2020
View reviewed changes
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs Outdated
Copy link
Contributor Author

@mavasani mavasani Jun 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes shows an example algorithmic change in execution model. Instead of iterating over all analyzers in the given AnalysisScope and then fetching each analyzers' registered actions from the dictionary, we iterate over the array of (analyzer, actions) and execute all relevant analyzers. For command line build, analysis scope is always the full scope and does not need any additional checks. For IDE invocation, checking if an analyzer belongs to the given analysis scope is a cheap lookup.

@mavasani mavasani force-pushed the SymbolStartPerf2 branch 3 times, most recently from 56ff307 to 255b074 Compare June 30, 2020 00:27
@mavasani
AnalyzerDriver performance improvements
b2fae38 
1. Avoid creating and querying dictionaries from analyzers to actions. Instead maintain a tuple of (analyzer, actions) and special case IDE partial analysis scope to skip analyzers not in analysis scope.
2. Cache grouped analyzer actions for per-symbol actions registered within SymbolStart actions instead of re-computing them for every member symbol.

Benchmark: https://github.com/dotnet/roslyn/blob/master/docs/wiki/Measuring-Compiler-Performance.md

Default benchmark:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 14.79 s | 0.147 s | 0.304 s | 14.87 s | 14.17 s | 15.45 s | 218000.0000 | 64000.0000 | 3000.0000 |   1.28 GB |

Default benchmark + one SymbolStart analyzer:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 16.96 s | 0.079 s | 0.088 s | 16.95 s | 16.85 s | 17.14 s | 225000.0000 | 66000.0000 | 2000.0000 |   1.32 GB |

Default benchmark:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 12.76 s | 0.077 s | 0.085 s | 12.75 s | 12.62 s | 12.92 s | 218000.0000 | 60000.0000 | 2000.0000 |   1.28 GB |

Default benchmark + one SymbolStart analyzer:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 14.87 s | 0.121 s | 0.101 s | 14.90 s | 14.72 s | 15.08 s | 220000.0000 | 65000.0000 | 2000.0000 |   1.29 GB |

1. **Microsoft.CodeAnalysis.CSharp.csproj**: `msbuild /v:m /m /t:rebuild /p:UseRoslynAnalyzers=true`: Improvement from _~60 seconds_ to _~51 seconds_
2. **RoslynAnalyzers.sln**:  Improvement from _~55 seconds_ to _~48 seconds_
AnalyzerDriver performance improvements

1. Avoid creating and querying dictionaries from analyzers to actions. Instead maintain a tuple of (analyzer, actions) and special case IDE partial analysis scope to skip analyzers not in analysis scope.
2. Cache grouped analyzer actions for per-symbol actions registered within SymbolStart actions instead of re-computing them for every member symbol.

Benchmark: https://github.com/dotnet/roslyn/blob/master/docs/wiki/Measuring-Compiler-Performance.md

Default benchmark:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 14.79 s | 0.147 s | 0.304 s | 14.87 s | 14.17 s | 15.45 s | 218000.0000 | 64000.0000 | 3000.0000 |   1.28 GB |

Default benchmark + one SymbolStart analyzer:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 16.96 s | 0.079 s | 0.088 s | 16.95 s | 16.85 s | 17.14 s | 225000.0000 | 66000.0000 | 2000.0000 |   1.32 GB |

Default benchmark:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 12.76 s | 0.077 s | 0.085 s | 12.75 s | 12.62 s | 12.92 s | 218000.0000 | 60000.0000 | 2000.0000 |   1.28 GB |

Default benchmark + one SymbolStart analyzer:
|                      Method |    Mean |   Error |  StdDev |  Median |     Min |     Max |       Gen 0 |      Gen 1 |     Gen 2 | Allocated |
|---------------------------- |--------:|--------:|--------:|--------:|--------:|--------:|------------:|-----------:|----------:|----------:|
| GetDiagnosticsWithAnalyzers | 14.87 s | 0.121 s | 0.101 s | 14.90 s | 14.72 s | 15.08 s | 220000.0000 | 65000.0000 | 2000.0000 |   1.29 GB |

1. **Microsoft.CodeAnalysis.Workspaces.csproj**: `msbuild /v:m /m /t:rebuild /p:UseRoslynAnalyzers=true`
2. **Compilers.sln**: `msbuild /v:m /m /t:rebuild /p:UseRoslynAnalyzers=true Compilers.sln`
@mavasani mavasani marked this pull request as ready for review June 30, 2020 14:43
@mavasani mavasani requested a review from a team as a code owner June 30, 2020 14:43
@mavasani mavasani requested review from a team and sharwell June 30, 2020 14:43
@mavasani
Copy link
Contributor Author

mavasani commented Jul 2, 2020

Ping @dotnet/roslyn-compiler for reviews

1 similar comment
@mavasani
Copy link
Contributor Author

mavasani commented Jul 6, 2020

Ping @dotnet/roslyn-compiler for reviews

cston
cston reviewed Jul 6, 2020
View reviewed changes
@mavasani
Copy link
Contributor Author

mavasani commented Jul 8, 2020

@cston Any further feedback? @gafter for additional review. Thanks.

genlu
genlu reviewed Jul 8, 2020
View reviewed changes
genlu
genlu reviewed Jul 8, 2020
View reviewed changes
cston
cston reviewed Jul 9, 2020
View reviewed changes
cston
cston reviewed Jul 9, 2020
View reviewed changes
cston
cston reviewed Jul 9, 2020
View reviewed changes
cston
cston reviewed Jul 9, 2020
View reviewed changes
@mavasani
Copy link
Contributor Author

mavasani commented Jul 9, 2020

Thanks!

@mavasani mavasani merged commit e2be823 into dotnet:master Jul 9, 2020
@ghost ghost modified the milestones: 16.8, Next Jul 9, 2020
@mavasani mavasani deleted the SymbolStartPerf2 branch July 9, 2020 23:58
@JoeRobich JoeRobich modified the milestones: Next, 16.8.P1 Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genlu genlu genlu approved these changes

@sharwell sharwell Awaiting requested review from sharwell

+2 more reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Analyzers Area-Compilers

Projects

None yet

Milestone

16.8.P1

Development

Successfully merging this pull request may close these issues.

5 participants

@mavasani @genlu @CyrusNajmabadi @cston @JoeRobich