Skip to content

perf(observable-events): rebuild the generator as a cached 4.8 pipeline - #165

Merged
glennawatson merged 1 commit into
mainfrom
feat/observable-events-incremental-pipeline
Aug 16, 2026
Merged

perf(observable-events): rebuild the generator as a cached 4.8 pipeline#165
glennawatson merged 1 commit into
mainfrom
feat/observable-events-incremental-pipeline

Conversation

@glennawatson

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Refactor and performance, with two fixes: the generator would not load on the .NET 8 SDK, and its generated handlers raised CS8622.

What is the new behavior?

The observable-event generator is an incremental pipeline that actually caches, built against Roslyn 4.8 so every supported SDK can load it.

  • It loads on the .NET 8 SDK. Roslyn is pinned to 4.8.0 and the explicit System.Collections.Immutable reference is gone, so the dependency set is whatever the consumer's compiler already carries.
  • Generated handlers carry the delegate's own nullable annotations, so a wrapper around EventHandler<T> no longer warns.
    • Gated on the consumer's language version: a C# 7.3 consumer gets no annotations and no #nullable enable, which is source it can compile.
  • Nothing but strings crosses the pipeline. No symbol, syntax node, compilation, or Location survives into a cached step; diagnostics and source locations are carried as values and rebuilt when reported.
  • Each output is keyed on the smallest model that decides it - one file per host, one per namespace of static events, one for the activation overloads - so a change to one host leaves the rest alone.
  • The activation API is an ordinary source output rather than post-initialization output. Post-initialization source is added to the compilation the pipeline then runs against, which makes that compilation new on every run and discards every semantic result cached against the previous one. A single inert post-initialization file costs the entire cache, so the generator emits none.
  • Requests are detected without binding to that API, which is no longer visible to the pipeline that produces it.
    • An Events() call that resolves belongs to somebody else and is skipped; one that does not is ours to answer.
    • The static request attribute is matched on how it is written, and the host comes from the typeof argument, which binds on its own.
  • Emission runs on a pooled character buffer instead of StringBuilder.

Generated output is unchanged apart from three fixes: <see cref="Foo<T>"/> in a documentation comment was malformed XML and is now <c>Foo&lt;T&gt;</c>, blank lines no longer carry trailing whitespace, and files end with a newline.

What is the current behavior?

  • Built against Roslyn 4.14, so the .NET 8 SDK compiler refused to load it (CS9057) and consumers on that SDK got no generated source at all - the build failed on the missing activation API.
  • Generated void Handler(object sender, ...) against delegates declaring object?, so every wrapped EventHandler<T> raised CS8622.
  • One 1027-line file collected every call site, combined the result with the compilation, and regenerated every file from a single source output whenever anything in the compilation changed.

What might this PR break?

  • The package's dependency floor moves to Roslyn 4.8. This widens support rather than narrowing it; nothing that worked before stops working.
  • An Events() call that binds to an unrelated method is still skipped, but the test is now inverted. A call that resolves to a method outside the activation class is left alone; a call that resolves to nothing is treated as a request. An ambiguous Events() call (CS0121) resolves to nothing and would now be treated as a request.
  • Consumers below C# 8 now get un-annotated source with no #nullable enable. Previously that directive was emitted unconditionally, which is a compile error before C# 8, so this only fixes those consumers.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

Review the pipeline wiring and the two extractors; the rest is mechanical decomposition of the old file.

  • EventGenerator.cs - pipeline shape and why the activation API is a source output.
  • Helpers/InstanceTargetExtractor.cs and Helpers/StaticTargetExtractor.cs - request detection without the activation API in scope.
  • Models/EquatableArray.cs - value equality is the cache key; a defaulted instance and a zero-length one have to compare equal.

A new ReactiveUI.Primitives.ObservableEvents.Benchmarks project drives the generator through CSharpGeneratorDriver over a corpus of 1, 10, and 50 event-bearing hosts, one host per file. Unchanged re-runs a primed driver against the very compilation it was primed on and is the control the incremental cases are read against.

Size Cold Unchanged UnrelatedEdit EventEdit
1 host 164 us 6.4 us (0.04x) 117 us (0.71x) 148 us (0.90x)
10 hosts 687 us 9.8 us (0.01x) 476 us (0.69x) 527 us (0.77x)
50 hosts 2871 us 26 us (0.009x) 2072 us (0.72x) 2128 us (0.74x)

An edit still costs about 0.72x a cold run and scales with the corpus. Roslyn re-runs every semantic syntax-provider transform whenever the compilation changes at all, and a CPU trace puts roughly 39 percent of that in the compiler binding the Events() call sites. That is intrinsic to keying generation on the receiver type of a call, and no pipeline shape avoids it; removing it would mean keying generation on a declaration instead.

- Split the 1027-line generator into models, extraction, and emission, matching
  the layout of ReactiveUI.Binding.SourceGenerators.
- Carry only value-equatable string models through the pipeline, so no symbol,
  syntax node, or compilation survives into a cached step.
- Key each output on the smallest model that decides it: one file per host, one
  per namespace of static events, one for the activation overloads.
- Emit the activation API as an ordinary source output. Post-initialization
  output is added to the compilation the pipeline runs against, which discards
  every cached semantic result; one inert file costs the whole cache.
- Detect requests without binding to that API, which is no longer visible to the
  pipeline: an Events() call that resolves belongs to someone else, and the
  static request attribute is matched on how it is written.
- Pin Roslyn to 4.8.0 and drop the System.Collections.Immutable reference. The
  previous 4.14 build refused to load on the .NET 8 SDK, leaving consumers with
  no generated source at all.
- Emit handler signatures with the delegate's own nullable annotations where the
  consumer's language version allows it, fixing CS8622 on every EventHandler<T>.
- Replace StringBuilder emission with a pooled character buffer.

At 50 hosts: an unchanged re-run drops from 2871us to 26us, and an edit costs
0.72x a cold run.
@glennawatson
glennawatson merged commit 348b69f into main Aug 16, 2026
8 checks passed
@glennawatson
glennawatson deleted the feat/observable-events-incremental-pipeline branch August 16, 2026 02:59
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.14%. Comparing base (de69b7f) to head (2971490).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   98.09%   98.14%   +0.04%     
==========================================
  Files         680      703      +23     
  Lines       21359    21733     +374     
  Branches     2622     2679      +57     
==========================================
+ Hits        20953    21329     +376     
  Misses        199      199              
+ Partials      207      205       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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