perf: avoid formatting interface names for data-source checks - #6782
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change centralizes ChangesData source attribute detection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The data-source detection optimization preserves supported detection behavior and has focused test coverage; no actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. A rabbit reads each line, Comment |
|
Reproduction and validation evidence This compares the actual generator assembly built from baseline
Build dotnet run -c Release -- --filter '*' --job Dry
dotnet run -c Release --no-build -- --filter '*InterfaceCheckBenchmarks*' --iterationCount 15 --warmupCount 6 --launchCount 1 --exporters json
dotnet run -c Release --no-build -- --filter '*MetadataGeneratorBenchmarks*' --iterationCount 15 --warmupCount 6 --launchCount 1 --invocationCount 1 --unrollFactor 1 --exporters jsonThe focused benchmark classifies real The complete Validation commands: dotnet build src/TUnit.Core/TUnit.Core.csproj -c Release -f netstandard2.0
dotnet run --project tests/TUnit.Core.SourceGenerator.Tests -c Release -f net10.0
dotnet run --project tests/TUnit.Core.SourceGenerator.Tests -c Release -f net472 -- --treenode-filter '/*/*/DataSourceAttributeDetectionTests/*'
dotnet build src/TUnit.Core.SourceGenerator.Roslyn414/TUnit.Core.SourceGenerator.Roslyn414.csproj -c Release
dotnet build src/TUnit.Core.SourceGenerator.Roslyn44/TUnit.Core.SourceGenerator.Roslyn44.csproj -c ReleaseResults: 133 passed and one existing skip in the full generator suite, with no snapshot changes. All 11 new cases pass on .NET 10 and .NET Framework 4.7.2. Both Roslyn variant builds pass with zero warnings/errors. Tests cover direct/inherited/indirect interfaces, generic attributes, similarly named interfaces in other namespaces, generic interface lookalikes, nested types, and null attributes. The nested-type fallback deliberately preserves the previous display-name matching behavior. Focused classification results:
Complete metadata-generator results:
Benchmark project and source<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net10.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.14.0" />
<Reference Include="TUnit.Core"><HintPath>TUnit.Core.dll</HintPath></Reference>
<None Update="before/*.dll;after/*.dll" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
</Project>
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using System.Reflection;
using System.Runtime.Loader;
using System.Text;
BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
public static class Versions
{
public static Assembly Load(string name)
{
var assembly = new AssemblyLoadContext(name).LoadFromAssemblyPath(
Path.Combine(AppContext.BaseDirectory, name, "TUnit.Core.SourceGenerator.dll"));
Console.WriteLine($"{name} MVID: {assembly.ManifestModule.ModuleVersionId}");
return assembly;
}
}
[MemoryDiagnoser]
public class MetadataGeneratorBenchmarks
{
[Params("Bare", "InlineData", "TypedInlineData")]
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 switch { "InlineData" => "[Arguments(42)] public void Test", "TypedInlineData" => "[Arguments<int>(42)] public void Test", _ => "public void Test" });
source.Append(m).Append(Scenario != "Bare" ? "(int value) {" : "() {");
source.Append("int x = 42; if (x * x + 1 != 1765) throw new Exception(); }");
}
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");
_after = LoadDriver("after");
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");
var beforeSources = beforeResult.Results.Single().GeneratedSources;
var afterSources = afterResult.Results.Single().GeneratedSources;
if (beforeSources.Length != 100 || afterSources.Length != 100 ||
!beforeSources.Select(s => (s.HintName, s.SourceText.ToString()))
.SequenceEqual(afterSources.Select(s => (s.HintName, s.SourceText.ToString()))))
throw new InvalidOperationException("Generated sources differ or are missing.");
}
private static GeneratorDriver LoadDriver(string name) => CSharpGeneratorDriver.Create(
(IIncrementalGenerator)Activator.CreateInstance(Versions.Load(name).GetType(
"TUnit.Core.SourceGenerator.Generators.TestMetadataGenerator")!)!);
[Benchmark(Baseline = true)]
public GeneratorDriver Before() => _before.RunGenerators(_compilation);
[Benchmark]
public GeneratorDriver After() => _after.RunGenerators(_compilation);
}
[MemoryDiagnoser]
public class InterfaceCheckBenchmarks
{
[Params(false, true)]
public bool Typed { get; set; }
private INamedTypeSymbol _attributeClass = null!;
private Func<INamedTypeSymbol, bool> _before = null!;
private Func<INamedTypeSymbol, bool> _after = null!;
[GlobalSetup]
public void Setup()
{
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));
var compilation = CSharpCompilation.Create("AttributeSymbols", references: references);
_attributeClass = Typed
? compilation.GetTypeByMetadataName("TUnit.Core.ArgumentsAttribute`1")!.Construct(compilation.GetSpecialType(SpecialType.System_Int32))
: compilation.GetTypeByMetadataName("TUnit.Core.ArgumentsAttribute")!;
_before = Load("before");
_after = Load("after");
if (!_before(_attributeClass) || !_after(_attributeClass))
throw new InvalidOperationException("Expected both argument attributes to be recognized.");
}
private static Func<INamedTypeSymbol, bool> Load(string name) =>
Versions.Load(name).GetType("TUnit.Core.SourceGenerator.CodeGenerators.Helpers.DataSourceAttributeHelper")!
.GetMethod("IsDataSourceAttribute")!.CreateDelegate<Func<INamedTypeSymbol, bool>>();
[Benchmark(Baseline = true)]
public bool Before() => _before(_attributeClass);
[Benchmark]
public bool After() => _after(_attributeClass);
} |
Greptile SummaryThis PR optimizes data-source attribute detection by avoiding fully qualified interface-name formatting in the common case while retaining the previous comparison as a compatibility fallback.
Confidence Score: 5/5The PR appears safe to merge, with no concrete correctness, compatibility, security, or repository-rule issues identified. The optimized path preserves the ordinary interface classification and retains the previous display-name comparison as a fallback, while the added tests cover the relevant behavioral boundaries.
|
| Filename | Overview |
|---|---|
| src/TUnit.Core.SourceGenerator/CodeGenerators/Helpers/DataSourceAttributeHelper.cs | Adds an allocation-conscious symbol-property fast path and retains legacy display-name matching for unusual symbols. |
| src/TUnit.Core.SourceGenerator/Extensions/AttributeDataExtensions.cs | Delegates data-source classification to the shared helper without changing the extension contract. |
| tests/TUnit.Core.SourceGenerator.Tests/DataSourceAttributeDetectionTests.cs | Adds focused coverage for accepted and rejected interface shapes, legacy nested-name behavior, and null input. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AttributeData or named type] --> B[DataSourceAttributeHelper]
B --> C{Interface name matches?}
C -- No --> D[Continue scanning]
C -- Yes --> E{Ordinary non-generic interface in TUnit.Core?}
E -- Yes --> F[Data source]
E -- No --> G{Legacy fully qualified name matches?}
G -- Yes --> F
G -- No --> D
D --> H{More interfaces?}
H -- Yes --> C
H -- No --> I[Not a data source]
Reviews (1): Last reviewed commit: "perf: avoid formatting interface names f..." | Re-trigger Greptile
1 participant
Data-source detection repeatedly formats fully qualified names for implemented interfaces, including unrelated generic interfaces. This work runs while filtering and emitting test attributes.
Reject interfaces with a different simple name, then recognize the usual non-generic
TUnit.Core.IDataSourceAttributethrough symbol properties. Keep the previous display-name comparison as a fallback for unusual shapes. Route both existing detection entry points through the same implementation. Generated source remains unchanged.Focused BenchmarkDotNet results using actual baseline (
e322d68799) and modified generator assemblies:ArgumentsAttributeArgumentsAttribute<int>The checks are approximately 6–12× faster and allocate 104–352 fewer bytes per call.
Complete
TestMetadataGeneratorpasses over 10,000 tests in 100 classes:The allocation reductions are approximately 1.9 MB and 10.1 MB for the data-driven suites. Full-generator timing improvements are inconclusive: confidence intervals overlap and the bare-test control also changed substantially. These measurements exclude parsing and compilation of generated code; no whole-build speedup is claimed.
Validation: 133 generator tests pass with one existing skip and no snapshot changes. All 11 new classification cases pass on .NET 10 and .NET Framework 4.7.2. Roslyn 4.4 and 4.14 builds pass without warnings/errors. Benchmark setup verifies identical generated filenames and source text for all three workloads. Tests explicitly preserve legacy matching of nested interfaces with the same display name.
Environment: BenchmarkDotNet 0.15.8, Roslyn 4.14.0, Windows 11, i7-12700K, .NET 10.0.12, SDK 11.0.100-preview.7.26381.103; 15 measured iterations, 6 warmups, 1 launch. Full reports, confidence intervals, source, and reproduction commands are in the PR comment.
Summary by CodeRabbit
Bug Fixes
Tests