Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta analyzer to flag use of Compilation.GetSemanticModel inside diagnostic analyzers #3114

Closed
mavasani opened this issue Dec 17, 2019 · 5 comments · Fixed by #3163
Closed
Labels
Area-Microsoft.CodeAnalysis.Analyzers Feature Request help wanted The issue is up-for-grabs, and can be claimed by commenting

Comments

@mavasani
Copy link
Member

mavasani commented Dec 17, 2019

We have seen tons of performance issues in sub-optimally written analyzers that end up creating completely new semantic models by invoking Compilation.GetSemanticModel. This leads the compiler to rebind the entire file, and doing it multiple times (say with SyntaxNode callbacks) can be a performance hog. For example, #2576 identified this can lead a 4x-5x slowdown. With nullability flow analysis in place inside the compiler, this can be even worse.

We should write an analyzer that flags use of this API inside a diagnostic analyzer. Implementation of this analyzer should potentially be a sub-type of DiagnosticAnalyzerCorrectnessAnalyzer.

@mavasani mavasani added Feature Request Area-Microsoft.CodeAnalysis.Analyzers help wanted The issue is up-for-grabs, and can be claimed by commenting labels Dec 17, 2019
@mavasani
Copy link
Member Author

Tagging @Evangelink

@mavasani mavasani changed the title Meta analyzer to flag use Compilation.GetSemanticModel inside diagnostic analyzers Meta analyzer to flag use of Compilation.GetSemanticModel inside diagnostic analyzers Dec 17, 2019
@kzu
Copy link

kzu commented Aug 14, 2020

So, what is the replacement for an analyzer that actually does need to access symbol info to report accurate diagnostic?

@mavasani
Copy link
Member Author

Diagnostic description in error list, when expanded, should give the info:

<value>'GetSemanticModel' is an expensive method to invoke within a diagnostic analyzer because it creates a completely new semantic model, which does not share compilation data with the compiler or other analyzers. This incurs an additional performance cost during semantic analysis. Instead, consider registering a different analyzer action which allows used of a shared 'SemanticModel', such as 'RegisterOperationAction', 'RegisterSyntaxNodeAction', or 'RegisterSemanticModelAction'.</value>

@mavasani
Copy link
Member Author

In general, it is always better performance wise to switch to RegisterOperationAction for semantic analysis of executable code and RegisterSymbolAction for semantic analysis of symbols and non-executable parts of the code.

@kzu
Copy link

kzu commented Aug 16, 2020

Got it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Microsoft.CodeAnalysis.Analyzers Feature Request help wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants