Further reduce allocations in SemanticToken calculations #12206
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of realizing ImmutableArrays in several codepaths, we can instead just have the code append the new items to an existing list. Also, in ConvertSemanticRangesToSemanticTokensData, switches to optimistically allocating an int[] instead of a pooled list for two reasons: 1) In almost all cases, the correct size is calculated before population 2) The pooled list that was being created ended up being too large to be placed back in the pool, so we ended up losing both that list and it's backing array to GC.
davidwengier
approved these changes
Sep 9, 2025
Member
davidwengier
left a comment
davidwengier
left a comment
There was a problem hiding this comment.
Other than the build errors and some naming tweaks, this makes sense to me.
...ks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorSemanticTokensBenchmark.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/SemanticTokensVisitor.cs
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Test insertion run from commit 4 didn't have expected gains. Looks like the test is using a larger page than I was testing against. Switched to a custom pool which should work better against larger sematic token request. Test insertion queued for new numbers. |
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/SemanticTokensVisitor.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
...osoft.CodeAnalysis.Razor.Workspaces/SemanticTokens/AbstractRazorSemanticTokensInfoService.cs
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@DustinCampbell -- I think I liked every single suggestion you made! Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of realizing ImmutableArrays in several codepaths, we can instead just have the calculations append items to an existing list.
Also, in ConvertSemanticRangesToSemanticTokensData, switches to optimistically allocating an int[] instead of a pooled list for two reasons:
This wasn't a huge memory hitter for me in the profile I'm looking at, but still is ~0.6% of allocations in the RoslynCodeAnalysisService process during the C#/html completions in the razor cohosting speedometer test.
Test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/669091
Numbers looked good in the test insertion: SemanticRange[] allocations pretty much gone, and the int[] allocations in ConvertSemanticRangesToSemanticTokensData are down significantly too.