-
Notifications
You must be signed in to change notification settings - Fork 229
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
UtilityAnalyzer: Use RegisterCompilationStartAction #6576
UtilityAnalyzer: Use RegisterCompilationStartAction #6576
Conversation
cc @pavel-mikula-sonarsource as this may conflict with your AnalyzerContext refactoring. |
Hi @martin-strecker-sonarsource. What was the observed change in analysis time? |
Here is a comparison of an AnalyzerRunner.exe run for a single UtilityAnalyzer:
Current master:
This PR
|
For all utility analyzers
Master
PR
With EnableConcurrentExecution = true
|
Excellent. Thanks looks awesome! |
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/FileMetadataAnalyzerTest.cs
Show resolved
Hide resolved
7402be7
to
d442636
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
f6ca28a
to
75f8a25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the code, without reviewing the UTs yet
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSematicModelReportingContext.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSematicModelReportingContext.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarCompilationStartAnalysisContext.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarAnalysisContextBase.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SonarAnalysisContextBase changes have to be reverted
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/UtilityAnalyzerBaseTest.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be doable with ReadParameters(SonarCompilationStartAnalysisContext)
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/AnalysisContext/SonarSematicModelReportingContextTest.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/UtilityAnalyzerBaseTest.cs
Outdated
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/UtilityAnalyzerBaseTest.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final polishing
.../SonarAnalyzer.UnitTest/AnalysisContext/SonarCompilationStartAnalysisContextTest.Register.cs
Outdated
Show resolved
Hide resolved
.../SonarAnalyzer.UnitTest/AnalysisContext/SonarCompilationStartAnalysisContextTest.Register.cs
Outdated
Show resolved
Hide resolved
3ac7fbe
to
dacee9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed an issue with inheritnace. Should be easy to fix.
analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/AnalysisContext/SonarAnalysisContextTest.Register.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/AnalysisContext/SonarAnalysisContextTest.Register.cs
Outdated
Show resolved
Hide resolved
analyzers/tests/SonarAnalyzer.UnitTest/AnalysisContext/SonarAnalysisContextTest.Register.cs
Show resolved
Hide resolved
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSematicModelReportingContext.cs
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
On Akka.Net, I did not see any change in performance, see the semantic models are most likely cached as they are queried by different rules of the same analysis:
Before: 67. 65. 67
After: 62, 66, 69
Fixes #6558