perf: limit converter discovery to declarations

[thomhurst]

AotConverterGenerator walks every syntax node in every test body and repeatedly formats attribute type names while deciding which declarations need converters. Restrict traversal to declarations, resolve BaseTestAttribute once per compilation, and compare symbols directly. Avoid allocating LINQ predicates while checking methods and classes.

Nested classes, partial declarations, constructor parameters, parameter attributes on partial-method implementations, and existing conversion generation are preserved. No runtime execution or public API changes.

Performance evidence

Baseline: a27cfb34bb. BenchmarkDotNet 0.15.8, 10 measured iterations and 5 warm-up iterations, Roslyn 4.14.0, Windows 11, Intel Core i7-12700K (20 logical processors), .NET 10.0.12 x64. SDK selected by the repository's global.json: 11.0.100-preview.7.26381.103.

Each operation runs the converter generator over 10,000 tests in 100 classes. Setup creates and validates the compilation, loads the actual old and new generator assemblies in separate AssemblyLoadContext instances, and checks identical generated sources. The benchmark prints distinct assembly MVIDs and returns each resulting driver. Both builds are measured side by side, after a successful Dry run, without concurrent builds.

Input	Before mean	After mean	Before error	After error	Allocated before	Allocated after
Bare synchronous tests	39.33 ms	14.59 ms	3.253 ms	3.616 ms	17.25 MB	4.54 MB
20 additional statements per body	232.64 ms	15.20 ms	40.241 ms	3.715 ms	151.53 MB	4.54 MB
One inline-data row per method	59.20 ms	19.02 ms	15.100 ms	8.205 ms	18.09 MB	5.69 MB

Error is BenchmarkDotNet's 99.9% confidence-interval half-width. Timings retain substantial variance, but the allocation reductions and large timing differences are clear. This table is a fresh run after stopping a lingering Docker setup process; earlier exploratory timings are not used here. These are converter-only measurements, excluding parsing, project startup, other generators, and compilation of generated code.

Whole-project rebuild experiment

A separate project references TUnit 1.65.68 and replaces only its Core source-generator assembly with the baseline or modified build. It contains 10,000 bare tests in 100 files. Restore is outside timing; each timed dotnet build -c Release --no-restore follows an edit to one file. One warm-up per version is discarded, then five rounds alternate the version order. Both outputs execute exactly 10,000 passing tests.

Round	Before rebuild	After rebuild
1	11,018.03 ms	7,670.69 ms
2	5,937.24 ms	5,673.31 ms
3	7,930.87 ms	8,019.65 ms
4	5,892.08 ms	5,202.96 ms
5	5,392.31 ms	7,267.76 ms

These exploratory rebuild timings are inconclusive. They preceded the background-process cleanup above. The mean improves while the median worsens; noise exceeds the isolated saving for bare tests. This PR claims reduced converter work and allocations, not a demonstrated overall rebuild speedup or elimination of the gap in the framework comparison.

Validation

• Full Core source-generator test/snapshot suite: 122 passed, one pre-existing skipped test, zero failures. No existing verified snapshots changed.
• New regression test compiles generated converters and covers nested classes, partial classes, constructor data, partial-method parameter attributes, and types referenced only inside bodies.
• The new regression test also passes against the original generator assembly.
• Roslyn 4.14 variant builds with zero warnings/errors; the base generator uses the repository's Roslyn 4.7 configuration.
• Synthetic baseline and modified test applications each execute 10,000 passing tests.
• Benchmark setup verifies identical generated sources for all three input shapes.

The complete benchmark source and reproduction steps are posted in a PR comment. Temporary benchmark projects and results are kept outside the repository.

[chatgpt-codex-connector]

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-11T14:16:19.858650Z	10b40c0	PR opened

ℹ️ 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]

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: 866415ae-653a-492e-b152-60c855331110

📥 Commits

Reviewing files that changed from the base of the PR and between a27cfb3 and 10b40c0.

📒 Files selected for processing (2)

• src/TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs
• tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The AOT converter now resolves BaseTestAttribute once, scans declaration nodes only, and uses symbol equality for test detection. Integration coverage validates nested and partial declarations while excluding body-local, property, and field declarations.

Changes

AOT parameter scanning

Layer / File(s)	Summary
Declaration-only parameter scanning src/TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs	The generator resolves BaseTestAttribute once and restricts traversal to supported declaration nodes.
Symbol-based test classification src/TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs, tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs	Test detection uses SymbolEqualityComparer.Default. The integration test covers nested and partial declarations and excludes body-local, property, and field declarations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 10b40

The generator refinement preserves supported declaration scanning while avoiding unsupported executable-body traversal. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: limiting converter discovery to declarations for performance.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/converter-declaration-scan

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.}

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

[thomhurst]

Reproducing the converter benchmark

Build src/TUnit.Core.SourceGenerator in Release at baseline a27cfb34bb and this PR's head. Save each bin/Release/netstandard2.0/TUnit.Core.SourceGenerator.dll in separate before and after directories inside a temporary console project outside the checkout. Build src/TUnit.Core -c Release -f net10.0 and copy its TUnit.Core.dll into that project.

Create the project with dotnet new console -f net10.0, copy the repository's global.json, and add BenchmarkDotNet and Microsoft.CodeAnalysis.CSharp. The measured versions are BenchmarkDotNet 0.15.8 and Microsoft.CodeAnalysis.CSharp 4.14.0.

Add these project items:

<ItemGroup>
  <Reference Include="TUnit.Core"><HintPath>TUnit.Core.dll</HintPath></Reference>
  <None Update="before/*.dll;after/*.dll" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

The benchmark loads both builds in separate AssemblyLoadContext instances while sharing the host's Roslyn types. It prints their different MVIDs and checks that generated source is identical. This avoids accidentally measuring the same DLL twice when MSBuild reuses output directories across jobs.

Complete benchmark source

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using System.Text;
using System.Runtime.Loader;

BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);

[MemoryDiagnoser]
public class ConverterBenchmarks
{
    [Params("Bare", "BodyHeavy", "InlineData")]
    public string Scenario { get; set; } = "Bare";
    private CSharpCompilation _compilation = null!;
    private GeneratorDriver _before = null!;
    private GeneratorDriver _after = null!;

    [GlobalSetup]
    public void Setup()
    {
        var trees = new List<SyntaxTree>();
        for (var c = 0; c < 100; c++)
        {
            var source = new StringBuilder("using System; using TUnit.Core; public class Tests" + c + " {");
            for (var m = 0; m < 100; m++)
            {
                source.Append("[Test]");
                source.Append(Scenario == "InlineData" ? "[Arguments(42)] public void Test" : "public void Test");
                source.Append(m).Append(Scenario == "InlineData" ? "(int value) {" : "() {");
                source.Append("int x = 42; if (x * x + 1 != 1765) throw new Exception();");
                if (Scenario == "BodyHeavy")
                    for (var i = 0; i < 20; i++) source.Append("x = Math.Abs(x) + 1;");
                source.Append('}');
            }
            source.Append('}');
            trees.Add(CSharpSyntaxTree.ParseText(source.ToString()));
        }
        var references = ((string)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES")!).Split(Path.PathSeparator)
            .Select(p => MetadataReference.CreateFromFile(p)).ToList();
        references.Add(MetadataReference.CreateFromFile(typeof(TUnit.Core.TestAttribute).Assembly.Location));
        _compilation = CSharpCompilation.Create("SyntheticSuite", trees, references,
            new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
        var errors = _compilation.GetDiagnostics().Where(d => d.Severity == DiagnosticSeverity.Error).ToArray();
        if (errors.Length != 0) throw new InvalidOperationException(string.Join("\n", errors.Select(e => e.ToString())));
        _before = LoadDriver("before", Path.Combine(AppContext.BaseDirectory, "before", "TUnit.Core.SourceGenerator.dll"));
        _after = LoadDriver("after", Path.Combine(AppContext.BaseDirectory, "after", "TUnit.Core.SourceGenerator.dll"));
        var beforeResult = _before.RunGenerators(_compilation).GetRunResult();
        var afterResult = _after.RunGenerators(_compilation).GetRunResult();
        if (beforeResult.Diagnostics.Concat(afterResult.Diagnostics).Any(d => d.Severity == DiagnosticSeverity.Error))
            throw new InvalidOperationException("Generator diagnostics");
        if (!beforeResult.GeneratedTrees.Select(t => t.ToString()).SequenceEqual(afterResult.GeneratedTrees.Select(t => t.ToString())))
            throw new InvalidOperationException("Generated sources differ");
    }

    private static GeneratorDriver LoadDriver(string name, string path)
    {
        var assembly = new AssemblyLoadContext(name).LoadFromAssemblyPath(path);
        Console.WriteLine($"{name} generator MVID: {assembly.ManifestModule.ModuleVersionId}");
        var generator = (IIncrementalGenerator)Activator.CreateInstance(assembly.GetType("TUnit.Core.SourceGenerator.Generators.AotConverterGenerator")!)!;
        return CSharpGeneratorDriver.Create(generator);
    }

    [Benchmark(Baseline = true)]
    public GeneratorDriver Before() => _before.RunGenerators(_compilation);

    [Benchmark]
    public GeneratorDriver After() => _after.RunGenerators(_compilation);
}

Run:

dotnet run -c Release -- --filter '*ConverterBenchmarks*' --job Dry --artifacts ./dry
dotnet run -c Release --no-build -- --filter '*ConverterBenchmarks*' --iterationCount 10 --warmupCount 5 --artifacts ./measured --exporters json

Each operation runs the entire converter generator over 10,000 test methods across 100 classes. Syntax creation, assembly loading, and compilation diagnostics are outside measurement. The initial generator driver is reused without retaining its returned cached output, so each operation performs generation again. This measures generator work, not process startup or compilation of generated code.

The three inputs are bare synchronous tests, tests with 20 additional statements in each body, and one [Arguments(42)] row per method. Benchmark methods return the resulting driver; the memory column is allocation per complete generator pass.

[greptile-apps]

Greptile Summary

This PR reduces AOT converter-discovery work by pruning traversal below executable member declarations, resolving BaseTestAttribute once per compilation, and replacing LINQ/string comparisons with direct loops and symbol equality.

• Preserves converter discovery from test parameters, declaration attributes, nested and partial classes, and constructor parameters.
• Adds focused compilation coverage for preserved declaration paths and exclusion of body-only type references.

Confidence Score: 5/5

The PR appears safe to merge, with declaration-based converter discovery preserved across the covered language constructs.

No actionable correctness, security, or repository-rule violations remain; the changed traversal still reaches every syntax and symbol source consumed by converter generation.

Important Files Changed

Filename	Overview
src/TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs	Restricts converter scanning to declaration-relevant syntax and replaces repeated formatting and LINQ checks with cached symbol resolution and direct iteration.
tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs	Adds focused coverage for nested, partial, constructor, method-parameter, and body-only converter-discovery scenarios.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Compilation syntax trees] --> B[Traverse declarations]
    B --> C[Test methods]
    B --> D[Test classes]
    C --> E[Parameters and attributes]
    D --> F[Class attributes and constructors]
    E --> G[Collect referenced types]
    F --> G
    G --> H[Find conversion operators]
    H --> I[Generate AOT converter registrations]

Loading

_{Reviews (1): Last reviewed commit: "perf: limit converter discovery to decla..." | Re-trigger Greptile}

[github-actions]

Review

Reviewed the diff in src/TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs (the switch to declaration-only tree traversal via the syntax-node predicate, the attribute-list short-circuit, and moving IsTestMethod/IsTestClass to compare INamedTypeSymbol via SymbolEqualityComparer instead of ToDisplayString()), plus the new regression test tests/TUnit.Core.SourceGenerator.Tests/AotConverterScanTests.cs.

The overall direction is good: pruning descent into method/property/field bodies is a solid perf win for this hot generator path, and switching to symbol equality is more correct than string-comparing display names. I traced through two things that looked risky at first and turned out fine:

• The syntax-node pruning only stops syntactic descent into bodies/parameter-lists/attribute-lists — all the data the scanner needs (parameters, attributes, constructors) comes from symbol APIs (methodSymbol.Parameters, classSymbol.Constructors, etc.), which aren't affected by pruning.
• The if (method.AttributeLists.Count == 0) continue; short-circuit is safe for partial methods, since Roslyn merges attributes from all partial declaration parts onto the resulting symbol, so at least one syntactic part carries the attribute directly (the new test exercises exactly this case and passes).

One issue worth fixing before merge:

baseTestAttribute is resolved once via GetTypeByMetadataName with no null handling (AotConverterGenerator.cs:110). If that call returns null, IsTestMethod silently returns false for every method (since baseType in the comparison loop is never null, so SymbolEqualityComparer.Default.Equals(baseType, null) can never be true) — meaning the generator finds zero test methods/classes and emits no AOT converters, with no diagnostic or exception.

GetTypeByMetadataName returns null not just when a type is missing, but also when it's ambiguous — e.g. a project referencing TUnit.Core.BaseTestAttribute through more than one path (a ProjectReference plus a transitive PackageReference, or a version-conflict scenario). This exact failure mode is already known and handled elsewhere in this generator project: InfrastructureGenerator.GetFirstUniquePublicType has an explicit comment — "GetTypeByMetadataName returns null when the type name is ambiguous" — and works around it by resolving through compilation.GetTypeByMetadataName per-candidate and checking equality rather than trusting a single global lookup.

The previous implementation (baseType.ToDisplayString() == "TUnit.Core.BaseTestAttribute") was robust to this ambiguity since it never depended on a single canonical symbol resolving. The new regression test doesn't catch the regression because ReferencesHelper.References only ever includes a single copy of TUnit.Core, so the type resolves unambiguously in that test compilation.

Suggested fix: keep the single GetTypeByMetadataName lookup as a fast path, but fall back to the previous display-string comparison (or GetTypeByMetadataName per ambiguous candidate, mirroring GetFirstUniquePublicType) when baseTestAttribute is null, so a version-conflict/multi-reference scenario degrades gracefully instead of silently dropping all AOT converters.

🤖 Generated with Claude Code