Add a UTF-8 write-literal phase so incremental codegen stops duplicating component methods - #85051
Conversation
…thods A project mixing .cshtml pages and .razor components could emit a component with duplicated generic type-inference methods (CreateXxx_0), producing uncompilable output (CS0111/CS0121). Editing an unrelated .cshtml so its @inherits base type gained or lost a WriteLiteral(ReadOnlySpan<byte>) overload was enough to trigger it. The project-wide UTF-8 support map was combined into every document's final code-generation step, which also ran the intermediate-node optimization passes. Those passes mutate a per-document tree that is lowered once and cached, so when the map changed the step re-ran for every document -- including untouched components -- and replaying the passes appended the type-inference methods a second time. UTF-8 detection now lives in its own engine phase, Utf8WriteLiteralPhase, that runs after optimization and before C# lowering. It reads a per-document Utf8SupportMap threaded onto the RazorCodeDocument the way tag helpers are, so the source generator can run it on its own between the cached optimization stage and read-only emission. The generator's back half splits into OptimizedDocuments (cached, map-independent), Utf8Documents (runs the phase, compared by document identity plus the flag), and GeneratedCode, so a map change re-emits only the document it actually affected. DevDiv 3050748. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
This PR restructures Razor source-generator codegen to avoid duplicated component type-inference methods during incremental runs by isolating UTF-8 literal detection into a dedicated engine phase and adding new incremental-cache boundaries.
Changes:
- Adds
Utf8WriteLiteralPhaseand wires it into the default Razor engine phase pipeline (between optimization and C# lowering). - Refactors the source generator pipeline into distinct incremental steps (
OptimizedDocuments→Utf8Documents→GeneratedCode) and adds ETW spans for the new stages. - Adds/updates unit tests covering the reported incremental duplication scenario and UTF-8 literal decision changes, while removing older integration coverage tied to the previous feature-based implementation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/RazorSourceGeneratorCshtmlTests.cs | Adds regression + incremental-shape tests for UTF-8 map changes and avoiding duplicated component methods. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/SourceGeneratorProjectEngine.cs | Splits processing into optimization / UTF-8 decision / C# lowering stages. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGeneratorEventSource.cs | Adds ETW events for optimization + UTF-8 computation spans. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.Helpers.cs | Removes registration of the old UTF-8 feature from the generator engine setup. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs | Reworks the incremental pipeline to introduce new cache boundaries and the UTF-8-only step. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorProjectEngine.cs | Registers the new UTF-8 phase and removes the old optimization pass. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/RazorCodeDocument.cs | Threads a per-document Utf8SupportMap through RazorCodeDocument via WithUtf8SupportMap. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/Utf8WriteLiteralPhase.cs | New phase that records the UTF-8 literal decision on document options for legacy (.cshtml) docs. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/Utf8WriteLiteralDetectionPass.cs | Removes the prior optimization-pass-based detection implementation. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/Utf8SupportMap.cs | New value-comparable support map implementation used by the generator and the UTF-8 phase. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/IUtf8WriteLiteralFeature.cs | Removes the prior engine feature interface. |
| src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/DefaultUtf8WriteLiteralFeature.cs | Removes the prior feature implementation (map storage + lookup). |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/RazorProjectEngineTest.cs | Updates default phase/feature assertions for the new phase and removed pass. |
| src/Razor/src/Compiler/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/IntegrationTests/CodeGenerationIntegrationTest.cs | Removes UTF-8 detection tests that depended on the old feature-based configuration. |
davidwengier
left a comment
There was a problem hiding this comment.
Should consider adding a test for the duplicated attribute scenario, from https://devdiv.visualstudio.com/DevDiv/_workitems/edit/3052471, if it's not too complicated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/RazorSourceGeneratorCshtmlTests.cs:730
- This assertion only checks that exactly one GeneratedCode output was Modified, but it doesn’t verify which document changed. If the component re-emits and the page stays cached (or vice versa), this test would still pass.
var reasons = result.TrackedSteps["GeneratedCode"]
.SelectMany(step => step.Outputs.Select(output => output.Reason))
.ToArray();
Assert.Equal(2, reasons.Length);
Assert.Equal(1, reasons.Count(reason => reason == IncrementalStepRunReason.Modified));
- IsSupported normalizes a null file path to string.Empty before the per-file lookup, matching how Create stores keys. A document with no Source.FilePath previously skipped the per-file entry and fell through to the raw base type name, which can be a different format. - Pool the unresolved-entries and resolved lists in Create, and build nested metadata names with a pooled StringBuilder instead of a List + Reverse + Join. - Strengthen the unaffected-component incremental test to assert the run reason per source file, so it verifies the component's emission stays cached while the .cshtml re-runs rather than just counting one re-emit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/Utf8SupportMap.cs:238
- Utf8SupportMap._fileToType is built with StringComparer.OrdinalIgnoreCase, but Equals currently compares dictionaries via SequenceEqual, which is case-sensitive on the stored key strings. Two semantically equivalent maps that differ only by file-path casing would compare unequal, causing unnecessary incremental invalidations when path casing varies (common on Windows).
return _fileToType.SequenceEqual(other._fileToType) &&
_typeSupport.SequenceEqual(other._typeSupport);
}
… update A .cshtml @inherits change flips the project-wide UTF-8 support map and re-runs code generation for an unrelated bystander .cshtml. Assert its [assembly: RazorCompiledItem(...)] attribute is emitted exactly once and is not duplicated by the re-run. DevDiv 3052471. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/Utf8SupportMap.cs:26
- The XML doc comment claims the per-file lookup is keyed by
(filePath, rawInheritsText), but the implementation only keys_fileToTypebyfilePath. This is misleading for future maintainers and makes the 2-level lookup description inaccurate.
/// A value-comparable map that determines whether a file's <c>@inherits</c> base type supports
/// UTF-8 <c>WriteLiteral</c>. Uses a two-level lookup:
/// <list type="number">
/// <item>Per-file: maps <c>(filePath, rawInheritsText)</c> to a fully-qualified type name</item>
/// <item>Per-type: maps fully-qualified type name to <see langword="bool"/></item>
_fileToType is keyed with StringComparer.OrdinalIgnoreCase and GetHashCode already hashes those keys case-insensitively, but Equals compared them with SequenceEqual (case-sensitive). Maps differing only by file-path casing compared unequal, causing spurious incremental invalidation, and Equals was inconsistent with GetHashCode. Compare via the dictionary's own lookup. Also correct the type doc: the per-file level is keyed by filePath alone, not (filePath, rawInheritsText). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs:427
- This Select lambda doesn’t capture any locals; marking it
staticavoids an unnecessary closure allocation and matches the surrounding incremental pipeline style (most other lambdas here arestatic).
This issue also appears on line 441 of the same file.
.Select((tuple, cancellationToken) =>
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/RazorSourceGeneratorCshtmlTests.cs:809
- Using reflection to read the ValueTuple
Item1field is brittle (field name/type changes will throw at runtime) and harder to diagnose. ValueTuple implements System.Runtime.CompilerServices.ITuple, which provides a stable way to read the first element without naming the internal document type.
// Key the run reason by source file path so we verify *which* document re-emitted -- the
// "GeneratedCode" step output value is a (filePath, document) tuple; grab Item1 by reflection
// to avoid naming the internal document type.
var reasonsByFile = result.TrackedSteps["GeneratedCode"]
.SelectMany(step => step.Outputs)
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/RazorSourceGenerator.cs:441
- This Select lambda doesn’t capture any locals; marking it
staticavoids an unnecessary closure allocation and matches the surrounding incremental pipeline style.
.Select((pair, cancellationToken) =>
…s and test - Utf8SupportMap.Create's fast path already holds the resolved base-type symbol, so check it directly instead of round-tripping type.GetFullName() (a display name) back through the metadata-name overload, which would mis-resolve generic or nested types. - Mark the non-capturing OptimizedDocuments/Utf8Documents/GeneratedCode pipeline lambdas static to avoid closure allocations. - Read the tracked GeneratedCode output's file path via ITuple instead of reflection over the ValueTuple field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 230cea96-9056-412f-81f9-15156f4f3215
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Razor/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/SourceGenerators/SourceGeneratorProjectEngine.cs:220
- ProcessUtf8 executes phases on a new RazorCodeDocument (with Utf8SupportMap attached) but only returns the bool flag. The updated RazorCodeDocument is not propagated to the caller, so any phase work that relies on returning a new RazorCodeDocument instance (vs mutating shared IR nodes in place) would be lost before ProcessCSharp runs. This is currently safe because the Utf8WriteLiteralPhase mutates DocumentIntermediateNode.Options in-place, but it makes the pipeline fragile if additional phases ever appear between Utf8WriteLiteralPhase and C# lowering (or if Utf8WriteLiteralPhase changes to return a new document node). Consider threading the updated document forward (e.g., have ProcessUtf8 return an updated SourceGeneratorRazorCodeDocument along with the flag, and pass that to ProcessCSharp).
public bool ProcessUtf8(SourceGeneratorRazorCodeDocument sgDocument, Utf8SupportMap utf8SupportMap, CancellationToken cancellationToken)
{
var codeDocument = sgDocument.CodeDocument.WithUtf8SupportMap(utf8SupportMap);
codeDocument = ExecutePhases(Phases[_utf8PhaseIndex.._csharpLoweringPhaseIndex], codeDocument, cancellationToken);
return codeDocument.GetDocumentNode()?.Options?.WriteHtmlUtf8StringLiterals ?? false;
Brings the branch up to date with main (942 commits). The only conflict was InheritsInfo: main's dotnet#85051 refactored the UTF-8 write-literal feature, moving InheritsInfo from DefaultUtf8WriteLiteralFeature.cs (deleted) into Utf8SupportMap.cs. Re-applied the reference-type change there so the incremental-generator driver's instantiations over it stay shared rather than JITting a dedicated value-type form.
A project that mixes
.cshtmlpages and.razorcomponents can emit a component with duplicated generic type-inference methods (CreateXxx_0), producing uncompilable output (CS0111/CS0121). Editing an unrelated.cshtmlso its@inheritsbase type gains (or loses) aWriteLiteral(ReadOnlySpan<byte>)overload is enough to trigger it. DevDiv 3050748.The cause is the project-wide UTF-8 support map. It was combined into every document's final code-generation step, and that step also ran the intermediate-node optimization passes, whose per-document tree is lowered once and cached. When the map changed, that step re-ran for every document -- including untouched components -- and replaying the optimization passes appended the type-inference methods to the already-populated tree a second time.
UTF-8 detection moves out of the optimization passes into its own engine phase,
Utf8WriteLiteralPhase, registered afterDefaultRazorOptimizationPhaseand before C# lowering. The phase reads a per-documentUtf8SupportMapoff theRazorCodeDocument(threaded in the same way tag helpers are, viaWithUtf8SupportMap) and records the UTF-8 flag on the document -- definitively, so support that disappears turns byte literals back off.The generator's back half then splits into three stages with cache boundaries between them:
OptimizedDocuments-- the mutating optimization passes, up to but not including the UTF-8 phase. Independent of the map, so it stays cached per document and never replays (the correctness fix).Utf8Documents-- attaches the support map and runs just theUtf8WriteLiteralPhase, reporting the resulting flag. It re-runs when the map changes, but its output is compared by document identity plus the flag, so it reports unchanged for documents whose decision did not move.GeneratedCode-- C# emission. It stays cached unlessUtf8Documentsreports a change, so an@inheritsedit re-emits only the document it actually affected.Each stage emits its own ETW span (
RazorOptimize*,RazorComputeUtf8Literals*,RazorCodeGenerate*).Regression tests: two for the cross-file duplication (base-type overload added, and an
@inheritsalias retargeted), one for byte literals reverting to string literals when the overload is removed, and two for the incremental shape -- a map-only change keepsOptimizedDocumentscached while emission re-runs only for the affected page, leaving an unrelated component's emission cached.Microsoft Reviewers: Open in CodeFlow