Skip to content

perf(source-gen): close incremental caching gaps in static property and property injection generators - #6925

Merged
thomhurst merged 2 commits into
mainfrom
perf/generator-incremental-caching-gaps
Sep 29, 2026
Merged

thomhurst merged 2 commits into
mainfrom
perf/generator-incremental-caching-gaps

Conversation

@thomhurst

@thomhurst thomhurst commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

Two small incremental-caching fixes in TUnit.Core.SourceGenerator. They only affect how much work the generators redo in the IDE after an edit. Generated output does not change.

What changed

  • StaticPropertyInitializationGenerator: the pipeline filtered class chains with chain.Length > 0. That never removes anything, because every chain includes the class itself. As a result, every class with a base list or partial modifier went into .Collect(), and adding, removing or renaming any such class re-ran the parse step. The filter now keeps only chains where at least one segment has a static data-source property. Dropped chains contributed nothing to the deduplicated result, so the output is identical.
  • PropertyInjectionSourceGenerator: the concrete-generic-types transform returned a List<ConcreteGenericTypeModel>. List has reference equality, so that node was always Modified, and the flattened step re-ran on every compilation change. It now returns EquatableArray<ConcreteGenericTypeModel>; the element type already implements value equality.
  • I checked the other pipelines in this project for similar non-equatable step outputs: DynamicTestsGenerator, InfrastructureGenerator, ModuleInitializerPolyfillGenerator, and pipelines 1 and 2 of PropertyInjection. All of them already use equatable models or EquatableArray, so nothing else needed changing. AotConverterGenerator, TestMetadataGenerator and HookMetadataGenerator were left alone because other PRs cover them.

Measurements

These are IDE-only caching changes, so I measured tracked incremental step reasons rather than wall-clock time. Each row runs the generator, applies an unrelated edit, then re-runs it:

Scenario Tracked step Before After
Add class Other : IDisposable (no static data-source properties) ParseStaticProperties Unchanged (re-ran) Cached
Edit an unrelated property (int Plain → long Plain) PropertyInjection_ConcreteGenericTypes Unchanged (re-ran) Cached

In both scenarios the source outputs were already Cached. The gain is that the upstream steps no longer re-run and re-compare their values.

Testing done

  • Added AddUnrelatedClassWithBaseListShouldNotRerunParse and EditUnrelatedMemberShouldNotRerunConcreteGenericTypes to tests/TUnit.SourceGenerator.IncrementalTests. Both fail on main and pass with this change. The full incremental suite passes (51/51, run via dotnet vstest with the xunit adapter copied into bin, since the project is not run in CI).
  • tests/TUnit.Core.SourceGenerator.Tests (net10.0) passes: 172 succeeded, 0 failed, no snapshot changes.

Summary by CodeRabbit

  • Bug Fixes
    • Unrelated code edits no longer trigger unnecessary source-generator processing when generating property injections or initializing static properties. This helps keep incremental processing focused on relevant changes, avoiding extra work during development when unrelated properties or classes are edited.

…nd property injection generators

StaticPropertyInitializationGenerator filtered class chains with Length > 0, which never drops
anything because every chain includes the class itself. Filter to chains that contain a static
data-source property so unrelated classes with a base list stay out of the collected input.

PropertyInjectionSourceGenerator's concrete-generic transform returned a List, so its output was
always Modified. Return an EquatableArray so unchanged results are cached.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T08:35:50.805024Z d237ab3 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 028554b5-29cb-4746-9b45-fe59974dcc67

📥 Commits

Reviewing files that changed from the base of the PR and between 1345ddc and d237ab3.

📒 Files selected for processing (1)
  • src/TUnit.Core.SourceGenerator/CodeGenerators/StaticPropertyInitializationGenerator.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The static-property generator now filters inheritance chains that contain no properties. The property injection generator now returns concrete generic type results as an equatable array. Incremental tests check that unrelated edits leave the corresponding generator steps cached.

Changes

Source generator cache behavior

Layer / File(s) Summary
Filter static-property chains
src/TUnit.Core.SourceGenerator/CodeGenerators/StaticPropertyInitializationGenerator.cs, tests/TUnit.SourceGenerator.IncrementalTests/StaticPropertyInitializationGeneratorIncrementalTests.cs
The generator retains chains that contain properties. A test adds an unrelated class and checks that the parse output count remains 2 and the parse step is cached.
Use equatable generic type results
src/TUnit.Core.SourceGenerator/Generators/PropertyInjectionSourceGenerator.cs, tests/TUnit.SourceGenerator.IncrementalTests/PropertyInjectionSourceGeneratorIncrementalTests.cs
Concrete generic type discovery returns an EquatableArray<ConcreteGenericTypeModel>. A test changes an unrelated property and checks that the discovery step is cached.

Priority: ⚪ Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to d237a

The generator changes preserve qualifying static-property inputs and make generic discovery results comparable by value. No material merge risk was found in the reviewed changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: improving incremental caching in the static property and property injection source generators.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the chains with care,
And keeps the ones with properties there.
Arrays now compare in steady rows,
While cached steps stay as the edit flows.
The rabbit hops, then rests below,
As tidy generator results flow.

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Review

Both changes are correct and match the description.

  • StaticPropertyInitializationGenerator: The old chain.Length > 0 filter was a no-op because every chain contains the class itself. HasAnyProperty fixes that. A derived class whose base has static data-source properties still passes, since the chain includes the base segments, so the deduplicated output is unchanged.
  • PropertyInjectionSourceGenerator: Returning EquatableArray<ConcreteGenericTypeModel> instead of List<> gives value equality, so the node can report Cached and stop downstream re-runs. The .Count to .Length update is consistent.
  • Tests: The two new incremental tests assert the specific tracked steps, which is the right way to guard against regressions here.

Minor, non-blocking: .Where(static chain => HasAnyProperty(chain)) could be a method group (.Where(HasAnyProperty)). Use static on the method to keep the no-capture guarantee.

I couldn't run the tests here, and I didn't see the Codex summary. LGTM.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1345ddc9d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Optimizes incremental caching in code generators for test framework.

The PR appears safe to merge; no actionable issue was identified in the changes since the previous review.

Summary

This PR narrows the static-property generator’s collected input to chains containing a relevant property and gives the concrete-generic-types transform a value-equatable output. The latest change replaces an enumerator-based property check with an equivalent indexed loop.

Reviews (2) · Last reviewed commit: "perf(source-gen): use indexed loop in Ha..."

@github-actions

Copy link
Copy Markdown
Contributor

Review

Both changes look correct and well-scoped.

  • StaticPropertyInitializationGenerator: the old chain.Length > 0 filter was a no-op, and HasAnyProperty now drops chains with no static data-source property before .Collect(). The indexed loop avoids the boxed enumerator, and the reasoning that dropped chains contribute nothing to the deduplicated output is sound.
  • PropertyInjectionSourceGenerator: switching List<T> to EquatableArray<T> fixes the reference-equality problem. Empty, Length and ToEquatableArray all exist in EquatableArray.cs, and the .Where(x => x.Length > 0) update is correct.
  • Tests: both new incremental tests target the exact regressions, and the PR reports they fail on main.

Minor, non-blocking points:

  1. .Where(static chain => HasAnyProperty(chain)) can be a method group (.Where(HasAnyProperty)). Keep the lambda if you want it to stay static.
  2. The PR notes that TUnit.SourceGenerator.IncrementalTests isn't run in CI, so these guards won't catch regressions automatically. Wiring that project into CI would be worth a follow-up.

I couldn't run the tests here. The code-review skill also failed, so this review is manual. LGTM.

intellitect-bot pushed a commit to IntelliTect/EssentialCSharp.Web that referenced this pull request Sep 30, 2026
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.71.0 to
1.72.4.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.72.4

<!-- Release notes generated using configuration in .github/release.yml
at v1.72.4 -->

## What's Changed
### Other Changes
* fix(source-gen): stop parameter resolver keeping every non-public
test-class method (IL2111) by @​thomhurst in
thomhurst/TUnit#6937
### Dependencies
* chore(deps): update tunit to 1.72.0 by @​thomhurst in
thomhurst/TUnit#6934


**Full Changelog**:
thomhurst/TUnit@v1.72.0...v1.72.4

## 1.72.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.72.0 -->

## What's Changed
### Other Changes
* perf(source-gen): resolve parameter reflection info through a shared
runtime helper by @​thomhurst in
thomhurst/TUnit#6923
* perf(analyzers): trim remaining analyzer hot-path symbol lookups and
binds by @​thomhurst in thomhurst/TUnit#6928
* perf(mocks): move shared MockCall wrapper plumbing into runtime base
classes by @​thomhurst in thomhurst/TUnit#6929
* perf(source-gen): close incremental caching gaps in static property
and property injection generators by @​thomhurst in
thomhurst/TUnit#6925
* perf(source-gen): stop InfrastructureGenerator pinning an old
Compilation by @​thomhurst in
thomhurst/TUnit#6926
* perf(assertions-analyzers): cache assertion symbols and cut per-call
work by @​thomhurst in thomhurst/TUnit#6927
* perf(source-gen): emit hooks per class with direct, non-async bodies
by @​thomhurst in thomhurst/TUnit#6924
* test: fix flaky ObjectInitializer continuation-thread test by
@​thomhurst in thomhurst/TUnit#6932
* fix: CI flakes from leaked hook contexts, ActivityCollector race and
Repro5700 rendezvous by @​thomhurst in
thomhurst/TUnit#6933
* fix(aspnetcore): honor WebApplicationFactoryClientOptions in
CreateClient by @​thomhurst in
thomhurst/TUnit#6931
### Dependencies
* chore(deps): update tunit to 1.71.0 by @​thomhurst in
thomhurst/TUnit#6920


**Full Changelog**:
thomhurst/TUnit@v1.71.0...v1.72.0

Commits viewable in [compare
view](thomhurst/TUnit@v1.71.0...v1.72.4).
</details>

Updated [TUnit.AspNetCore](https://github.com/thomhurst/TUnit) from
1.71.0 to 1.72.4.

<details>
<summary>Release notes</summary>

_Sourced from [TUnit.AspNetCore's
releases](https://github.com/thomhurst/TUnit/releases)._

## 1.72.4

<!-- Release notes generated using configuration in .github/release.yml
at v1.72.4 -->

## What's Changed
### Other Changes
* fix(source-gen): stop parameter resolver keeping every non-public
test-class method (IL2111) by @​thomhurst in
thomhurst/TUnit#6937
### Dependencies
* chore(deps): update tunit to 1.72.0 by @​thomhurst in
thomhurst/TUnit#6934


**Full Changelog**:
thomhurst/TUnit@v1.72.0...v1.72.4

## 1.72.0

<!-- Release notes generated using configuration in .github/release.yml
at v1.72.0 -->

## What's Changed
### Other Changes
* perf(source-gen): resolve parameter reflection info through a shared
runtime helper by @​thomhurst in
thomhurst/TUnit#6923
* perf(analyzers): trim remaining analyzer hot-path symbol lookups and
binds by @​thomhurst in thomhurst/TUnit#6928
* perf(mocks): move shared MockCall wrapper plumbing into runtime base
classes by @​thomhurst in thomhurst/TUnit#6929
* perf(source-gen): close incremental caching gaps in static property
and property injection generators by @​thomhurst in
thomhurst/TUnit#6925
* perf(source-gen): stop InfrastructureGenerator pinning an old
Compilation by @​thomhurst in
thomhurst/TUnit#6926
* perf(assertions-analyzers): cache assertion symbols and cut per-call
work by @​thomhurst in thomhurst/TUnit#6927
* perf(source-gen): emit hooks per class with direct, non-async bodies
by @​thomhurst in thomhurst/TUnit#6924
* test: fix flaky ObjectInitializer continuation-thread test by
@​thomhurst in thomhurst/TUnit#6932
* fix: CI flakes from leaked hook contexts, ActivityCollector race and
Repro5700 rendezvous by @​thomhurst in
thomhurst/TUnit#6933
* fix(aspnetcore): honor WebApplicationFactoryClientOptions in
CreateClient by @​thomhurst in
thomhurst/TUnit#6931
### Dependencies
* chore(deps): update tunit to 1.71.0 by @​thomhurst in
thomhurst/TUnit#6920


**Full Changelog**:
thomhurst/TUnit@v1.71.0...v1.72.0

Commits viewable in [compare
view](thomhurst/TUnit@v1.71.0...v1.72.4).
</details>

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
Pull Requests — d237ab30 Deployed Sep 29, 2026 by thomhurst via modularpipeline (windows-latest) #19598
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