perf: generate per-class helper to reduce JIT compilations by ~18,000

[thomhurst]

Summary

• Generates a shared __ClassHelper per test class that consolidates CreateInstance and ModuleInitializer registration, eliminating ~18,000 JIT compilations in the 10,000-test benchmark
• For each non-generic test class, one helper file is emitted containing a shared CreateInstance method and a batched Initialize() that registers all test sources for that class
• Generic types/methods and inherited tests (InheritanceDepth > 0) continue using the per-method pattern unchanged

Profiling results (1,000 classes × 10 methods = 10,000 tests)

Total JIT compilations: 75,311 → 57,251 (24% reduction)

Category	Before	After	Change
TUnit.Generated	60,000	42,000	-18,000
Calculator (test code)	11,009	11,009	0
System + Microsoft	2,913	2,825	-88
TUnit framework	1,239	1,236	-3
Other	150	181	+31

TUnit.Generated method breakdown:

Method	Before	After	Saved
.ctor (TestSource)	10,000	10,000	0
GetTests	10,000	10,000	0
__CreateAttributes	10,000	10,000	0
__InvokeTest	10,000	10,000	0
__CreateInstance (per-method)	10,000	0 (removed)	10,000
Initialize (companion class)	10,000	0 (removed)	10,000
CreateInstance (ClassHelper)	0	1,000	-1,000
Initialize (ClassHelper)	0	1,000	-1,000

Runtime (10,000 tests, 5 runs, Release):

Run	TUnit Duration	Wall Time
1	3.661s	4.224s
2	3.614s	4.167s
3	3.630s	4.173s
4	3.551s	4.096s
5	3.632s	4.169s
Avg	3.618s	4.166s

New files

• TUnit.Core.SourceGenerator/Models/ClassTestGroup.cs — per-class grouping model (primitives only, no ISymbol)

Modified files

• TestMetadataGenerator.cs — per-class pipeline (GroupMethodsByClass + GenerateClassHelper), conditional skip of __CreateInstance and ModuleInitializer for direct non-generic tests
• InstanceFactoryGenerator.cs — GenerateInstanceFactoryBody() to pre-generate factory code as string
• FileNameHelper.cs — GetSafeClassHelperName() for deterministic class helper naming
• 109 .verified.txt snapshot files updated across all 4 TFMs

Test plan

• Source generator builds (all 4 Roslyn variants)
• TUnit.TestProject builds with 0 errors on net10.0
• BasicTests (3 tests), DataDrivenTests (15), ClassConstructorTest (1), AsyncInheritedTestsRepro (8), GenericMethodTests (2) all pass at runtime
• Snapshot tests pass on net8.0, net9.0, net10.0, net472 (114/114 + 1 pre-existing skip)
• PublicAPI tests pass (no public API changes)
• Pack v99.0.0, rebuild 10k-test repro, verify JIT reduction (75,311 → 57,251)
• Runtime profiling: 10,000 tests in ~3.6s TUnit duration / ~4.2s wall time

[claude]

Code Review: per-class helper to reduce JIT compilations

This is a well-designed performance optimization with clear goals and thorough test coverage. The core idea — amortising per-method boilerplate into a single per-class artefact — is sound, and the PR description with the JIT reduction table makes the trade-off immediately obvious. A few things worth discussing before merging:

---

1. Collect() defeats per-class incremental caching (Medium)

ClassTestGroup is carefully designed to hold only primitives so Roslyn can cache it, and that's exactly right. However, the pipeline:

var classHelperProvider = testMethodsProvider
    .Select(static (data, _) => data.Left)
    .Where(...)
    .Collect()                                     // ← fan-in here
    .SelectMany(static (methods, _) => GroupMethodsByClass(methods))
    .Combine(enabledProvider);

The Collect() call gathers all test methods into a single node before grouping. Any change to any test method invalidates the entire collected node, which forces all class helpers to be regenerated on the next build. For the 10k-test benchmark that's ~1,000 helper files regenerated on every keystroke in the IDE — the opposite of what the primitive-only model was designed for.

A truly incremental alternative would produce one ClassTestGroup per method (not per class) from an upstream Select, then deduplicate during output — or accept this trade-off but document it explicitly with a comment so the next maintainer understands why the caching model is intentionally coarse here.

---

2. Implicit coupling between useClassHelper and GenerateTypedInvokers (Medium)

In GenerateTestMetadata:

var useClassHelper = \!needsList && testMethod.InheritanceDepth == 0;   // guards generics via needsList

But in GenerateTypedInvokers (called later):

if (testMethod.InheritanceDepth == 0)   // no \!needsList guard here
{
    writer.AppendLine($"InstanceFactory = {classHelperName}.CreateInstance,");
}

This works today because needsList = IsGenericType || IsGenericMethod (line 354), so generics always have needsList = true → useClassHelper = false. However, GenerateTypedInvokers only sees InheritanceDepth == 0 and relies on the outer useNamedMethods guard to implicitly exclude generics. If the condition for needsList ever changes, a generic test with InheritanceDepth == 0 would silently reference a ClassHelper that is never generated, producing a compile error in user code.

Making the coupling explicit is cheap:

if (testMethod is { InheritanceDepth: 0, IsGenericType: false, IsGenericMethod: false })
{
    writer.AppendLine($"InstanceFactory = {classHelperName}.CreateInstance,");
}

---

3. Non-deterministic ordering of Register calls (Low)

TestSourceClassNames is populated via LINQ GroupBy + Select, which does not guarantee stable ordering between compilation runs. Roslyn's incremental engine uses value equality on the output to decide whether to re-emit, so a non-deterministic order on TestSourceClassNames would cause spurious cache misses and unnecessary file re-emissions. The snapshot tests pass today (likely because Collect() returns items in source-declaration order in the test harness), but this could behave differently under parallel Roslyn analysis or when files are added/removed.

Sorting before materialising the array would make this deterministic:

TestSourceClassNames = g
    .Select(m => FileNameHelper.GetDeterministicFileNameForMethod(m\!.TypeSymbol, m.MethodSymbol)
        .Replace(".g.cs", "_TestSource"))
    .OrderBy(static s => s, StringComparer.Ordinal)   // ← add this
    .ToEquatableArray(),

---

4. Minor: containingTypes list naming is slightly misleading

In GetSafeClassHelperName, the local list is named containingTypes but actually starts by adding typeSymbol.Name itself (the target type), then walks up via ContainingType. After reversing the list therefore contains [outermost … typeSymbol], i.e., it's really "all types including self". The logic is correct, but the name suggests "only the enclosing types". A quick rename to typeHierarchy or a brief comment would remove the ambiguity.

---

What works well

• The ClassTestGroup primitive-only model is the right abstraction for incremental caching.
• EquatableArray<string> used correctly for structural equality.
• GetSafeClassHelperName correctly handles nested types, long names (hash truncation), and namespaced types.
• Per-method __CreateInstance duplication is eliminated cleanly; only generic types and inherited tests keep the old per-method pattern — a sensible conservative scope for this change.
• Error handling with a dedicated TUNIT0998 diagnostic rather than silently swallowing exceptions is good practice.
• 109 snapshot files updated with 114/114 snapshot tests passing is thorough coverage.

The runtime JIT improvement is real and worth pursuing. Items 1 and 2 above are the ones most worth addressing before merge; 3 and 4 are nice-to-haves.

[claude]

Follow-up Review: per-class helper generation

All four items from the previous review have been addressed in the follow-up commit. Here's a summary of each, plus two new observations found during this pass.

---

Previous review items — all resolved ✅

1. Collect() fan-in trade-off — Documented with an accurate comment that also references the precedent in PropertyInjectionSourceGenerator. This is the right call: the trade-off (GroupMethodsByClass re-runs on every edit, only changed files re-emitted due to ClassTestGroup value equality) is now explicit and intentional.

2. Implicit coupling in GenerateTypedInvokers — Fixed with the pattern match guard:

if (testMethod is { InheritanceDepth: 0, IsGenericType: false, IsGenericMethod: false })

This is now robust against future changes to the needsList conditions.

3. Non-deterministic Register ordering — Fixed. OrderBy(static s => s, StringComparer.Ordinal) on TestSourceClassNames makes the output stable. The snapshot diffs confirm the reordering took effect.

4. Misleading variable name — Renamed containingTypes → typeHierarchy in GetSafeClassHelperName. The comment explaining the walk direction is clear.

---

New observations

Dead code in GetSafeClassHelperName (Minor)

// Add generic parameters if any (for the innermost type)
if (typeSymbol.TypeArguments.Length > 0) { ... }

Both call sites guarantee IsGenericType == false before invoking GetSafeClassHelperName, so this branch is unreachable. For a INamedTypeSymbol, IsGenericType is true iff TypeArguments.Length > 0, so the guard is equivalent to if (false). This is harmless, and the code was clearly ported from GetDeterministicFileNameForMethod for consistency, but it's misleading dead code. A comment like // Note: only called for non-generic types; TypeArguments.Length is always 0 here (or a guard-and-throw) would prevent future confusion.

classHelperProvider filter doesn't include InheritanceDepth == 0 (Very Minor)

.Where(static m => m is not null && !m!.IsGenericType && !m.IsGenericMethod)

Because testMethodsProvider is sourced from ForAttributeWithMetadataName("TUnit.Core.TestAttribute") and GetTestMethodMetadata never sets InheritanceDepth (it always defaults to 0), all methods in this pipeline will have depth 0. However, the comment says "collect non-generic direct test methods" — adding && m.InheritanceDepth == 0 would make that intent explicit and guard against a future change where inherited methods could flow through testMethodsProvider. Not a bug today, but a cheap safety net.

---

Overall assessment

The core optimization is sound and cleanly implemented. The per-class helper correctly consolidates CreateInstance and ModuleInitializer registrations. The ClassTestGroup primitive-only design, the sorted TestSourceClassNames, the explicit generic guards, and the defensive TUNIT0998 diagnostic are all correct. The internal visibility on Initialize() (previously public) is a welcome reduction of generated public API surface. The snapshot test coverage (109 files, 4 TFMs) gives high confidence the output is correct.

The two new observations above are minor; the PR is ready to merge as-is.