Skip to content

Revert the sonic decl/impl split merge from main - #84831

Merged
JoeRobich merged 4 commits into
dotnet:mainfrom
chsienki:revert/sonic-merge
Aug 11, 2026
Merged

Revert the sonic decl/impl split merge from main#84831
JoeRobich merged 4 commits into
dotnet:mainfrom
chsienki:revert/sonic-merge

Conversation

@chsienki

@chsienki chsienki commented Aug 10, 2026

Copy link
Copy Markdown
Member

Backs the features/sonic decl/impl split out of main to unblock insertion. Reverts the merge (#84778) and the commits that stacked on top of it and cannot stand without it.

What reverts

What is kept

The other post-merge commits are independent of sonic and stay: arcade bumps, SDL param removal, CS8802, #84758 (syntax-tree sharing), and the two Razor features that merely shared files -- #84760 (unbound-attribute warnings) and #84764 (empty-outer-tag warning).

Preserving #84760 needed one manual resolution in UnresolvedAttributeIntermediateNode.cs: sonic added the CloneNode()/Clone() overrides and #84760 later added one property assignment inside CloneNode. The overrides are sonic infrastructure and go with the revert; #84760's IsDirectiveAttributeCandidate property and all its real usages are untouched.

Validation

Note

Reverting the merge means re-landing features/sonic later will need this revert itself reverted first -- expected for a back-out.

Microsoft Reviewers: Open in CodeFlow

Copilot AI lite review requested due to automatic review settings August 10, 2026 18:38
@chsienki
chsienki requested review from a team as code owners August 10, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

@chsienki

Copy link
Copy Markdown
Member Author

/pr-val

@github-actions

Copy link
Copy Markdown
Contributor

View PR Validation Run triggered by @chsienki

Parameters
  • Validation Type: pr-val
  • Pipeline ID: 8972
  • Pipeline Version: main
  • PR Number: 84831
  • Commit SHA: 967b0d3d06f812949fb015c16333726c66887c3f
  • Source Branch: revert/sonic-merge
  • Target Branch: main
  • Build ID: 14927709

@JoeRobich
JoeRobich merged commit 5864742 into dotnet:main Aug 11, 2026
27 checks passed
333fred added a commit to 333fred/roslyn that referenced this pull request Aug 11, 2026
…-15-langver

* upstream/main:
  Unsafe evolution: allow await in unsafe context (dotnet#84616)
  Fix empty localization stage on non-main/release branches in official pipeline (dotnet#84845)
  Reorder stages for localization and build (dotnet#84841)
  Revert the sonic decl/impl split merge from main (dotnet#84831)
  AI Workflow - switch to use personal token instead of organizational (dotnet#84832)
  [main] Update dependencies from dotnet/arcade (dotnet#84819)
  Do not offer introduce parameter for incomplete calls (dotnet#84769)
chsienki added a commit that referenced this pull request Aug 21, 2026
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.
@jjonescz jjonescz added this to the 18.11 milestone Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants