Skip to content

perf: parallelize post-resolution event invocation#4676

Merged
thomhurst merged 1 commit intomainfrom
perf/parallelize-post-resolution-events
Feb 6, 2026
Merged

perf: parallelize post-resolution event invocation#4676
thomhurst merged 1 commit intomainfrom
perf/parallelize-post-resolution-events

Conversation

@thomhurst
Copy link
Owner

Summary

  • Replace sequential foreach + await with Parallel.ForEachAsync for InvokePostResolutionEventsAsync during test discovery
  • Per-test events are independent, so for large test suites (thousands of tests) this eliminates a major sequential bottleneck
  • Includes threshold check (ParallelThresholds.MinItemsForParallel) to avoid parallelization overhead for small test counts
  • Falls back to Task.WhenAll on pre-.NET 6 targets

Test plan

  • Verify event receivers still fire correctly for all tests
  • Verify post-resolution hooks work as expected
  • Run full engine test suite

🤖 Generated with Claude Code

Replace sequential foreach+await with Parallel.ForEachAsync for
InvokePostResolutionEventsAsync. Per-test events are independent, so
for large test suites (thousands of tests) this eliminates a major
sequential bottleneck during discovery.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Architecture & Design Observations

This is a well-implemented performance optimization that follows good engineering practices. The change is clean, safe, and appropriately scoped. A few observations on the design:

Strengths:

  • Correct concurrency model: The use of AsyncLocal<TestContext> makes parallelization safe without requiring complex synchronization primitives.
  • Platform-appropriate APIs: Using Parallel.ForEachAsync on .NET 6+ with bounded parallelism (Environment.ProcessorCount) vs. Task.WhenAll on older frameworks shows thoughtful API selection.
  • Performance threshold: The MinItemsForParallel = 8 check avoids task scheduling overhead for small test suites—a detail often overlooked in parallelization efforts.
  • Proper async hygiene: Consistent use of ConfigureAwait(false) and avoiding blocking operations.

Architectural consideration (optional future enhancement):

The #else path (pre-.NET 6) uses unbounded Task.WhenAll, which could theoretically spawn thousands of concurrent tasks for very large test suites. While this is unlikely to cause issues in practice (tasks are I/O-bound or quickly complete), you might consider adding a degree-of-parallelism limiter for the legacy path if you encounter resource pressure:

// Example: using SemaphoreSlim for bounded concurrency on pre-.NET 6
var maxConcurrency = Environment.ProcessorCount;
using var semaphore = new SemaphoreSlim(maxConcurrency);

var tasks = allTests.Select(async test =>
{
    await semaphore.WaitAsync().ConfigureAwait(false);
    try
    {
        await _testBuilderPipeline.InvokePostResolutionEventsAsync(test).ConfigureAwait(false);
    }
    finally
    {
        semaphore.Release();
    }
});

await Task.WhenAll(tasks).ConfigureAwait(false);

However, this adds complexity and allocations, so it's only worth considering if profiling shows resource contention on legacy frameworks with very large test suites. The current implementation prioritizes simplicity, which is the right trade-off for now.

Why this matters: Performance optimizations should balance throughput gains against resource consumption. The current approach maximizes throughput on legacy frameworks at the cost of potentially unbounded concurrency. For the typical TUnit workload (post-resolution events are likely fast), this is fine. But if events become more resource-intensive (e.g., network calls, heavy I/O), bounded concurrency becomes more important.

Overall, this is solid work that meaningfully improves test discovery performance while maintaining code quality and safety.

@thomhurst thomhurst enabled auto-merge (squash) February 6, 2026 08:49
@thomhurst thomhurst merged commit c6f8f78 into main Feb 6, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the perf/parallelize-post-resolution-events branch February 6, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant