Perf Tweaks

[thomhurst]

No description provided.

[claude]

Claude finished @thomhurst's task —— View job

---

Performance Tweaks PR Review ✅

I've completed a comprehensive code review of this performance-focused pull request. Overall, this is excellent work with significant optimizations that align perfectly with TUnit's performance-first philosophy.

Performance Improvements Identified:

✅ ArgumentFormatter.cs (TUnit.Core/Helpers/ArgumentFormatter.cs:28-37):

• Pre-allocated arrays for formatted strings instead of string concatenation
• Using IList<object?> check to avoid multiple enumerations
• Consistent pattern applied to tuple and enumerable formatting

✅ TestContext.cs (TUnit.Core/TestContext.cs:21, 200-203):

• Added _cachedDisplayName field for display name caching
• Prevents expensive string formatting on repeated GetDisplayName() calls
• Smart invalidation pattern preserves correctness

✅ ReflectionTestDataCollector.cs (TUnit.Engine/Discovery/ReflectionTestDataCollector.cs:20-35):

• Comprehensive caching system with ConcurrentDictionary for multiple cache types
• Assembly caching with proper locking
• Pre-allocated result lists with known capacity (lines 105-110)
• Optimized method caching using GetOrAdd pattern (lines 175-188)

✅ TestScheduler.cs & TestGroupingService.cs:

• Improved test grouping with pre-allocated structures
• More efficient constraint processing
• Better memory allocation patterns

✅ TestFilterTypeExtractor.cs Deletion:

• Removed 79 lines of complex filtering logic that was no longer needed
• Simplifies maintenance and reduces code complexity

✅ TUnitServiceProvider.cs (TUnit.Engine/Framework/TUnitServiceProvider.cs:125-132):

• Cleaner service instantiation logic
• Removed unnecessary abstraction layers

✅ TestDiscoveryService.cs (TUnit.Engine/TestDiscoveryService.cs:31-33, 88-90):

• Pre-allocated collections with known sizes
• Reduced allocations in discovery pipeline

Code Quality Assessment:

🟢 Strengths:

1. Performance Focus: All changes directly target hot paths and allocation reduction
2. Thread Safety: Proper use of ConcurrentDictionary and locking where needed
3. Backward Compatibility: No breaking API changes
4. Code Consistency: Follows established TUnit patterns and modern C# conventions
5. Memory Efficiency: Pre-allocated arrays, cached computations, reduced string operations

🟡 Minor Observations:

1. TestContext.cs:200-203 - Display name caching implementation is solid, but could benefit from nullability documentation
2. ReflectionTestDataCollector.cs - Heavy use of static caching is appropriate for this use case but should be monitored for memory usage in very large test suites

Security & Reliability:

🟢 No Security Concerns: All changes are internal optimizations without external-facing attack vectors

🟢 Thread Safety: Proper concurrent collections usage throughout

🟢 Error Handling: Maintains existing exception handling patterns

Test Coverage Considerations:

❓ Recommendation: Since this PR optimizes critical performance paths, I recommend:

1. Running the source generator snapshot tests: dotnet test TUnit.Core.SourceGenerator.Tests
2. Running public API tests: dotnet test TUnit.PublicAPI
3. Performance regression testing on large test suites
4. Verifying both source-generated and reflection modes maintain behavioral parity

Final Assessment:

✅ Approved - This is exemplary performance optimization work that:

• Maintains TUnit's dual execution mode requirements
• Follows the repository's coding standards perfectly
• Implements proven performance patterns (caching, pre-allocation, concurrent collections)
• Reduces complexity by removing unused code
• Aligns with the "performance is paramount" principle

The optimizations are well-targeted at actual bottlenecks and should provide measurable improvements, especially for large test suites. No breaking changes or security concerns identified.

---