perf: eliminate per-class TestSource .ctor JITs via delegate registration

[thomhurst]

Summary

• Switches generated per-class TestSource classes from internal sealed class (implementing ITestSource + ITestDescriptorSource) to internal static class with static methods
• Adds a shared DelegateTestSource adapter in TUnit.Core that wraps Func<string, IReadOnlyList<TestMetadata>> and Func<IEnumerable<TestDescriptor>> delegates into the existing ITestSource/ITestDescriptorSource interfaces
• Registration now uses method groups (GetTests, EnumerateTestDescriptors) instead of new TestSource(), so no per-class constructor is JIT-compiled
• Eliminates ~1,000 unique .ctor JIT compilations in the 10,000-test repro (1,000 classes × 10 methods), replacing them with a single shared DelegateTestSource..ctor

JIT reduction: 15,000 → ~14,000 (~7% reduction), building on #5049.

Test plan

• TUnit.Core builds cleanly
• TUnit.Core.SourceGenerator builds cleanly
• TUnit.TestProject builds with 0 errors
• Source generator snapshot tests pass on all 4 TFMs (net472, net8.0, net9.0, net10.0)
• BasicTests and DataDrivenTests pass at runtime on net10.0

[claude]

Code Review: perf: eliminate per-class TestSource .ctor JITs via delegate registration

The core optimization is well-targeted and the implementation is clean. Here are my observations:

What's good

• JIT claim is valid: Moving from N unique internal sealed class types (each with its own .ctor the JIT compiles independently) to a single shared DelegateTestSource type is a real JIT win. The JIT compiles each unique generic/class .ctor separately, so N classes → N JIT compilations becomes 1.
• DelegateTestSource as an adapter is a clear, minimal pattern that preserves the existing ITestSource/ITestDescriptorSource interface contract everywhere the engine consumes these sources.
• AOT safety: Static classes with static methods are Native AOT–safe. Func<> delegates wrapping static methods have no reflection concerns.
• Snapshot coverage: All 4 TFM variants are consistently updated.

---

Concern: Net allocation cost went up, not down

Each call to the new Register overload allocates 3 objects instead of 1:

1. Func<string, IReadOnlyList<TestMetadata>> (method group → delegate)
2. Func<IEnumerable<TestDescriptor>> (method group → delegate)
3. DelegateTestSource (wrapping both)

vs. the old new TestSourceXxx() which allocated 1 object (a zero-field class, just an object header + vtable pointer).

The PR title says "eliminate per-class .ctor JITs" — that claim is accurate. But the description could be clearer that this trades JIT compilation overhead for slightly more startup allocations. Since all of this is at module-init time (not hot paths), the tradeoff is very reasonable — just worth calling out explicitly.

---

Architectural suggestion: skip DelegateTestSource entirely

If reducing allocations and JIT overhead both matter, there's a further step available: store the delegates directly in the Sources collections without the wrapper object:

Then the engine's consumption path would handle both collections. This avoids the DelegateTestSource allocation (3 objects → 2 per class). This is a more invasive change, so it may be worth a follow-up PR rather than blocking this one.

---

Minor: ITestDescriptorSource doc vs. implementation

The ITestDescriptorSource.EnumerateTestDescriptors doc says it "should be fast and allocation-free where possible". The DelegateTestSource implementation adds one level of delegate indirection compared to direct static method calls. Not a real concern in practice (module init only), just a slight inconsistency with the documented intent.

---

Summary

This is a solid, well-scoped optimization that correctly reduces JIT startup cost. The snapshot tests are all updated correctly, and the approach is AOT-compatible. The allocation trade-off is acceptable given this is startup-only. Consider the "store delegates directly" approach as a follow-up for a further win if allocation reduction becomes a priority.

Approved with the above observations noted.

[claude]

Code Review: perf: eliminate per-class TestSource .ctor JITs via delegate registration

The core optimization is well-targeted and the implementation is clean. Here are my observations:

What's good

• JIT claim is valid: Moving from N unique internal sealed class types (each with its own .ctor the JIT compiles independently) to a single shared DelegateTestSource type is a real JIT win. The JIT compiles each unique class .ctor separately, so N classes → N JIT compilations becomes 1.
• DelegateTestSource as an adapter is a clear, minimal pattern that preserves the existing ITestSource/ITestDescriptorSource interface contract everywhere the engine consumes these sources.
• AOT safety: Static classes with static methods are Native AOT–safe. Func<> delegates wrapping static methods have no reflection concerns.
• Snapshot coverage: All 4 TFM variants are consistently updated.

---

Concern: Net allocation count went up, not down

Each call to the new Register overload allocates 3 objects instead of 1:

1. Func<string, IReadOnlyList<TestMetadata>> (method group → delegate)
2. Func<IEnumerable<TestDescriptor>> (method group → delegate)
3. DelegateTestSource (wrapping both)

vs. the old new TestSourceXxx() which allocated 1 object (a zero-field class — just an object header + vtable pointer).

The PR title says "eliminate per-class .ctor JITs" — that claim is accurate. But the description could be clearer that this trades JIT compilation overhead for slightly more startup allocations. Since all of this happens at module-init time (not on any hot path), the tradeoff is very reasonable — just worth calling out explicitly.

---

Architectural suggestion: skip DelegateTestSource entirely

If reducing allocations and JIT overhead both matter, there's a further step available: store the delegates directly in the Sources collections without the wrapper object. Instead of wrapping in DelegateTestSource, the engine's consumption path could accept a parallel collection of raw delegate pairs. This would cut 3 allocations per class down to 2.

This is a more invasive change (touching the engine's consumption path), so it may be worth a follow-up PR rather than blocking this one.

---

Minor: ITestDescriptorSource doc vs. DelegateTestSource behaviour

The ITestDescriptorSource.EnumerateTestDescriptors doc says it "should be fast and allocation-free where possible". The DelegateTestSource implementation adds one level of delegate indirection compared to a direct static method call. Not a real concern in practice (module init only), but a slight inconsistency with the documented intent worth noting.

---

Summary

This is a solid, well-scoped optimization that correctly reduces JIT startup cost for large test suites. The snapshot tests are all updated correctly, and the approach is AOT-compatible. The allocation trade-off is acceptable given this is startup-only code. Consider the "store delegates directly without a wrapper type" approach as a follow-up for a further win if allocation reduction at startup becomes a priority.

Approved with the above observations noted.

[claude]

Code Review: perf: eliminate per-class TestSource .ctor JITs via delegate registration

Two previous reviews from claude[bot] were posted on this PR (likely from duplicate runs). The concerns raised in those reviews remain unaddressed, so I'll carry them forward and add a few new observations.

---

What's Good

• JIT reduction is real and correctly measured: N unique sealed class types each requiring a separate .ctor JIT compilation → 1 shared DelegateTestSource type. For a 1,000-class test suite, this is a genuine startup win.
• AOT-safe: static class with static methods + Func<> delegates over static targets = no reflection, no dynamic code. Fully AOT-compatible.
• Minimal blast radius: The engine's consumption of ITestSource/ITestDescriptorSource is unchanged. DelegateTestSource is a clean adapter.
• Snapshot hygiene: All 4 TFMs updated consistently.

---

Concern (Carried Forward): Allocation Count Went Up, Not Down

The previous reviews flagged this correctly. The new path allocates 3 objects per class instead of 1:

	Old	New
Allocations per class at module-init	1 (, zero-field object)	3 ( #1 + #2 + )

A possible nuance: modern .NET caches delegate objects for static method groups, but the cache is keyed per-target-method — and each generated class has its own distinct / method. So each Initialize() call allocates all 3 objects. The tradeoff (more GC-tracked objects at startup for fewer JIT compilations) is still reasonable given this is module-init-only code, but it's the opposite of what the PR description implies.

Suggestion: The PR description says "Eliminates ~1,000 unique .ctor JIT compilations" — which is accurate. It would be clearer to also state that this trades JIT compilation reduction for a 3× increase in startup object allocations. This helps future contributors understand the tradeoff.

---

Architectural Suggestion (Carried Forward): Skip Entirely

The previous review suggested storing delegate pairs directly in rather than wrapping them in a object. This would cut 3 allocations per class to 2:

The engine would then handle this collection alongside the existing collection. This is more invasive and may not be worth it given the startup-only context — but if you're already in this area, it's worth considering as a follow-up.

---

New Observation: Public API Surface Expansion

The new Register(Type, Func<string, IReadOnlyList<TestMetadata>>, Func<IEnumerable<TestDescriptor>>) overload appears in all 4 PublicAPI snapshots. While SourceRegistrar is already decorated with [EditorBrowsable(Never)] in non-debug builds, this is still a public method, which means:

1. It could be called by third-party code referencing TUnit.Core directly.
2. The Func<> signatures are now a public contract. If the method signature of GetTests or EnumerateTestDescriptors ever changes, the overload would need a version bump.

No action required for this PR, but worth keeping in mind when stabilising the public API surface.

---

Minor: Dispatch Overhead

At test discovery time, becomes (delegate invocation) instead of a direct virtual dispatch on the interface. One extra indirection. Not measurable in practice given this is discovery-only code — just noting it for completeness.

---

Summary

This is a valid, well-scoped startup optimization. The JIT reduction claim is accurate. The concerns from the previous reviews (allocation count increase, architectural alternative) are still open but not blocking — they could be addressed in a follow-up. The snapshot tests are correctly updated across all TFMs.

Approved. Consider noting the allocation trade-off explicitly in the PR description or a follow-up issue for completeness.

[claude]

Code Review: perf: eliminate per-class TestSource .ctor JITs via delegate registration

Two previous reviews from claude[bot] were posted on this PR (likely from duplicate runs). The concerns raised remain unaddressed, so I'm carrying them forward and adding a few new observations.

---

What's Good

• JIT reduction is real and correctly measured: N unique sealed class types each requiring a separate .ctor JIT compilation → 1 shared DelegateTestSource type. For a 1,000-class test suite, this is a genuine startup win.
• AOT-safe: static class with static methods + Func<> delegates over static targets = no reflection, no dynamic code. Fully AOT-compatible.
• Minimal blast radius: The engine's consumption of ITestSource/ITestDescriptorSource is unchanged. DelegateTestSource is a clean adapter.
• Snapshot hygiene: All 4 TFMs updated consistently.

---

Concern (Carried Forward): Allocation Count Went Up, Not Down

The previous reviews flagged this correctly. The new path allocates 3 objects per class instead of 1:

	Old	New
Allocations per class at module-init	1 (zero-field class instance)	3 (delegate #1 + delegate #2 + DelegateTestSource)

A possible nuance: modern .NET caches delegate objects for static method groups, but the cache is keyed per-target-method — and each generated class has its own distinct GetTests/EnumerateTestDescriptors method. So each Initialize() call allocates all 3 objects.

The tradeoff (more GC-tracked objects at startup for fewer JIT compilations) is reasonable since this is module-init-only code, but the PR description implies a pure win when it's actually an asymmetric tradeoff. Worth noting explicitly.

---

Architectural Suggestion (Carried Forward): Skip DelegateTestSource Entirely

Storing delegate pairs directly in Sources rather than wrapping them in a DelegateTestSource object would cut 3 allocations per class to 2, and remove the need for the wrapper type entirely. The engine would handle a parallel delegate collection alongside the existing ITestSource collection.

This is more invasive and touching the engine consumption path makes it a reasonable follow-up rather than a blocker here.

---

New Observation: Public API Surface Expansion

The new Register(Type, Func<string, IReadOnlyList<TestMetadata>>, Func<IEnumerable<TestDescriptor>>) overload appears in all 4 PublicAPI snapshots. While SourceRegistrar is decorated with [EditorBrowsable(Never)] in non-debug builds, this is still a public method. The Func<> signatures are now a public contract — if the method signatures of GetTests or EnumerateTestDescriptors ever change, this overload would need a version bump.

No action needed for this PR, but worth tracking when stabilising the public API surface.

---

Minor: Extra Dispatch in Hot(ish) Path

At test discovery time, DelegateTestSource.GetTests(id) becomes _getTests(id) (delegate invocation) rather than a direct interface virtual dispatch. One extra indirection. Not measurable in practice for discovery-only code — just noting for completeness.

---

Summary

This is a valid, well-scoped startup optimization that correctly reduces JIT compilation overhead for large test suites. The snapshot tests are correctly updated across all TFMs, and the approach is AOT-compatible.

Approved. The allocation trade-off is acceptable for module-init code. Consider documenting the tradeoff in the PR description and tracking the "skip DelegateTestSource" approach as a follow-up if startup allocation reduction becomes a priority.