Clone the IR tree so we don't have to re-lower from syntax. - #84715
Conversation
Add an internal IntermediateNode.Clone() that returns a deep copy of a node and its descendants, with a CopyCommonTo helper on the base type that copies the common state (source span, imported flag, diagnostics) and deep-clones children. Every intermediate-node kind that can appear in an unresolved tree overrides it; a node whose children live outside the Children collection (a side reference such as an attribute's name expression) deep-clones those explicitly. The base method throws so an unhandled kind fails loudly rather than silently producing an incomplete copy. Nothing calls Clone() yet; a following change resolves the tag-helper pipeline from a clone. IntermediateNodeCloneTest lowers representative documents to their unresolved form, clones the tree, and asserts a full structural and data dump of the clone matches the original, so an omitted field is caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c45f67d5-6fe0-4f6b-8c68-c268dc7c0621
When a project-wide tag-helper set changes, cached consumer documents must be re-resolved against the new set. Resolution binds unresolved intermediate nodes in place, so it needs a fresh unresolved tree. Reconstructing one by re-lowering from the syntax tree re-runs lowering, classification and the markup split for every replayed document. Capture a pristine, unresolved snapshot of the intermediate tree at the post-split discovery gate, store it on the code document, and resolve both the initial tag-helper pass and every replay over a fresh clone of it. The stored snapshot is never handed to resolution, so it stays valid across replays. A node kind missing a Clone() override throws rather than silently dropping content. Eliminates re-lowering on the replay path (108x to 1x per edit in the consumer-heavy benchmark): roughly 30% less CPU and 31% less allocation in the replay phase, with generated output unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c45f67d5-6fe0-4f6b-8c68-c268dc7c0621
…marks Razor_Edit_Independent replaces the leaf file's body with markup that omits its @page directive, dropping the route from the component's declaration. That is a public-surface change: it alters the decl emitted into the pre-compilation compilation and so invalidates whole-compilation tag-helper discovery. It measures a signature edit, not the common case of editing a leaf's body. Add Razor_Edit_IndependentIgnorable, which preserves @page and changes only the markup, so the declaration stays byte-identical and discovery is skipped -- the impl-only counterpart to Razor_Edit_DependentIgnorable. Comment the two content constants to explain the distinction and add a matching test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c45f67d5-6fe0-4f6b-8c68-c268dc7c0621
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR adds deep-cloning support to Razor’s intermediate representation (IR) nodes and uses it in the source generator pipeline to preserve an “unresolved” DocumentIntermediateNode snapshot, enabling tag-helper resolution/rewrite to be replayed without re-lowering from syntax. It also adds coverage and benchmarking for the new replay scenario.
Changes:
- Introduce
IntermediateNode.Clone()plus per-nodeCloneNode()implementations for IR node types. - Update the source generator project engine to cache an unresolved IR tree and resolve/rewrite against a cloned copy on replays.
- Add a new unit test to validate cloning fidelity and add a new microbenchmark/test scenario distinguishing “ignorable” independent edits.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/RazorTests.cs | Adds a (skipped) functional test for the new “independent ignorable edit” scenario. |
| src/Razor/src/Compiler/perf/Microsoft.AspNetCore.Razor.Microbenchmarks.Generator/RazorBenchmarks.cs | Adds a benchmark for the “independent ignorable edit” scenario and associated input content. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/SourceGeneratorProjectEngine.cs | Switches replay behavior to clone the stored unresolved DocumentIntermediateNode before resolution/rewrite. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Mvc/InjectIntermediateNode.cs | Implements CloneNode() for InjectIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeDocument.cs | Adds storage/accessors for an _unresolvedDocumentNode cached on the RazorCodeDocument. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UsingDirectiveIntermediateNode.cs | Implements CloneNode() for UsingDirectiveIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedExpressionAttributeValueIntermediateNode.cs | Implements CloneNode() for UnresolvedExpressionAttributeValueIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedElementIntermediateNode.cs | Implements CloneNode() for UnresolvedElementIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeValueIntermediateNode.cs | Implements CloneNode() for UnresolvedAttributeValueIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeIntermediateNode.cs | Implements CloneNode() for UnresolvedAttributeIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/PropertyDeclarationIntermediateNode.cs | Implements CloneNode() for PropertyDeclarationIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/NamespaceDeclarationIntermediateNode.cs | Implements CloneNode() for NamespaceDeclarationIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/MethodDeclarationIntermediateNode.cs | Implements CloneNode() for MethodDeclarationIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/IntermediateNode.cs | Adds IntermediateNode.Clone() and CloneNode() extensibility point (throws by default). |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/HtmlIntermediateToken.cs | Implements CloneNode() for HtmlIntermediateToken. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/HtmlContentIntermediateNode.cs | Implements CloneNode() for HtmlContentIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/HtmlAttributeValueIntermediateNode.cs | Implements CloneNode() for HtmlAttributeValueIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/HtmlAttributeIntermediateNode.cs | Implements CloneNode() for HtmlAttributeIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/FieldDeclarationIntermediateNode.cs | Implements CloneNode() for FieldDeclarationIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/DocumentIntermediateNode.cs | Implements CloneNode() for DocumentIntermediateNode (shares decl subtree/options/target by reference). |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/DirectiveTokenIntermediateNode.cs | Implements CloneNode() for DirectiveTokenIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/DirectiveIntermediateNode.cs | Implements CloneNode() for DirectiveIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/CSharpIntermediateToken.cs | Implements CloneNode() for CSharpIntermediateToken. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/CSharpExpressionIntermediateNode.cs | Implements CloneNode() for CSharpExpressionIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/CSharpExpressionAttributeValueIntermediateNode.cs | Implements CloneNode() for CSharpExpressionAttributeValueIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/CSharpCodeIntermediateNode.cs | Implements CloneNode() for CSharpCodeIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/CSharpCodeAttributeValueIntermediateNode.cs | Implements CloneNode() for CSharpCodeAttributeValueIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/ClassDeclarationIntermediateNode.cs | Implements CloneNode() for ClassDeclarationIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Extensions/TemplateIntermediateNode.cs | Implements CloneNode() for TemplateIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Extensions/SectionIntermediateNode.cs | Implements CloneNode() for SectionIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Extensions/RazorCompiledItemMetadataAttributeIntermediateNode.cs | Implements CloneNode() for RazorCompiledItemMetadataAttributeIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/RouteAttributeExtensionNode.cs | Implements CloneNode() for RouteAttributeExtensionNode. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentInjectIntermediateNode.cs | Implements CloneNode() for ComponentInjectIntermediateNode. |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/Intermediate/IntermediateNodeCloneTest.cs | Adds a new unit test asserting IR cloning produces a structurally equivalent deep copy (with intentional shared references). |
| protected override IntermediateNode CloneNode() | ||
| { | ||
| var clone = new UnresolvedAttributeIntermediateNode | ||
| { | ||
| AttributeName = AttributeName, | ||
| IsMinimized = IsMinimized, | ||
| ValueContent = ValueContent, | ||
| ValueSourceSpan = ValueSourceSpan, | ||
| AttributeStructure = AttributeStructure, | ||
| AttributeNameSpan = AttributeNameSpan, | ||
| AsTagHelperAttribute = AsTagHelperAttribute?.Clone(), | ||
| AsMarkupAttribute = AsMarkupAttribute?.Clone(), | ||
| HtmlAttributeNode = (HtmlAttributeIntermediateNode?)HtmlAttributeNode?.Clone(), | ||
| IsSynthesizedHelper = IsSynthesizedHelper, | ||
| }; |
There was a problem hiding this comment.
Seems like a good comment, and could apply to any property that is itself a node. Can we add a debug assert to each CloneNode method that deals with them, to verify that Children contains the value, to prevent future regressions? Or a test that uses reflection somehow, though presumably creation would be difficult.
There was a problem hiding this comment.
I did an audit and this node was the only case I could find that does this. I've made Clone() virtual so it can just own the entire process rather than trying to delegate back and forth between the base and the derived class.
Added a test that checks if the original node has a property with a value that also occurs in the children collection then the cloned node has the same relationship and doesn't end up with two copies instead.
| protected override IntermediateNode CloneNode() | ||
| { | ||
| var clone = new UnresolvedAttributeIntermediateNode | ||
| { | ||
| AttributeName = AttributeName, | ||
| IsMinimized = IsMinimized, | ||
| ValueContent = ValueContent, | ||
| ValueSourceSpan = ValueSourceSpan, | ||
| AttributeStructure = AttributeStructure, | ||
| AttributeNameSpan = AttributeNameSpan, | ||
| AsTagHelperAttribute = AsTagHelperAttribute?.Clone(), | ||
| AsMarkupAttribute = AsMarkupAttribute?.Clone(), | ||
| HtmlAttributeNode = (HtmlAttributeIntermediateNode?)HtmlAttributeNode?.Clone(), | ||
| IsSynthesizedHelper = IsSynthesizedHelper, | ||
| }; |
There was a problem hiding this comment.
Seems like a good comment, and could apply to any property that is itself a node. Can we add a debug assert to each CloneNode method that deals with them, to verify that Children contains the value, to prevent future regressions? Or a test that uses reflection somehow, though presumably creation would be difficult.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c45f67d5-6fe0-4f6b-8c68-c268dc7c0621
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeIntermediateNode.cs:75
- HtmlAttributeNode is assigned to reference the last entry in Children during lowering (DefaultRazorIntermediateNodeLoweringPhase.cs:1234). During cloning, the Children list is deep-cloned, but HtmlAttributeNode is handled separately, which can cause HtmlAttributeNode and the corresponding child in Children to diverge. Tag helper resolution mutates HtmlAttributeNode in place, so this can make later phases that walk Children observe stale/unmodified state.
A robust fix is to make HtmlAttributeNode a cached lookup over Children: lowering can set the cache for O(1), but cloned nodes can safely leave it unset and still retrieve the correct child clone.
visitor.VisitDefault(this);
}
protected override IntermediateNode CloneNode()
{
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/UnresolvedAttributeIntermediateNode.cs:87
- CloneNode() deep-clones HtmlAttributeNode, but HtmlAttributeNode is a cached reference to a node that also lives in Children (it’s set to Children[^1] during lowering). IntermediateNode.Clone() will also deep-clone Children, so the clone can contain two distinct HtmlAttributeIntermediateNode instances: one reachable via HtmlAttributeNode and another via Children.
If HtmlAttributeNode is changed to be a cached lookup over Children, cloning should avoid cloning it and instead leave it unset (or explicitly null) so it resolves to the child clone.
AttributeNameSpan = AttributeNameSpan,
AsTagHelperAttribute = AsTagHelperAttribute?.Clone(),
AsMarkupAttribute = AsMarkupAttribute?.Clone(),
HtmlAttributeNode = (HtmlAttributeIntermediateNode?)HtmlAttributeNode?.Clone(),
IsSynthesizedHelper = IsSynthesizedHelper,
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/HtmlIntermediateToken.cs:22
- CloneNode() uses the string-valued Content property, which forces evaluation of LazyContent and turns lazy tokens into eager ones (IsLazy becomes false). When cloning large IR trees to avoid re-lowering, this can introduce a lot of extra string materialization that the pipeline might otherwise never pay for.
If possible, consider preserving laziness by cloning the underlying LazyContent when IsLazy is true (same applies to CSharpIntermediateToken.CloneNode).
protected override IntermediateNode CloneNode()
=> new HtmlIntermediateToken(Content, Source)
{
IsSynthesizedHelper = IsSynthesizedHelper,
};
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c45f67d5-6fe0-4f6b-8c68-c268dc7c0621
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/IntermediateNode.cs:110
- CloneNode()'s doc comment currently states that any child nodes held outside Children are deep-cloned. Some node types (e.g. DocumentIntermediateNode sharing DeclDocumentNode) intentionally share known-inert subtrees, so this comment should allow for documented sharing to avoid a misleading contract.
/// Creates a copy of this node carrying only its own state -- the node-specific fields (including the
/// init-only <see cref="IsSynthesizedHelper"/>) and any child nodes held outside <see cref="Children"/>
/// (deep-cloned). The common state and the <see cref="Children"/> are copied by <see cref="Clone"/>.
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Intermediate/IntermediateNode.cs:89
- The XML doc for IntermediateNode.Clone() claims this is always a deep copy of the node and its descendants, but at least DocumentIntermediateNode.CloneNode() intentionally shares DeclDocumentNode by reference. The docs should be tightened to avoid implying a strict deep clone contract for all node-typed side properties.
This issue also appears on line 108 of the same file.
/// Returns a deep copy of this node and its descendants. The node-specific state is produced by
/// <see cref="CloneNode"/>; this method copies the common state (source span, imported flag,
/// diagnostics) and deep-clones the children onto it. A node with a property that aliases one of
/// its <see cref="Children"/> overrides this to re-point that property at the cloned child.
Due to the reordering of phases in sonic, tag helper lowering happens later in the process. This means if we need to replay back to a point earlier in the process because a TagHelper changed, the IR tree has been mutated and instead we have to perform the initial relowering from syntax again.
This PR introduces a
.Clone()mechanism to IR nodes, so that we can clone theDocumentNodebefore it gets mutated. This means we can just start from that point again instead of having to start over from syntax.The first commit is big-ish, but also almost entirely mechanical, so you can probably mostly glance over it. Commit 2 uses the clone to keep a copy of the IR tree and restore it when needed. Commit 3 just adds an extra benchmark because during testing I realized we had two different scenarios that we were equating as one and want to be able to measure them independently.
At some point I plan to make the IR nodes fully immutable, so we don't have to worry about cloning to avoid mutation, but this was the expedient way to get it working without an even bigger change.
Microsoft Reviewers: Open in CodeFlow