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.

Sign up for GitHub

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

Jump to bottom

Cache RegisterNodeAction checks result per SyntaxTree #8414

Merged
merged 6 commits into from
Nov 30, 2023
Merged

Cache RegisterNodeAction checks result per SyntaxTree #8414

merged 6 commits into from
Nov 30, 2023

Conversation

sebastien-marichal
Copy link
Contributor

@sebastien-marichal sebastien-marichal commented Nov 27, 2023 •
edited
Loading

Fixes #8406

Cache the result of ShouldAnalyzeTree and other checks per SyntaxTree in a dictionary.

Time spent in RegisterNodeAction

Before:
image

After:
image

Allocations

Allocation is going down from 13'633 MB to 11'838 MB overall.
Most of it is saved in RegisterNodeAction:

Before:
image

After:
image

@sebastien-marichal sebastien-marichal changed the title Cache RegisterNodeAction checks result Cache RegisterNodeAction checks result per SyntaxTree Nov 28, 2023
@sebastien-marichal sebastien-marichal marked this pull request as ready for review November 28, 2023 14:47
This was referenced Nov 28, 2023
Closed
Closed
martin-strecker-sonarsource
martin-strecker-sonarsource reviewed Nov 28, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Please repeat the analysis, so we can see the "new" allocations.

analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarCompilationStartAnalysisContext.cs Show resolved Hide resolved
analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarCompilationStartAnalysisContext.cs Outdated Show resolved Hide resolved
martin-strecker-sonarsource
martin-strecker-sonarsource requested changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

.. also there is one code smell

@sebastien-marichal
Copy link
Contributor Author

Allocations within RegisterNodeAction increased after avoiding creating capture classes with GetOrAdd:
image

But the overall allocation is better, down to 11'404 MB.

Build time is the same:

With Capture

  • Run 1: 00:00:33.82
  • Run 2: 00:00:33.43
  • Run 3: 00:00:33.38

Without Capture

  • Run 1: 00:00:33.25
  • Run 2: 00:00:33.93
  • Run 3: 00:00:34.09

@sebastien-marichal sebastien-marichal mentioned this pull request Nov 29, 2023
Closed
@sebastien-marichal sebastien-marichal mentioned this pull request Nov 29, 2023
Open
martin-strecker-sonarsource
martin-strecker-sonarsource approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 30, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 30, 2023 •
edited
Loading

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@martin-strecker-sonarsource martin-strecker-sonarsource merged commit 0c16385 into master Nov 30, 2023
24 checks passed
@martin-strecker-sonarsource martin-strecker-sonarsource deleted the sebastien/8406-dictionary branch November 30, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martin-strecker-sonarsource martin-strecker-sonarsource martin-strecker-sonarsource approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cache RegisterNodeAction checks result per SyntaxTree
2 participants
@sebastien-marichal @martin-strecker-sonarsource