Compiler: Replace SyntaxListBuilder with PooledArrayBuilder<T> and miscellaneous performance tweaks #11841
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
Add a handful of extension methods for PooledArrayBuidler<TNode> that facilitate constructing green syntax list nodes and red syntax lists.
This change includes fairly substantial refactoring of the methods on SyntaxUtilities, and changes all methods to use PooledArrayBuilder<T> rather than other collections.
This change updates the syntax API for Razor to add two abstract types: - BaseMarkupStartTagSyntax - inherited by MarkupStartTagSyntax and MarkupTagHelperStartTagSyntax - BaseMarkupEndTagSyntax - inherited by by MarkupEndTagSyntax and MarkupTagHelperEndTagSyntax The IStartTagSyntaxNode and IEndTagSyntaxNode interfaces have been removed.
GreenNode.WriteTo(...) performs three loops for each node: 1. Find the first non-null slot index. 2. Find the last non-null slot index. 3. Loop from the last non-null slot index to the first non-null slow index and push non-null children. These loops don't really perform any additional work, but the extra complexity doesn't add any value either.
Introduce SyntaxNode.DescendantTokens(...) to avoid building a string for each attribute's content just to check string.IsNullOrWhitespace.
😍😍😍 |
davidwengier
approved these changes
May 8, 2025
| @@ -42,7 +41,7 @@ public async Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorC | |||
| // We also check for tag helper start tags here, because an invalid start tag with a valid tag helper | |||
| // anywhere in it would otherwise not match. We rely on the IsTagUnknown method below, to ensure we | |||
| // only offer on actual potential component tags (because it checks for the compiler diagnostic) | |||
| var startTag = (IStartTagSyntaxNode?)node.FirstAncestorOrSelf<SyntaxNode>(n => n is IStartTagSyntaxNode); | |||
| var startTag = node.FirstAncestorOrSelf<BaseMarkupStartTagSyntax>(); | |||
There was a problem hiding this comment.
Thank you! This is literally the dream I had when I created that interface. If only I'd known how, and was brave enough, to use the syntax generator :)
chsienki
approved these changes
May 8, 2025
333fred
approved these changes
May 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change is primarily about removing the
Syntax.SyntaxListBuilderandSyntax.SyntaxListBuilder<T>from the compiler and replacing usages withPooledArrayBuilder<T>. This allows the compiler to use stack memory for small lists and only acquire pooled heap memory for larger lists.In addition, I've made several other changes and some small refactorings along the way. In particular, I've introduced
BaseMarkupStartTagSyntaxandBaseMarkupEndTagSyntaxtypes to share code between the normal markup tag syntax types and their tag helper variants.CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2704937&view=results
Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/635185
Toolset Test Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2704938&view=results#SourceCard