-
Notifications
You must be signed in to change notification settings - Fork 217
Compiler + Tooling: Span visitor improvements #11966
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
Merged
DustinCampbell
merged 14 commits into
dotnet:main
from
DustinCampbell:improve-span-visitors
Jun 24, 2025
Merged
Compiler + Tooling: Span visitor improvements #11966
DustinCampbell
merged 14 commits into
dotnet:main
from
DustinCampbell:improve-span-visitors
Jun 24, 2025
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
Provides additional overloads for the `SyntaxToken` factory methods in `SyntaxFactory`. These new overloads allow specifying the parent syntax node, position, and index, offering greater flexibility when creating syntax tokens. In addition, use one of the new SyntaxFactory.Token overloads in ClassifiedSpanVisitor rather than constructor a SyntaxToken directly.
Refactors ClassifiedSpanVisitor to use a `BlockSaver` ref struct to manage block scopes, improving readability and reducing code duplication. Removes unnecessary base visit action delegates.
Refactors span writing logic in the `ClassifiedSpanVisitor` to improve readability and eliminate redundant null checks. Retrieves `AcceptedCharacters` directly within the method, using the node's edit handler or defaulting to `Any`. Also adds an assertion to validate current block during span writing.
Changes span creation to only compute the current block's span when it is first needed. This avoids redundant computations, improving performance.
Uses a `SourceSpanComputer` to efficiently create markup spans for attributes, avoiding unnecessary string allocations and improving performance of the `ClassifiedSpanVisitor`.
Moves the `ClassifiedSpanVisitor` to the `Legacy` namespace since it is only accessed by code in that namespace. This change is part of an effort to better organize the Razor compiler's internal structure. It specifically isolates legacy code.
Replaces the pooled `ImmutableArray.Builder` with a pooled `ClassifiedSpanVisitor` to avoid allocating a new visitor instance each time. The `ImmutableArray.Builder` is still effectively pooled, since it is owned by `ClassifiedSpanVisitor`. This change reduces overall memory allocations and improves performance.
Replaces explicit null checks with `ArgHelper.ThrowIfNull` in `RazorSyntaxTreeExtensions` for conciseness.
Moves the `TagHelperSpanVisitor` to the `Legacy` namespace since it is only accessed by code in that namespace. This change is part of an effort to better organize the Razor compiler's internal structure. It specifically isolates legacy code.
Refactors formatting visitor to use a ref struct SpanComputer for generating text spans, improving efficiency. Consolidates span creation logic and simplifies block management with a BlockSaver struct for cleaner code.
This method is no longer used outside of SyntaxUtilities.
Improves performance by utilizing a pooled array builder to avoid allocations when collecting formatting spans. This change reduces memory pressure and improves the efficiency of the formatting process.
Utilizes a pooled array builder when computing tag helper spans, reducing memory allocations. Avoids calling into the legacy `GetTagHelperSpans` method, further improving performance by avoiding allocations of `TagHelperSpanVisitor`. In addition, returns `SourceSpans` rather than `TagHelperSpanInternal` since tooling doesn't use the tag helper binding info.
Refactors the way classified spans are computed for Razor code documents intooling. This change replaces the usage of legacy compiler APIs with a new `ClassifiedSpanVisitor`. The new implementation is streamlined to only produce the information needed for the `GetLanguageKind()` API, improving performance and reducing dependencies. This also includes changing the `ClassifiedSpanInternal` to `ClassifiedSpan` and removing the dependency on `Microsoft.AspNetCore.Razor.Language.Legacy`.
davidwengier
approved these changes
Jun 22, 2025
Member
davidwengier
left a comment
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!
ToddGrun
approved these changes
Jun 23, 2025
Contributor
ToddGrun
left a comment
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.
![]()
chsienki
approved these changes
Jun 23, 2025
Member
Author
|
Pingubg @dotnet/razor-compiler for a second review. |
333fred
approved these changes
Jun 24, 2025
This was referenced Jun 26, 2025
Closed
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 represents several changes to improve efficiency and reduce allocations in the
ClassifiedSpanVisitor,TagHelperSpanVisitor, andFormattingVisitor.ClassifiedSpanVisitorinstances to reduce allocations.Legacynamespace, isolating it from current implementations.ClassifiedSpanVisitorandFormattingVisitor.SpanComputerhelper to avoid creating syntax nodes just to visit them inClassifiedSpanVisitorandFormattingVisitor.GetTagHelpersandGetClassifiedSpansin tooling. These only produce the data needed to support tooling's GetLanguageKind.CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2734526&view=results
Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/645479
Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2734527&view=results