perf: eliminate <>c display class from generated TestSource classes

[thomhurst]

Summary

• Eliminates all lambdas from generated non-generic TestSource classes, preventing the C# compiler from generating a <>c display class
• Follows up on perf: make test metadata creation fully synchronous #5045 which reduced JIT compilations from 136k to 105k — this targets the remaining ~30k <>c overhead entries
• For 10,000 non-generic test sources (9 JIT methods each), removes 3 <>c methods per source: .ctor(), .cctor(), and the ClassMetadata/AssemblyMetadata factory lambda

Changes

TUnit.Core:

• Add AssemblyMetadata.GetOrAdd(string key, string name) — takes name directly instead of Func<AssemblyMetadata> factory
• Add ClassMetadata.GetOrAdd(string name, ClassMetadata value) — takes pre-constructed value instead of Func<ClassMetadata> factory, handles property back-references internally

TUnit.Core.SourceGenerator:

• MetadataGenerationHelper: Emit parameter-based GetOrAdd calls (no factory lambdas)
• TestMetadataGenerator: For non-generic tests, emit named static methods (__CreateAttributes, __CreateInstance, __InvokeTest) referenced via method groups instead of lambdas
• InstanceFactoryGenerator: Add GenerateInstanceFactoryAsMethod(), refactor with extracted body methods for reuse

Generated code before vs after

// BEFORE: 3 lambdas → compiler generates <>c display class
AttributeFactory = static () => [ new TestAttribute() ],
InstanceFactory = static (typeArgs, args) => new BasicTests(),
InvokeTypedTest = static (instance, args, ct) => { ... },
Class = ClassMetadata.GetOrAdd("key", static () => { ... })
Assembly = AssemblyMetadata.GetOrAdd("name", static () => new AssemblyMetadata { ... })

// AFTER: 0 lambdas → no <>c display class
AttributeFactory = __CreateAttributes,
InstanceFactory = __CreateInstance,
InvokeTypedTest = __InvokeTest,
Class = ClassMetadata.GetOrAdd("key", new ClassMetadata { ... })
Assembly = AssemblyMetadata.GetOrAdd("name", "name")

Profiling Results (10,000 tests, 1,000 classes)

Measured with DOTNET_JitDisasmSummary=1 on the reproduction project (Calculator.Tests.TUnit, net10.0, Release).

Metric	Before	After	Improvement
Total JIT compilations	105,312	75,311	-30,001 (28.5%)
TUnit.Generated JITs	90,001	60,000	-30,001 (33.3%)
TUnit.Generated <>c JITs	60,001	0	-60,001 (100%)
Methods per TestSource	9	6	-3
Wall-clock time	5.47s	3.82s	-1.65s (30.2%)

Eliminated methods per TestSource (x 10,000)

Method	Before	After
<>c:.ctor()	10,000	0
<>c:.cctor()	10,000	0
<>c:<GetTests>b__0_3() (ClassMetadata + AssemblyMetadata factory)	10,000	0

Remaining 6 methods per TestSource

.ctor, GetTests, __CreateAttributes, __CreateInstance, __InvokeTest, ModuleInitializer:Initialize

Test plan

• Source generator snapshot tests pass on net8.0, net9.0, net10.0
• Public API tests pass on all TFMs
• TUnit.TestProject builds with zero errors
• Profile with 10k-test repro project: 0 <>c entries, 30k fewer JITs, 30% faster wall time
• CI pipeline passes

[claude]

Code Review: perf: eliminate <>c display class from generated TestSource classes

Overall Assessment

The optimization strategy is sound and well-executed. Using named static methods instead of lambdas to prevent compiler-generated <>c display classes is a recognized JIT optimization technique. The snapshot test coverage gives confidence in correctness, and the AssemblyMetadata change is elegantly minimal. A few issues are worth addressing.

---

Issues Found

1. Logic Duplication in Attribute Collection (Medium)

The attribute-gathering LINQ chain is copy-pasted between two methods in TestMetadataGenerator.cs:

In GenerateMetadata:

var attributes = methodSymbol.GetAttributes()
    .Where(a => !DataSourceAttributeHelper.IsDataSourceAttribute(a.AttributeClass))
    .Concat(testMethod.TypeSymbol.GetAttributesIncludingBaseTypes())
    .Concat(testMethod.TypeSymbol.ContainingAssembly.GetAttributes());

In EmitAttributeFactoryMethod:

var attributes = testMethod.MethodSymbol.GetAttributes()
    .Where(a => !DataSourceAttributeHelper.IsDataSourceAttribute(a.AttributeClass))
    .Concat(testMethod.TypeSymbol.GetAttributesIncludingBaseTypes())
    .Concat(testMethod.TypeSymbol.ContainingAssembly.GetAttributes());

These are identical. If the attribute filtering logic ever needs changing (e.g., a new attribute category to exclude), it must be updated in two places. Extract to a shared private method:

private static IEnumerable<AttributeData> GetTestAttributes(TestMethodMetadata testMethod)
    => testMethod.MethodSymbol.GetAttributes()
        .Where(a => !DataSourceAttributeHelper.IsDataSourceAttribute(a.AttributeClass))
        .Concat(testMethod.TypeSymbol.GetAttributesIncludingBaseTypes())
        .Concat(testMethod.TypeSymbol.ContainingAssembly.GetAttributes());

---

2. Logic Duplication in CancellationToken/Parameters Parsing (Medium)

The same parameter-parsing logic appears in both GenerateTypedInvokers and EmitInvokeTestMethod:

var hasCancellationToken = parameters.Length > 0 &&
    parameters.Last().Type.GloballyQualified() == "global::System.Threading.CancellationToken";
var parametersFromArgs = hasCancellationToken
    ? parameters.Take(parameters.Length - 1).ToArray()
    : parameters.ToArray();
var returnPattern = GetReturnPattern(testMethod.MethodSymbol);

This duplicates logic that already existed in GenerateTypedInvokers. Now EmitInvokeTestMethod re-derives the same values independently. This is a consequence of the new method being a standalone emitter rather than sharing context with the invoker generation code. Either pass these as parameters or extract a small helper struct/record to hold the parsed invoker context.

---

3. ClassMetadata.GetOrAdd Pre-Allocates Unconditionally (Worth Documenting)

The new GetOrAdd(string, ClassMetadata value) overload always receives a pre-constructed ClassMetadata from the caller (the generated code). On cache hits — which are the common case when multiple test methods share the same test class — the object is immediately discarded.

Contrast with the new AssemblyMetadata.GetOrAdd(string, string) overload, which is correctly lazy via the ConcurrentDictionary.GetOrAdd(TKey, Func<TKey,TArg,TValue>, TArg) pattern:

// ✅ Correctly lazy — no allocation unless cache miss
return Cache.GetOrAdd(key, static (_, n) => new AssemblyMetadata { Name = n }, name);

ClassMetadata can't easily replicate this pattern because of the parameters-update logic. But it's worth noting that for a class with 10 test methods, 9 ClassMetadata objects (plus arrays for Properties, Parameters, TypeInfo, AssemblyMetadata, etc.) are allocated and GC'd at startup per class.

This trade-off (fewer JIT compilations vs. more startup allocations) is reasonable given the goals of this PR, but a brief comment in ClassMetadata.GetOrAdd(string, ClassMetadata) explaining the trade-off would help future readers understand why this overload exists alongside the factory-based one.

---

4. Double-Underscore Method Names (Minor)

The generated methods are named __CreateAttributes, __CreateInstance, __InvokeTest. In C#, identifiers beginning with __ are by convention (and in some specs) reserved for compiler/runtime internals. While this works in practice for private methods in sealed generated classes, consider a prefix that signals TUnit provenance more clearly:

// Alternative naming — unambiguously user-space
private static TUnit_CreateAttributes() { ... }
private static TUnit_CreateInstance(...) { ... }
private static TUnit_InvokeTest(...) { ... }

---

What's Done Well

• AssemblyMetadata.GetOrAdd(string key, string name) is a clean, minimal, properly-lazy addition. Using the (TKey, Func<TKey, TArg, TValue>, TArg) ConcurrentDictionary overload avoids the allocation-on-hit problem entirely.
• Extracted body methods (GenerateClassConstructorStubBody, GenerateTypedConstructorCallBody, GenerateConcreteTestInvokerBody) in InstanceFactoryGenerator.cs are the right structure for reuse between lambda and method-group paths.
• The useNamedMethods flag correctly scopes the optimization to non-generic, non-list tests where the display class elimination applies.
• Snapshot test coverage across all three TFMs gives strong confidence in the generated output being correct.
• The overall goal — eliminating ~30k JIT compilations from startup — is well-justified and the technique is correct.

---

Summary

The two duplication issues (items 1 and 2) are the most important to address because they create maintenance debt in an already-complex code generator. The pre-allocation trade-off (item 3) is acceptable but should be documented. Items 3 and 4 are minor. The PR is otherwise in good shape.

[thomhurst]

Addressed the review feedback in 680304b:

Item 1 (Attribute duplication): Extracted GetTestAttributes() shared helper used by both GenerateMetadata and EmitAttributeFactoryMethod.

Item 2 (Parameter parsing duplication): Extracted ParseInvokerParameters() returning a tuple, used by both GenerateTypedInvokers and EmitInvokeTestMethod.

Item 3 (ClassMetadata pre-allocation): Added XML doc comment to ClassMetadata.GetOrAdd(string, ClassMetadata) explaining the trade-off.

Item 4 (Double-underscore names): Keeping __ prefix. These are compiler-generated methods emitted by our source generator — the __ convention correctly signals "compiler infrastructure, don't touch" to users, which is the standard C# source generator convention. A TUnit_ prefix would incorrectly suggest these are part of TUnit's public API surface.

[claude]

Follow-up Review: perf: eliminate <>c display class from generated TestSource classes

Previous Issues — Status Update

All three medium-priority issues from my previous review have been addressed:

✅ #1 (Attribute collection duplication) — Extracted to GetTestAttributes(TestMethodMetadata) helper (TestMetadataGenerator.cs). Both GenerateMetadata and EmitAttributeFactoryMethod now delegate to it.

✅ #2 (CancellationToken/parameters parsing duplication) — Extracted to ParseInvokerParameters(IMethodSymbol) returning a tuple. Both GenerateTypedInvokers and EmitInvokeTestMethod now call it.

✅ #3 (ClassMetadata pre-allocation undocumented) — XML doc comment added to the new overload clearly explains the allocation trade-off vs. JIT savings.

⚠️ #4 (Double-underscore method names) — Not addressed. The generated methods remain __CreateAttributes, __CreateInstance, __InvokeTest. The concern stands: __-prefixed identifiers are conventionally reserved for compiler/runtime use in C#. These live inside sealed, generated, internal classes so it's unlikely to cause real problems, but TUnit_CreateAttributes (or just CreateAttributes given the class is already unique) would better signal provenance.

---

New Finding: TryUpdate Failure Path Returns Wrong Instance

In ClassMetadata.GetOrAdd(string, ClassMetadata value):

if (Cache.TryGetValue(name, out var existing))
{
    if (existing.Parameters.Length == 0 && value.Parameters.Length > 0)
    {
        Cache.TryUpdate(name, value, existing); // could return false!
        return value;                           // returned regardless
    }
    return existing;
}

If TryUpdate loses a race (another thread already updated the cache), this returns value — the caller's local, non-cached ClassMetadata — while the cache holds a different instance. Two threads could end up with different ClassMetadata instances for the same class key.

However, this is pre-existing behavior — the original GetOrAdd(string, Func<ClassMetadata>) overload has the exact same pattern:

Cache.TryUpdate(name, newValue, existing);
return newValue; // same issue — returned regardless of TryUpdate result

This PR faithfully replicates the existing logic rather than introducing a new bug. The practical impact is minimal since module initializers run at startup, and the Parameters-upgrade path is rare. A comment noting the known non-atomicity would be helpful, but this is inherited debt, not new work.

---

Overall

The three substantive issues from the prior review are cleanly resolved. The extractions (GetTestAttributes, ParseInvokerParameters) improve maintainability meaningfully. The __ naming convention remains the only open minor concern. The PR is in good shape and achieves its goal of eliminating the <>c display class overhead.