Discover only fallback types in the slow tag-helper path - #84808
Draft
chsienki wants to merge 2 commits into
Draft
Discover only fallback types in the slow tag-helper path#84808chsienki wants to merge 2 commits into
chsienki wants to merge 2 commits into
Conversation
When a component can't be split (an @inherits/@implements/@typeparam header or unroutable body markup), tag-helper discovery falls back to a second pass over the compilation augmented with the discovery-only decl trees. That pass walked the whole assembly and kept only the fallback types, rebuilding descriptors for every already-discovered splittable component -- costly on large component libraries where most types are components. Resolve the fallback component types from the decl trees and discover only those. Producers examine each type independently, so the descriptors are identical to the full walk's for those types, and the existing ownership filter is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
Member
Author
|
/pr-val |
Contributor
|
You do not have permission to trigger this workflow. Only Microsoft employees who are contributors to the roslyn repository can run pipelines. |
Contributor
|
Failed to trigger the pipeline. Please check the workflow logs for details. |
Member
Author
|
/pr-val |
Contributor
|
View PR Validation Run triggered by @chsienki Parameters
|
davidwengier
approved these changes
Aug 7, 2026
| CancellationToken cancellationToken) | ||
| { | ||
| using var builder = new PooledArrayBuilder<INamedTypeSymbol>(); | ||
| var seen = new HashSet<INamedTypeSymbol>(SymbolEqualityComparer.Default); |
| var semanticModel = compilation.GetSemanticModel(tree); | ||
| var root = tree.GetRoot(cancellationToken); | ||
|
|
||
| foreach (var typeDeclaration in root.DescendantNodes().OfType<TypeDeclarationSyntax>()) |
Member
There was a problem hiding this comment.
Can the DescendantNodes be passed a delegate here to avoid descending into anything but namespaces and type declarations, and make it a tiny bit more efficient?
Resolving the fallback component symbols by creating a semantic model per discovery-only decl tree bound each tree against the whole augmented compilation. On a large component library that ran on every generator pass during completion and allocated heavily -- far more than the full-assembly walk it replaced. Look the types up through the compilation's declaration table instead: key a name predicate off each fallback type's final name segment and let the existing descriptor-name filter trim the over-selection. No semantic models are created, and only the fallback types' descriptors are built. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8188d9c-18d2-4e12-8b25-4c8d99114193
Member
Author
|
/pr-val |
Contributor
|
View PR Validation Run triggered by @chsienki Parameters
|
chsienki
added a commit
that referenced
this pull request
Aug 21, 2026
…#84844) Re-lands the sonic decl/impl split (backed out in #84831 to unblock insertions) together with the slow-discovery perf work (#84808) and a new fix that removes the remaining cold-allocation regression on fallback components. ## Why this is safe to re-land now The split was reverted because a VS insertion showed a cold-allocation regression on fallback-heavy scenarios (Component Classification, cohosting completion) with no offsetting speed gain. This branch addresses the root cause rather than just re-applying the revert. ## What's here 1. **Un-revert** of #84831 -- restores the sonic merge and the fixes that landed on top of it (#84818 fallback child-content discovery, #84813 RZC public-API compat, #84795 formatting options). 2. **#84808** -- slow discovery resolves fallback types via the declaration table (`GetSymbolsWithName`) instead of a per-tree semantic model, and discovers only the fallback types rather than re-walking the whole augmented compilation. (Removed the +535 MB semantic-model regression.) 3. **Reuse the initial parse for fallback discovery** (new) -- the remaining regression. ## The new fix A fallback component (an `@inherits`/`@implements`/`@typeparam` header, or body markup the analysis can't route) can't be split, so its descriptor comes from discovery over an augmented compilation. Each fallback component was being processed **twice**: the generation engine's initial parse, then a freshly created declaration engine that **re-parsed the same source** just to produce a discoverable decl. For an app where nearly every component `@inherits` a base (MudBlazor, OrchardCore) every component is a fallback, so that second parse dominates cold discovery allocation. The split phase already builds the discoverable declaration surface for a *split* component from the nodes it just classified. This builds the same surface for a *fallback* component and stashes it on the document node; the generator lowers it in place -- reusing the initial parse -- instead of re-parsing through a separate engine. It's never emitted to pre-compilation (the bodiless type shell still fills that role for C# type resolution); only its syntax tree feeds slow discovery. The rare shapes with no render method or namespace keep the re-parse path. ## Measurements Cold source-generator allocation over 100 fallback components (in-memory A/B): | | Allocated | |---|---| | Pre-split baseline | 17.1 MB | | Sonic + #84808 (before this fix) | 19.6 MB | | **This branch** | **13.6 MB** | The fallback path is now **below** the pre-split baseline -- a genuine win, not break-even. ## Validation - `Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests`: 219 passed / 1 skipped (incl. #84818's fallback child-content test). - `Microsoft.AspNetCore.Razor.Language.UnitTests` (component / tag helper / split / bind / discovery / clone): 1672 passed. - Razor compiler builds clean. Draft pending the RPS + Speedometer insertion run. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/84844) --- ## Update: warm/interactive path fix (html-span completion) A matched-baseline Speedometer re-run showed the cold-discovery regressions gone but a smaller residual on **html-span completion in a component** (`CLR_BytesAllocated_devenv`) -- the interactive/warm path, a different signature than the cold source-generator work above. Root cause: the fallback decl incremental pipeline compared decls by **instance**, while the split-decl (`DeclSources`) fast path compares by **text**. A fallback component's decl is markup-free and checksum-suppressed, so a markup-only edit (every keystroke during completion) yields a byte-identical decl -- but the instance changed each run, invalidating the parse and, through the `Collect`, re-running slow discovery over *every* fallback component on each edit. Fix: compare fallback decls by text (`Text.ContentEquals`), mirroring the fast path. A markup-only edit now leaves the fallback decl `Unchanged`, keeping its parse and slow discovery cached. Measured (100 fallback components, warm re-run on a markup edit): | | warm alloc | |---|---| | Pre-split baseline | 0.41 MB | | Before this fix | 3.2 MB (~8x) | | **After this fix** | **0.42 MB** | Guarded by `IncrementalCompilation_WhenFallbackComponentMarkupChanges_SlowDiscoveryStaysCached` (verified to fail without the comparer). Full SG suite 220 passed / 1 skipped.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Draft for RPS validation. Addresses the
CLR_BytesAllocatedregression seen on the Roslyn->VS insertion (RazorEditingTests.CompletionInCohostingForComponents, MudBlazor).Root cause
The decl/impl split's slow tag-helper discovery re-walks the whole compilation whenever any component falls back (an
@inherits/@implements/@typeparamheader or unroutable body markup). It kept only the fallback types but rebuilt descriptors for every already-discovered splittable component. On large component libraries like MudBlazor (hundreds of components, most falling back), that is a redundant near-full second descriptor build -> the observed cold-classification allocation regression.Change
slowTagHelpersnow resolves just the fallback component type symbols (from the discovery-only decl trees) and discovers only those, via a new per-typeTagHelperDiscoverer.GetTagHelpers(ImmutableArray<INamedTypeSymbol>). Correct becauseComponentTagHelperProducer.AddTagHelpersForTypeis per-type/independent, so the descriptors are identical to the full walk's for those types; the existingfallbackTypeNamesownership filter is retained as a safety net.Validation
Microsoft.CodeAnalysis.Razor.Compilerbuilds clean (netstandard2.0 + net10.0), 0 warnings.Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests: 218 passed, 1 skipped, 0 failed.Microsoft.CodeAnalysis.Razor.UnitTests(TagHelper/Component/Discovery): 544 passed.Opening as draft to run through RPS and quantify the allocation win before review.
Microsoft Reviewers: Open in CodeFlow