Skip to content

Find protocol implementors and union members in referenced assemblies, and enforce placement (F3) - #8537

Merged
Aaronontheweb merged 1 commit into
feature/serialization-v2-expansion-adoptionfrom
feature/serialization-v2-implementor-walk
Sep 11, 2026
Merged

Aaronontheweb merged 1 commit into
feature/serialization-v2-expansion-adoptionfrom
feature/serialization-v2-implementor-walk

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Stack, bottom to top: S1 #8525 -> S2 #8526 -> S3 #8527 -> object elements #8528 -> S4 #8530 -> S5 #8532 -> S6 #8533 -> F1 #8534 -> F2 #8536 -> F3 #8537 -> S7 #8538. Each PR is one commit on top of the one below it. This PR is F3, the last feature step. It closes the last pinned failure in the cross-assembly baseline.

What changes

The generator now sees across assembly boundaries in both directions the design asked for, and enforces where a serializer may live. The rules are Decisions 19 and 21 in the design record and Decisions E and G on the decision page.

  • Protocol implementors in referenced assemblies count. For each referenced assembly that itself references Akka.Serialization.V2, the generator walks its public types once, collects the marked implementors of each protocol and each marked union base, and adds them to the serializer's closed set, with schemas from the F1 metadata step. Baseline case 6, the last pinned failure, flips: an implementor declared only in a referenced assembly gets a dispatch arm, helpers, and a binding.
  • [AkkaUnion] with no member list means every [AkkaSerializable] implementor the generator can see, on both sides of the boundary. AkkaUnionAttribute gains a parameterless constructor, extend-only. The explicit list keeps working, and the existing union diagnostics run over the combined set.
  • Placement rules. AKKASG043, error: a type implements a protocol an upstream serializer already owns, and that assembly cannot see this one, so the type would never be dispatched. AKKASG044, warning: a serializer with no messages anywhere and no registrations. Rule 3 extends AKKASG031 across assemblies with a new message, as the decision page labels it. Rule 4 has no id: the generated "unsupported type" exception now names both the value's assembly and the generating assembly. That is why six existing golden files changed by two lines each.
  • The startup one-owner check. When registrations are composed into a setup, one dictionary pass throws with both serializer names if two registrations claim the same message type.
  • The walk is cheap. Local marked implementors come from the cached messages, not a symbol walk. The referenced-assembly walk is cached per metadata reference, so it runs once per assembly per IDE session. A test proves an edit to the user's own source does not re-walk an unchanged reference, and that a replaced reference does.
  • Docs and design record. Guide sections on marked union bases, placement, and the startup check. Decisions 19 and 21 gain addenda, including one disagreement: the decision page's fix list for AKKASG043 mentions declaring a serializer part, which does not exist yet, so the message leaves it out.

What it took

Three rounds. The first cut allocated 18 percent more per keystroke by walking the compilation for types the extraction step had already produced as cached values. The cache was then keyed on the assembly symbol, which Roslyn reuses only while the old symbol is still alive; under memory pressure the proof test failed. It is now keyed on the metadata reference. The proof test then failed in the full suite because this project runs tests at the same time despite its runner file, so a global counter picked up other tests' walks. The test now counts walks by the assembly names it compiles. The runner setting is untouched and worth its own look.

Known limit, scheduled as S7

The ManifestPrefix expansion from F2 still finds its closed set with a walk inside the serializer's per-keystroke step. It is correct across assemblies now, proven by a golden case, but it does not consume the cached facts. S7 moves it.

Cost

Base is the F2 tip. The corpus references Akka.Remote, which references V2, so the referenced-assembly walk is in play on every row. The cache is what keeps the rows flat. Allocations measured twice on the same machine.

Row Base (F2) This PR
Fresh driver, full corpus 22.9 ms, 19.00 MB 23.8 ms, 19.07 MB
Warm driver, one field renamed 16.9 ms, 9.57 MB 18.2 ms, 9.64 MB
Warm driver, comment edit 9.4 ms, 3.55 MB 9.7 ms, 3.61 MB
Warm driver, unrelated file edited 7.7 ms, 3.53 MB 8.0 ms, 3.59 MB

How it was checked

369 tests pass (350 plus 19: placement diagnostics, marked union bases, the startup check, three golden cases, a caching scenario, and the cache-proof test), three runs in a row. V2, the generator, and Akka.Remote build with warnings as errors. Akka.API.Tests: 18 pass. cspell passes on the guide. Local runs used NuGetAudit=false on the command line during the restore break that #8535 fixed; nothing of that is committed.

@Aaronontheweb
Aaronontheweb added this pull request to stack #8531 September 9, 2026 13:28
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from ab82136 to 47b77d9 Compare September 9, 2026 13:32
@Aaronontheweb Aaronontheweb changed the title Serialization.V2: referenced-assembly implementor walk; marked union bases; closed-set discovery in the facts stage (Decisions 19 and 21, F3) Find protocol implementors and union members in referenced assemblies, and enforce placement (F3) Sep 9, 2026
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from 47b77d9 to ebd6c33 Compare September 9, 2026 17:15
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from ebd6c33 to 0b334a9 Compare September 9, 2026 18:08
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from 0b334a9 to ae84690 Compare September 9, 2026 22:52
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from ae84690 to a4162f9 Compare September 10, 2026 23:44
…bases; closed-set discovery in the facts stage (Decisions 19 and 21)

Decision 19: the generator now walks every referenced assembly that itself
references Akka.Serialization.V2, adopting every [AkkaSerializable] implementor
of a serializer's protocol for top-level dispatch. Decision 21: [AkkaUnion]
regains a parameterless form that discovers its member set the same way,
across the same assembly boundary. Both share one walk, moved into the
CompilationFacts stage and wired directly into ResolvedSerializers. AKKASG029
and AKKASG012 widen to the combined set. Four placement rules turn a
misplaced serializer into a build-time or runtime diagnostic: two new ids
(AKKASG043, AKKASG044), one extended existing id (AKKASG031, across
assemblies), and an improved runtime exception message. A startup one-owner
check in SerializerRegistration.CreateSetup covers the case neither
compilation can see the other's serializer.

ManifestPrefix expansion is now cross-assembly-correct too, but still runs
per-registration rather than sharing CompilationFacts' own cached walk -- a
documented caching-granularity follow-up, not a functional gap.

An initial pass at the facts-stage walk left an allocation regression on the
SourceGeneratorBenchmarks corpus (up to ~20% over baseline, driven by
Akka.Remote sitting in the corpus's reference set). Fixed two ways: local
marked implementors are now a pure filter over the already-collected message
array instead of a redundant symbol walk, and the referenced-assembly walk is
memoized in a process-lifetime ConditionalWeakTable, so it runs once per
reference rather than once per edit. All four benchmark rows now land within
~1% of the ac3376f baseline.

That cache was first keyed on the referenced assembly's own IAssemblySymbol,
which reuses across compilations only while an earlier bound symbol for the
reference is still reachable through Roslyn's own internal, weak symbol
cache -- not guaranteed, and observed to fail under memory pressure on a
second machine, producing a real (not flaky-only-in-appearance) cache miss on
every edit in exactly the conditions an IDE creates constantly. Rekeyed on
Compilation.GetMetadataReference(assemblySymbol) instead -- the workspace and
the generator driver both keep the same MetadataReference instance across an
edit for as long as the reference set itself is unchanged, and the walk's
result depends only on the PE file's metadata, not on which symbol instance
happens to represent it right now. Falls back to the assembly symbol only
when a compilation has no separate reference for it (a source or
compilation-referenced assembly). The caching-proof test now asserts a delta
across a three-run sequence: zero additional walks across an edit that
reuses the same reference, and exactly one more against a freshly compiled,
genuinely distinct reference over identical source -- proving the cache is
scoped to the reference itself, not global. The test also neutralizes an
incidental confound the test harness introduces (its own base reference set
includes the executing test assembly, which itself qualifies for the walk),
warming that entry before measuring so the asserted delta is attributable
only to the references this test constructs.

That test's own delta, in turn, used a single process-wide counter -- wrong
in this test project specifically, whose tests do not execute one at a time
on a single thread despite parallelizeAssembly/parallelizeTestCollections
both being false in the copied xunit.runner.json (confirmed present, with
that content, in both bin/Debug and bin/Release output; no
[assembly: CollectionBehavior] attribute exists anywhere in the project
either): a different test's own walk of its own, differently-named reference
can land inside another test's counting window and inflate its delta. Added
ReferencedAssemblyWalksByAssemblyName, a ConcurrentDictionary<string, int>
keyed by the walked assembly's own name, incremented next to the existing
counter; the caching-proof test now snapshots and diffs that dictionary
filtered to two assembly names it alone uses, immune to any concurrently
running test's own walk of a different name.

19 new tests (placement diagnostics, marked union bases, the startup check,
three golden-output cases, an incremental caching-proof scenario, and the
referenced-assembly-walk caching-proof scenario); the six pre-existing golden
baselines with the improved runtime exception text were regenerated and
reviewed.
@Aaronontheweb
Aaronontheweb force-pushed the feature/serialization-v2-implementor-walk branch from a4162f9 to 092552d Compare September 11, 2026 15:05
@Aaronontheweb
Aaronontheweb merged commit 0d78532 into dev Sep 11, 2026
16 of 17 checks passed
@Aaronontheweb
Aaronontheweb deleted the feature/serialization-v2-implementor-walk branch September 11, 2026 17:29
Aaronontheweb added a commit that referenced this pull request Oct 2, 2026
…sted message types (#8721)

Docs no longer say an object collection element is unsupported (#8528).
AKKASG007 and AKKASG015 cross-assembly text no longer says the generator
cannot read a schema from a referenced assembly (#8534, #8537).
New AKKASG045 reports an [AkkaSerializable] type that is private or
protected, or nested in one, instead of CS0122 in generated code.

Closes #8717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant