Skip to content

Popcorn v8: Roslyn source generator + AOT/trim support - #73

Merged
NicholasMTElliott merged 44 commits into
masterfrom
spike/source-generator
Apr 24, 2026
Merged

NicholasMTElliott merged 44 commits into
masterfrom
spike/source-generator

Conversation

@NicholasMTElliott

Copy link
Copy Markdown
Contributor

Summary

v8 replaces the reflection-based Popcorn engine with a Roslyn source generator (Popcorn.SourceGenerator) + a clean runtime library (Popcorn.Shared). Enables PublishAot=True + PublishTrimmed=True, ships faster than raw System.Text.Json on the nested-data case, and ships side-by-side with v7 on NuGet.

41 commits, 135 files changed, representing the spike/source-generator branch worked across months.

Highlights

  • Performance (vs raw System.Text.Json, on ComplexModelList): Popcorn-default = 0.10× time / 0.20× alloc. Popcorn-all = 0.87× time / 0.93× alloc — faster than STJ when emitting everything on nested data. Full 3-way report under benchmarks/results/v2-baseline/.
  • Native AOT + trimming: PopcornAotExample builds with PublishAot=True, runs four endpoints end-to-end. Canary smoke exercised by the new AOT CI job.
  • Custom envelope + exception middleware (Tier-1 shipped): [PopcornEnvelope] + [PopcornPayload] / [PopcornError] / [PopcornSuccess] markers + UsePopcornExceptionHandler(). Reflection-free dispatch via a generator-emitted error writer + PopcornErrorWriterRegistry.
  • [SubPropertyDefault("[Make,Model]")] (Tier-1 shipped): v7's [SubPropertyIncludeByDefault], renamed and generator-backed. Pre-parsed once per process into a static readonly field.
  • Generator diagnostics JSG003–JSG008: malformed envelopes (missing/duplicate markers, wrong payload/error types, generic-outer nesting) + AOT non-starter polymorphic shapes (object / abstract / interface members).
  • Test suite: 182 passing / 2 skipped / 0 failing in Popcorn.FunctionalTests (21 test files); 19 passing in Popcorn.SourceGenerator.Tests. Zero CS86xx warnings in generated code.
  • NuGet packages: Skyward.Api.Popcorn.SourceGen (analyzer, developmentDependency) + Skyward.Api.Popcorn.SourceGen.Shared (runtime), both at 8.0.0-preview.1. SourceLink + snupkg. Verified via local throwaway-consumer smoke test.
  • CI surface (all new on this branch):
    • tests.yml — runs full test suite on PR + push.
    • aot-ci.yml — builds AOT Docker image, smokes the four endpoints including exception middleware.
    • benchmarks.yml — ratio-gate on three load-bearing Popcorn / STJ ratios; fails on > 25% regression. Ratios are stable across runners (±5%) while absolute ns/op varies ±20–30%.
    • main.yml — stripped of legacy v7 pack steps; now v8-only.

Scope decisions documented in the PR

Deliberately not shipped, with replacement patterns documented in docs/MigrationV7toV8.md:

Dropped Reason v8 replacement
Sorting, pagination, filtering, authorization Never used in practice with the legacy engine; modern ASP.NET covers this cleanly Endpoint-level handling (IQueryable.OrderBy, Skip/Take, [Authorize], policy handlers)
SetContext(Dictionary<string,object>) Superseded by DI Register services, inject into endpoints
SetInspector(lambda) for error wrapping Moved to middleware UsePopcornExceptionHandler() + [PopcornEnvelope]
MapEntityFramework<S,P,Ctx> / [ExpandFrom] v7's interception feature, not clean parity; specced design was a factory not a dispatcher [Never] on source / hand-written factory / Mapster.SourceGenerator
[Translator] with DI Serialization-time DI is an antipattern (N+1, hidden I/O, scope threading) Endpoint-side resolution (batchable, testable)
IPopcornBlindHandler<TFrom,TTo> Redundant with standard JsonConverter<T> Standard JsonConverter<T> on JsonSerializerOptions.Converters

Each decision has a detailed section in the migration guide with the recommended pattern.

Breaking changes from v7 (abridged)

  • Attribute renames: [IncludeByDefault] → [Default], [IncludeAlways] → [Always], [InternalOnly] → [Never], [SubPropertyIncludeByDefault] → [SubPropertyDefault].
  • Fluent-lambda config surface removed entirely. Type discovery moves to [JsonSerializable(typeof(ApiResponse<T>))] on a JsonSerializerContext.
  • ?include= now matches the wire name (from [JsonPropertyName] / JsonNamingPolicy), not the C# identifier. Client-visible contract.
  • New namespaces: Popcorn (attributes), Popcorn.Shared (envelopes, middleware, DI helpers).
  • Side-by-side install with v7 is supported — package IDs differ (Skyward.Api.Popcorn.SourceGen* vs Skyward.Api.Popcorn*).

See docs/MigrationV7toV8.md for the full upgrade story.

Legacy still on master

Not touched by this PR: PopcornNetStandard/ and PopcornNetStandard.WebApiCore/ (the v7 reflection engine and middleware). They remain in the tree for a release or two so v7 consumers can continue shipping from master during migration.

Test plan

  • tests.yml passes (182 functional + 19 generator tests).
  • aot-ci.yml passes — Docker build succeeds, all four endpoints respond as expected.
  • benchmarks.yml passes — three ratios within ±25% of committed baseline.
  • Inspect a sample generated converter in $(BaseIntermediateOutputPath)Generated/ for the AOT example to confirm it's readable.
  • Install both NuGet packages in a throwaway consumer project from a local feed; confirm analyzer runs and runtime types resolve.

🤖 Generated with Claude Code

NicholasMTElliott and others added 30 commits November 22, 2024 14:01
Will need to validate if this can run on earlier .net versions.
Generator now skips enums in the reference walk (so JsonStringEnumConverter /
per-type [JsonConverter] attrs work transparently) and walks inherited members
up the base-class chain so [Always]/[Default] on a base class apply to derived
types. Adds a broad xUnit TDD ledger (126 pass / 51 skipped) covering every
Tier-1/2 feature in the v2 API plan, plus memory-bank rewrite (projectbrief,
productContext, systemPatterns, techContext, activeContext, progress) and new
apiDesign.md + migrationAnalysis.md capturing the source-gen scope decisions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…middleware

Scope decision: sorting, pagination, filtering, and authorizers were never
used in practice with the legacy reflection engine and carried build-time
dispatch + middleware complexity that we no longer want to pay for. Their
skipped TDD stubs and fixture models are removed.

Replace them with the last Tier-1 feature — configurable response envelope
and structured exception handling — wired up via marker attributes so the
generator can stay AOT-safe and reflection-free on the hot path.

Runtime (Popcorn.Shared):
- ApiError record; ApiResponse<T> gains Error slot and FromError factory.
- PopcornOptions with EnvelopeType + DefaultNamingPolicy; idempotent
  AddPopcorn(Action<PopcornOptions>).
- [PopcornEnvelope] / [PopcornPayload] / [PopcornError] / [PopcornSuccess]
  marker attributes.
- PopcornErrorWriterRegistry (Volatile-guarded) + UsePopcornExceptionHandler
  middleware that buffers the response, honors the configured naming
  policy, preserves custom headers, and strips Content-Length from
  aborted inner handlers.

Source generator:
- Recognizes [PopcornEnvelope] types in [JsonSerializable] attrs alongside
  ApiResponse<T> and walks the [PopcornPayload] type for inner references.
- Walks envelope base classes for markers; supports envelopes nested in
  non-generic outer types.
- Emits per-envelope error writers via both AddPopcornOptions (JSON-level)
  and AddPopcornEnvelopes (DI-level, AOT-friendly).
- Emits diagnostics JSG003–JSG007 for malformed envelopes (missing
  payload, duplicate markers, non-Pop<T> payload, non-ApiError error,
  generic-outer nesting).

Tests: 140 passing / 18 skipped / 0 failing in Popcorn.FunctionalTests
(up from 126/51/0 before the scope shift). New Popcorn.SourceGenerator.Tests
project with 6 generator-diagnostic tests using CSharpGeneratorDriver.
AOT example still builds with PublishAot=True.

Memory bank updated to document the scope decision and the shipped surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tests (+6 functional, all passing — 146 total in Popcorn.FunctionalTests):
- Middleware rethrows when Response.HasStarted == true.
- Multiple [PopcornEnvelope] types dispatched by the same generated writer.
- ApiError.Detail round-trips through the default error envelope.
- [JsonPropertyName] on a marker property overrides the C# wire name.
- Envelope declared as a record class works like a plain class.
- AddPopcornEnvelopes() is idempotent on repeated calls.

New fixtures in Models/EnvelopeModel.cs: RecordEnvelope<T>, RenamedMarkerEnvelope<T>,
AlternateEnvelope<T>. TestJsonContext registers them.

AOT canary (PopcornAotExample):
- Declares AotCustomEnvelope<T> with [PopcornEnvelope] + slot markers.
- Wires AddPopcorn(o => o.EnvelopeType = ...), AddPopcornEnvelopes, and
  UsePopcornExceptionHandler. Adds /boom endpoint to exercise the custom error
  path end-to-end under PublishAot=True.

Memory bank:
- systemPatterns.md: envelope dispatch architecture (build-time + runtime +
  middleware), extended Key Components, envelope discovery contract,
  Middleware Constraints section, JSG003-JSG007 + exception-middleware surfaces.
- techContext.md: solution layout includes Popcorn.SourceGenerator.Tests and
  new Popcorn.Shared files; generator entry points cover AnalyzeEnvelope,
  ReportEnvelopeDiagnostics, IsPopOfT/IsApiError, OpenGenericCSharpName,
  AddPopcornEnvelopes, naming-policy-aware writer; testing strategy documents
  the CSharpGeneratorDriver harness.

roadmap.md (new, repo root): status snapshot, Tier-1 ([SubPropertyDefault]),
Tier-2 ([Translator], IPopcornBlindHandler, [ExpandFrom]), known bugs,
polymorphism partial, infra (benchmarks, AOT CI, NuGet), open questions,
suggested sequence, and merge-to-master gate checklist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CreateDictionarySerializer was reading firstRef.Children on the first
sibling of value.PropertyReferences to decide what include list to pass
to each dictionary value. The parser eagerly sets Children =
PropertyReference.Default on every parsed name, so the old code
couldn't distinguish "no children" from a real sibling list and
silently collapsed to Default. ?include=[Dict[Id,Name]] therefore
rendered each value with !default rules, not [Id, Name]. Siblings,
wildcards, and negations inside dictionary-value brackets were all
dropped.

Pass value.PropertyReferences through verbatim. Covered by four new
tests in DictionaryTypesTests.cs (explicit subset, wildcard, negation,
nested-dictionary propagation) that all fail against the old code.

The companion parser test in IncludeParserEdgeTests.cs was a false
alarm — the parser produces the correct tree for [Dict[Value[Name]]].
Un-skipped and strengthened its assertions to walk the full tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ve matrix

Baseline audit of NRT × value-type nullability × position (scalar /
list-element / dict-value) × container-nullability × element-nullability
surfaced four interacting generator defects. All fixed here, all under
one coherent commit:

Bug 3 — Pop<T?> vs Pop<T> signature mismatch (62 × CS8620).
  The generator's converter-name canonicalization stripped NRT `?`
  annotations via NameType, but the actual callsites used raw
  ToDisplayString() which preserved them. So the method signature
  `Pop<List<int>>` didn't match the callsite `Pop<List<int>?>` when a
  property was declared nullable. Fix: new TypeNameForPop helper uses a
  SymbolDisplayFormat without IncludeNullableReferenceTypeModifier —
  NRT ? on ref types vanishes while Nullable<T> on value types is
  preserved (distinct CLR identity). All Pop<T> type-argument emissions
  route through it.

Bug 4 — primitive registration cross-contamination.
  `[JsonSerializable(typeof(ApiResponse<int?>))]` added "int" to the
  generator's allTypeNames set without emitting a Pop<int> method body.
  Every downstream list/dict/array converter that checked
  `allTypeNames.Contains(...)` then decided "yes, recurse via Pop" and
  emitted a call to a non-existent `PopSystemInt32`. One root-level
  primitive registration cascaded into compile errors across unrelated
  converters. Fix: IsBlindSerializableType predicate covers
  primitives/enums/ignored types (including Nullable<T> wrappers);
  excluded from allTypeNames; blind-serializable target types emit a
  direct JsonSerializer.Serialize path.

Bug 5 — IDictionary / ReadOnlyDictionary broken iterator dispatch.
  IDictionaryTypeName constant used "<TKey,TValue>" (no space) but
  Roslyn's ToDisplayString emits "<TKey, TValue>" (with space), so
  InheritsOrImplements never matched. Dictionary<K,V> was caught by a
  fallback OriginalDefinition check, but IDictionary<K,V> and
  ReadOnlyDictionary<K,V> as target types fell through to the
  IEnumerable branch — iterating KeyValuePair<K,V> while treating K as
  the item type. Fix: one-character whitespace edit in the constant.

Bug 6 — RegisterConverters.g.cs missing #nullable enable.
  Emitted JsonNamingPolicy? in a signature without the directive,
  producing CS8669. Fix: prepend #nullable enable to the file.

Coverage
- NullabilityCoverageModel.cs + NullabilityCoverageTests.cs (26 tests):
  scalar, list, array, dict, nested-container permutations × int / int?
  / string / string? / struct / struct? / class / class? × container-
  nullable variants. Null-element cases, [Default] propagation through
  every nullable shape, and root-level ApiResponse<T> where T is
  int? / string? / NullStruct? / List<int?> / Dict<string,int?> /
  NullClass / NullClass?.
- NullabilityDiagnosticsTests.cs (8 generator-driver tests): asserts
  zero CS86xx warnings on a matrix-shaped source, enforces both dedup
  invariants (NRT on ref types collapses to one converter) and the
  contra-invariant (Nullable<value-type> gets a separate converter).
- GeneratorTestHarness extended with RunAndCompile for post-generator
  compile-diagnostic assertions.

Suite
  Functional: 178 passing / 17 skipped / 0 failing  (was 170/25/0).
  Generator-driver: 14 passing / 0 failing  (was 13/1).
  CS86xx warnings in generated code: 64 → 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vention notes

Follow-up to 3c7c890. Three small changes from the code-review round:

- Removed the dead `Dictionary<TKey, TValue>` OriginalDefinition
  fallback in GenerateJsonConverter. After the IDictionaryTypeName
  whitespace fix, the IDictionary branch catches every
  Dictionary<K,V> too — the fallback is unreachable.

- Strengthened the two dedup generator-driver tests. Previously they
  only asserted "exactly one converter file emitted" for
  {Thing, Thing?} and {List<Th>, List<Th?>}. Now they also assert the
  generated source contains `Pop<Thing>` (not `Pop<Thing?>`) — catches
  a future regression where someone re-introduces raw ToDisplayString()
  at the Pop<T> emission site before CS8620 shows up at consumers.

- Documented TypeNameForPop and IsBlindSerializableType as load-bearing
  generator conventions in systemPatterns.md, plus the ", " whitespace
  requirement on Roslyn-matched type-name constants. Three
  low-severity findings from the code review (pragma scope,
  non-generic-subclass-of-Dictionary latent crash, stringly-typed
  hashset fragility) are captured under Known Issues > Deferred in
  activeContext.md.

All 178 functional + 14 driver tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dropped the "Known bugs (all FIXED)" audit and the completed
merge-gate checkboxes. Historical context lives in
memory-bank/progress.md and git log.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the SubPropertyDefaultAttribute on properties/fields so a type
declares the include list to use when the property is included without
explicit sub-children. Generator pre-parses the string once per process
into a `private static readonly` field and substitutes it at the two
nested-Pop<T> callsites (complex member, complex-array element) whose
default was previously PropertyReference.Default. ReferenceEquals against
the Default singleton is the clean signal for "no explicit sub-children".
Recursive by construction; [Always]/[Never] on the sub-type still win;
explicit sub-children still override.

Flips all 4 previously-skipped SubPropertyDefaultTests to passing
(182/13/0 functional, 14/0 driver). AOT example still builds clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a fair-fight comparison to the serialization benchmark: separate
`Stj_Reflection`, `Stj_SourceGen`, and `Popcorn{Default,All,Custom}`
paths so the STJ metadata-source-gen resolver is measured against
Popcorn's own source-gen converters directly, not against reflection.

Captures a baseline under `benchmarks/results/v2-baseline/` with BDN's
github-markdown report, CSV, and a summary README documenting headline
findings:

- STJ source-gen ≈ STJ reflection at these model sizes.
- Popcorn default (selective) is ~8× faster / ~10× less alloc than STJ
  on ComplexModelList (the selective-fetch thesis).
- Popcorn `!all` is at parity (0.97× time) on ComplexModelList.
- Flat-simple with `!all` is 1.8× — acceptable envelope tax.

Also makes `Program.cs` non-blocking in non-interactive shells (guards
the trailing ReadKey). `BenchmarkDotNet.Artifacts/` working dirs are
gitignored; only curated `benchmarks/results/` is tracked.

Legacy `PopcornNetStandard` (reflection) 3-way comparison still
pending — tracked in roadmap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires PopcornNetStandard (runtime-reflection engine) into the
SerializationPerformance benchmark alongside the existing STJ and
Popcorn source-gen paths, replaces the 2-way baseline report with
a 3-way version, and closes the corresponding merge-gate item.
Headline: Popcorn source-gen beats legacy reflection in every cell
(3-8x faster, 3-10x less allocation); Popcorn-default on
ComplexModelList is ~4.7x faster than legacy-default — v2 migration
has no regression scenario.

Legacy config notes documented in the README: AlwaysInclude left
empty (latent ArgumentException collision when combined with `!all`
or any include list naming the Always field); `!all` replaced with
explicit property enumeration for LegacyAll. Both workarounds yield
identical output for the benchmark models.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#1 Replace LINQ Any/FirstOrDefault in the emitted converter body with
index-based for-loops. Same complexity, no per-call delegate dispatch
or iterator machinery. Biggest time win on complex objects (up to -29%).

#2 Hoist useAll/useDefault/naming out of the inner complex-object body
into a new Pop{X}Inner overload. List and dictionary converters now
compute these once before the foreach and pass them in per item
instead of re-scanning PropertyReferences and allocating a naming
delegate per iteration. Big allocation win on list scenarios (-10% to
-22% across Default/All/Custom) and further time improvements.

#3 Skip the HashSet<object> allocation at the converter entry when
the target type's transitive property graph is provably cycle-free.
A new IsConverterCycleSafe DFS classifies types the generator
recursively walks; cycle-safe types pass null through instead of
allocating a fresh HashSet, and the body uses null-conditional
guards. Cycle-risky types (e.g. ComplexNestedModel with a Child
back-reference, CircularReferenceModel, DeepNestingModel) are
unchanged — verified by inspecting generated output. Biggest single-
object-scalar win.

Cumulative vs pre-optimization baseline on DefaultJob:
- SimpleModel_PopcornDefault: -22% time, -30% alloc
- SimpleModelList_PopcornDefault: -24% time, -18% alloc
- SimpleModelList_PopcornAll: -19% time, -12% alloc
- ComplexModel_PopcornAll: -32% time
- ComplexModelList_PopcornAll: now 0.87x Stj (was 0.97x) — crossed
  parity, Popcorn is now faster than STJ when emitting everything
  on nested data.

All 182 functional tests still pass; 14 generator unit tests still
green. Walk-through and raw per-step logs under
benchmarks/results/v2-baseline/opt-iterations/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-step BDN output captured during the three-pass optimization run
so the README's incremental tables are reproducible from the source
data. Renamed from .log (gitignored) to .txt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflect the LINQ→for, hoist-flags (Pop{X}Inner split), and HashSet-
elision changes in the docs:

- systemPatterns.md: add IsConverterCycleSafe + FlagSetupCode/Inner
  split to the load-bearing-conventions section so future contributors
  know these are structural, not ad-hoc.
- techContext.md: update the Source Generator Entry Points list with
  UnwrapPayloadType, IsConverterCycleSafe, IsNamedTypeCycleSafe, the
  CreateComplexObjectInnerBody rename, and the emitInnerOverload flag.
- progress.md: new subsection documenting the three optimizations and
  their incremental effects; updated Migration Thesis numbers to
  reflect the post-optimization 3-way ratios (Popcorn faster than STJ
  on ComplexModelList_All).
- activeContext.md: refreshed recent-activity commit list; added two
  deferred-quality notes (cycle-safety conservatism for unregistered
  types, and the three remaining perf levers enumerated in the opt-
  iterations README).
- roadmap.md: refreshed benchmark-report headline to the new ratios.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface the v2 benchmark results in the docs users actually read.

- New docs/Performance.md walks through the benchmark story in plain
  language: what was measured, headline table (ratios vs raw STJ for
  the four payload shapes), why v2 is faster, caveats, and how to
  reproduce. Links out to the detailed baseline under
  benchmarks/results/v2-baseline/ for full numbers and methodology.
- README.md gets a short Performance section under "Why would I use
  it?" with the three most striking highlights (~10x faster on
  selective fetch, faster than STJ even when emitting everything on
  nested data, beats legacy v1 engine in every cell) and a link to
  the full Performance page.
- Added to docs/TableOfContents.md and README "Further Reading" list.

Numbers quoted match the post-optimization 3-way baseline in commit
373f387 (DefaultJob, .NET 9.0.15).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… to roadmap

- Generator: new JSG008 warning when a serialized member is typed as
  object, abstract class, or interface (checked after unwrapping
  arrays / IEnumerable<T> / IDictionary<K,V> / Nullable<T>). Surfaces
  the only genuine AOT non-starter case at build time.
- Tests: five new cases in EnvelopeDiagnosticsTests (object, interface,
  abstract class, List<object>, concrete-type negative). Full
  SourceGenerator.Tests suite: 19 passing.
- Docs: new docs/MigrationV7toV8.md covers attribute renames, dropped
  features, DI replacement for SetContext, middleware+envelope
  replacement for SetInspector, include-wire-name contract, JSG008
  remediation, and a 13-item checklist. Linked from README and TOC.
- Roadmap: document the Pop{X}Inner regression introduced in 03ff6a5
  as the top merge-gate fix (solution currently does not build clean);
  promote the 5 deferred-quality items and 3 remaining perf levers
  from activeContext.md; add legacy-deprecation-timeline and
  example-refresh items; reflect JSG008 + migration guide as shipped.
- memory-bank: activeContext references the Pop{X}Inner regression;
  defer-quality list collapsed into pointers to the roadmap entries;
  systemPatterns error-surfaces section documents JSG008.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The step-2 generator optimization (03ff6a5) split every complex-object
converter into a 4-arg Pop{X} wrapper + a 6-arg Pop{X}Inner body so
list/dict converters could hoist useAll/useDefault/naming ONCE before
their foreach and call Pop{X}Inner per item. But the Inner overload is
only emitted when emitInnerOverload=true, which happens only in the
complex-object branch of GenerateJsonConverter. Collection / dict /
Nullable<T> / blind targets leave emitInnerOverload=false.

CreateArraySerializer and CreateDictionarySerializer were calling
Pop{X}Inner unconditionally once the item/value type was in
allTypeNames, so payloads containing List<List<X>>, Dict<K,List<X>>,
Dict<K,Dict<K,X>>, List<Nullable<UserStruct>>, etc. produced 7 CS0103
errors at consumer build time.

Fix: new TargetEmitsInner(ITypeSymbol) helper duplicates the dispatch
check (not blind, not array, not Nullable<T>, not IDictionary,
not IEnumerable). Array/dict serializers gate on it: fast path
(Pop{X}Inner + hoisted flags) when true; 4-arg Pop{X} wrapper
fallback when false. The fallback re-computes useAll/useDefault/naming
per item — exactly the pre-03ff6a5 behavior for these shapes only.

Results:
- dotnet build dotnet/Popcorn.sln -c Release: 0 errors (was 7).
- FunctionalTests: 182 passing / 13 skipped / 0 failing (baseline).
- SourceGenerator.Tests: 19 passing (unchanged).
- Generated SystemCollectionsGenericListSystemCollectionsGenericListSystemInt32JsonConverter.g.cs
  now calls PopSystemCollectionsGenericListSystemInt32(...) instead of
  the non-existent ...Inner variant.

Perf impact: none on the hot paths (list-of-user-class, dict-of-user-
class keep the Pop{X}Inner fast path). Only nested-collection shapes
lose the per-item hoist — those shapes were never in the benchmark
suite and had no shipped baseline to regress from.

roadmap.md and memory-bank/activeContext.md updated: merge-gate item
closed, Known Issues back to "no open bugs".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The v7 MapEntityFramework pattern intercepted serialization (serializer saw
S, emitted P); [ExpandFrom] as specced only emitted a From(TSource) factory,
so it wasn't clean parity. The three real use cases have cleaner answers:
[Never] on internal source properties, a hand-written 3-line factory, or
Mapster.SourceGenerator for complex mapping.

- Delete ExpandFromTests.cs (4 skipped tests). FunctionalTests: 182 / 9 / 0.
- Rewrite docs/MigrationV7toV8.md §7 to document the three replacement patterns.
- Remove [ExpandFrom] from roadmap Tier-2, apiDesign.md attribute surface,
  migrationAnalysis.md feasibility ledger, and activeContext/progress next-steps.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NicholasMTElliott and others added 14 commits April 23, 2026 20:57
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lindHandler

Same reasoning as the [ExpandFrom] drop — the planned Tier-2 features each
have cleaner answers using patterns already native to ASP.NET Core + STJ:

- [Translator] with DI: fires N+1 queries when serializing collections,
  moves I/O into the response-writing path, and forces IServiceProvider
  threading through JsonSerializerOptions (not natively supported).
  Replacement: resolve services at the endpoint, populate the DTO,
  serialize. MigrationV7toV8.md §5 documents the pattern.

- IPopcornBlindHandler<TFrom,TTo>: redundant with standard
  JsonConverter<T> registration, which STJ already supports under AOT.
  Popcorn composes transparently (falls through to JsonSerializer.Serialize
  for unknown types). MigrationV7toV8.md §8 documents the pattern.

Remove the 3 skipped tests in TranslatorTests.cs (keep the 3 passing
computed-property tests). Delete BlindHandlerTests.cs entirely. Rewrite
migration guide §5 and §8. Clear Tier-2 from roadmap + memory-bank.

FunctionalTests: 182 passing / 2 skipped / 0 failing (the 2 remaining
skips are polymorphism-dispatch tests — Tier-2-deferred, not dropped).
v2.0 is now feature-complete. Remaining merge gates are both infra:
AOT CI job + NuGet packaging.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Package IDs: Skyward.Api.Popcorn.SourceGen (analyzer) +
Skyward.Api.Popcorn.SourceGen.Shared (runtime library). Side-by-side
installable with legacy Skyward.Api.Popcorn v7. First preview:
8.0.0-preview.1.

Popcorn.Shared.csproj — adds PackageId/Version/Authors/Description/
Tags/ProjectUrl/RepositoryUrl/License/Readme/Copyright, SourceLink via
Microsoft.SourceLink.GitHub, IncludeSymbols=true + snupkg format,
EmbedUntrackedSources, PublishRepositoryUrl. Packs LICENSE + README
at package root.

Popcorn.SourceGenerator.csproj — same metadata, but DevelopmentDependency=
true + SuppressDependenciesWhenPacking=true so the package flows as
analyzer-only with no runtime deps. Embeds Popcorn.Shared.dll into
analyzers/dotnet/cs/ (required for Roslyn attribute symbol resolution)
but NOT into lib/netstandard2.0/ (consumers get runtime types from the
separate Shared package).

.github/workflows/main.yml — extends existing legacy-v7 pack+push with
two additional dotnet pack steps for the v8 packages; fetch-depth: 0
so SourceLink can resolve commit hashes. Push step unchanged (globs
*.nupkg).

MigrationV7toV8.md — final package IDs and version (8.0.0-preview.1) in
the packages/using section.

Verified locally via dotnet pack:
- Skyward.Api.Popcorn.SourceGen.Shared.8.0.0-preview.1.nupkg (16 KB) +
  .snupkg (10 KB). Contains lib/netstandard2.0/Popcorn.Shared.dll, deps.
- Skyward.Api.Popcorn.SourceGen.8.0.0-preview.1.nupkg (38 KB). Contains
  analyzers/dotnet/cs/{Popcorn.SourceGenerator.dll, Popcorn.Shared.dll},
  no lib/, no transitive dependencies. developmentDependency=true set.
- Both nuspec files carry repository commit hash (SourceLink active).

Merge-gate NuGet item: code-complete. Remaining work is operational:
smoke-test install from a throwaway consumer project, tag 8.0.0-preview.1,
push tag, CI publishes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- docs/Releases.md: add 8.0.0-preview.1 entry with package IDs, highlights,
  and abridged breaking-changes summary.
- roadmap.md: mark smoke-test and Releases.md items done. Remaining NuGet
  work is operational (tag + push).
- memory-bank/activeContext.md: backfill commits 6c437cb, be55b65, efb05b9
  into Recent Activity.

Smoke-test ran a throwaway net9.0 classlib consumer with PackageReferences
to both packages from a local feed. Restore succeeded, generator ran,
emitted SmokeConsumerCarJsonConverter.g.cs +
SystemCollectionsGenericListSmokeConsumerCarJsonConverter.g.cs +
RegisterConverters.g.cs. STJ's own source generator picked up the emitted
Pop<Car> / Pop<List<Car>> types. Build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New workflow .github/workflows/aot-ci.yml runs on PR + push to master
and spike/** branches. Builds the PopcornAotExample Dockerfile via
docker/build-push-action with GitHub Actions cache, starts the
container, waits for readiness, and asserts all four endpoints:

- /todos      Success:true, Id:1+Id:2, [Never] IsComplete absent.
- /null       Data:null in the envelope.
- /sub        Id:1 + nested ToDo object.
- /boom       status 500, Ok:false, Problem populated with the
              exception message. Exercises the UsePopcornExceptionHandler
              middleware + generator-emitted custom-envelope error
              writer (PopcornErrorWriterRegistry dispatch path).

concurrency group cancels superseded runs on the same ref. Failure
dumps docker logs; teardown always stops the container.

Endpoint assertions verified locally (JIT mode — docker daemon wasn't
available on the dev box). Actual response shapes captured:
  /todos  {"Success":true,"Data":[{"Id":1,"ToDo":null,"Title":"..."},{"Id":2,"ToDo":{...},"Title":null}]}
  /null   {"Success":true,"Data":null}
  /sub    {"Success":true,"Data":{"Id":1,"ToDo":{"i":1,"j":2,"k":3},"Title":"..."}}
  /boom   status=500 {"Ok":false,"Problem":{"Code":"InvalidOperationException","Message":"aot boom"}}

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Before this: the 201 tests (182 FunctionalTests + 19 SourceGenerator.Tests)
only ran on developer boxes. A regression in the generator or runtime
could land on spike/source-generator (or master) without the automated
suite catching it. The existing main.yml only triggers on release tags
(and only packs NuGet); aot-ci.yml runs only the AOT Docker smoke.

.github/workflows/tests.yml — triggers on PR + push to master / spike/**.
Installs .NET 8.0 SDK (test projects target net8.0, so no DOTNET_ROLL_
FORWARD needed). Caches NuGet packages keyed on csproj hashes. Runs
dotnet test on both test projects individually rather than
'dotnet test Popcorn.sln' because the solution also contains
PopcornNet5Example (net5.0, EOL — no targeting pack on modern SDKs)
and PopcornAotExample (covered by aot-ci.yml; needs the AOT toolchain).

Uploads trx results as an artifact on failure. Concurrency-group
cancels superseded runs on the same ref.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Going forward, only Skyward.Api.Popcorn.SourceGen and
Skyward.Api.Popcorn.SourceGen.Shared ship from this repo. The legacy v7
packages (Skyward.Api.Popcorn + Skyward.Api.Popcorn.DotNetCore) remain
on NuGet for install compatibility but are no longer published from
CI. Removes the Build/Package steps for PopcornNetStandard and
PopcornNetStandard.WebApiCore. Also bumps checkout action from v2 to v4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GitHub runners vary ±20–30% in absolute ns/op (cold VMs, noisy
neighbors, CPU variance), but the ratio between two benchmarks on
the same run is stable because both numerator and denominator see
the same noise. So this gate snapshots RATIOS, not wall-clock times.

Three load-bearing ratios tracked (Popcorn / STJ source-gen, same shape):

  SimpleModelList_PopcornAll_vs_Stj     = 1.492  (worst case)
  ComplexModelList_PopcornAll_vs_Stj    = 0.872  (headline — Popcorn < STJ)
  ComplexModelList_PopcornDefault_vs_Stj = 0.111  (selectivity win)

New pieces:
- dotnet/benchmarks/SerializationPerformance/Program.cs: new 'ci' case
  filters to the 5 backing benchmarks (3 ratios × ~2 operands each,
  sharing the Stj_SourceGen denominator) using the class's default
  SimpleJob (3 warmup + 15 iterations). ~2 min wall time. ShortRun
  explicitly not used: the denominator (Stj_SourceGen) jumps ~30% on
  cold iterations, squashing the ratio and producing false signals.

- benchmarks/results/ci-baseline.json: committed snapshot with
  thresholdPercent=25. Update discipline: when a real perf change
  lands, re-run 'ci' locally and commit the new baseline in the same
  PR; casual regressions don't bump the baseline, they surface as
  CI failures (the gate working).

- .github/scripts/compare-benchmark-ratios.py: reads the BDN
  *-report-full.json, computes each ratio, compares to baseline.
  Exit 1 on regression > threshold, exit 0 with NOTE on improvement
  > threshold (reminds you to bump the baseline to tighten the gate).
  Verified locally against three cases: unchanged (PASS), +50%
  regression (FAIL), -50% improvement (PASS with NOTE).

- .github/workflows/benchmarks.yml: PR + push trigger, NuGet cache,
  renders the BDN markdown report + ratio-gate output to
  $GITHUB_STEP_SUMMARY, uploads raw BDN artifacts for 30 days.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Not a CI gate — runtime-specific installs (SDKs, ILCompiler packages, VS C++
build tools for AOT on Windows) are too machine-specific to bake into a
shared pipeline. This is an ad-hoc local tool for questions like "did .NET 9
regress us?", "did .NET 10 help?", "is AOT faster than JIT for our workload?".

Two pieces:

1. dotnet/benchmarks/MatrixPerformance/ — a separate benchmark project
   (multi-targets net8.0;net9.0;net10.0) that runs 5 of the same benchmarks
   the CI ratio gate measures. AOT-safe (no PopcornNetStandard legacy ref,
   InProcessNoEmitToolchain, TrimmerRootAssembly pins for MatrixPerformance,
   Popcorn.Shared, and BenchmarkDotNet). PublishAot is csproj-local (not
   CLI-level) to avoid cascading NETSDK1207 into netstandard2.0 refs.

2. benchmarks/matrix/ — orchestration scripts. run-matrix.sh iterates the
   6-cell matrix as separate dotnet run / dotnet publish invocations (per-TFM
   for JIT; per-TFM + AOT publish for AOT). summarize-matrix.py aggregates
   the per-cell BenchmarkDotNet JSON into two markdown tables: absolute
   means and Popcorn/STJ ratios.

README documents setup gotchas encountered along the way:
- Passing -p:PublishAot=true on the CLI cascades to ProjectReferences and
  trips NETSDK1207 on netstandard2.0 libs. Put it in the csproj.
- vswhere.exe must be on PATH on Windows for ILCompiler to find link.exe.
- BDN 0.14's CommandLineParser breaks under AOT (immutable-type reflection).
  Call BenchmarkRunner.Run<T>(config) without args.
- BDN's default toolchain spawns child dotnet processes; single-file AOT
  binaries can't. Use InProcessNoEmitToolchain (not Emit — Reflection.Emit
  is blocked under AOT).
- Trimmer strips [Benchmark]-tagged methods and BDN's Runnable type.
  Use TrimmerRootAssembly entries.

Sample results (from my dev box, 2026-04-23) embedded in the README:
- JIT-net10 is 10-20% faster than JIT-net9 across the board (the .NET 10
  improvements are real).
- Popcorn's worst-case ratio tightens from 1.55 to 1.39 on JIT-net10 — the
  generator's code shape benefits disproportionately from the new JIT.
- AOT is 1.7-2x slower than JIT for this workload — steady-state serialization
  favors RyuJIT tier-1 + PGO over static compilation. AOT wins elsewhere
  (startup, memory footprint) but not here.
- Popcorn/STJ ratios stable across all 5 working cells: 1.4-1.5 worst-case,
  0.79-0.87 headline, 0.09-0.10 selectivity. Confirms the 25% CI threshold
  is comfortably above runtime-variance noise.

Results directories gitignored (machine-specific).
MatrixPerformance added to Popcorn.sln.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
With .NET 8 runtime now installed the 6th cell (JIT-net8.0) fills in, and
the complete picture reveals something the 5-cell version missed:

.NET 9 *did* regress Popcorn's worst-case ratio (SimpleModelList PopcornAll
/ Stj): 1.38 on net8 → 1.55 on net9 → 1.39 on net10. Absolute times got
faster at every transition (STJ and Popcorn both benefited from 8 → 9 →
10), but 9's improvements favored STJ over Popcorn, widening the ratio.
.NET 10 closed that gap again. Consistent with reports of .NET 9 shipping
STJ-source-gen optimizations that didn't flow through to plug-in
JsonConverter<T> registrations (how Popcorn hooks in).

Headline and selectivity ratios are runtime-agnostic (0.79-0.87 /
0.08-0.11 across all 6 cells). The CI gate's 25% threshold survives
comfortably across every cell we measured.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same-repo PRs fired both push and pull_request events, so tests/AOT/benchmarks
each ran twice per commit. Keep push only on master (for merge coverage); let
pull_request cover spike/** branches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NicholasMTElliott
NicholasMTElliott merged commit e425688 into master Apr 24, 2026
3 checks passed

This branch was previously deployed

1 inactive deployment
github-pages — e425688b Deployed Apr 24, 2026 by NicholasMTElliott via deploy #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant