perf(engine): reduce lock contention in scheduling and hook caches (#…

…5686)

Three hot-path contention points observed in CPU profiles of ~1000-test
runs (Monitor.Enter_Slowpath at 4.33% exclusive, large
GetQueuedCompletionStatus idle stalls):

1. BeforeHookTaskCache.GetOrCreateBeforeClassTask and
   AfterHookPairTracker.GetOrCreateAfterClassTask guarded all classes
   behind one shared Lock, serialising unrelated classes. Replaced with
   the existing ThreadSafeDictionary<,>.GetOrAdd pattern (already used
   for GetOrCreateBeforeAssemblyTask), which internally uses Lazy<T> with
   ExecutionAndPublication to guarantee single-execution per key without
   a shared monitor. Keeps a lock-free TryGetValue fast path so the
   common cache-hit case doesn't allocate a closure.

2. TestScheduler.ExecuteTestsAsync unlimited branch called
   Parallel.ForEachAsync without MaxDegreeOfParallelism, which defaults
   to ProcessorCount but still queues near-simultaneous continuations on
   large test sets, saturating the IOCP thread. Explicit cap at
   Environment.ProcessorCount * 2 converges with the already-bounded
   limited-parallelism path.

EventReceiverRegistry single-lock (item 3 in the issue) left for a
follow-up as it's lower priority and the two above cover the bulk of
the observed contention.

TUnit.Engine/Scheduling/TestScheduler.cs

@@ -15,6 +15,12 @@ namespace TUnit.Engine.Scheduling;
 
 internal sealed class TestScheduler : ITestScheduler
 {
+    // Cap the unlimited-parallelism path to ProcessorCount * 2 to avoid oversubscribing
+    // the thread pool / IOCP when thousands of continuations are scheduled at once.
+    // Without this, Parallel.ForEachAsync defaults to ProcessorCount which still produces
+    // near-simultaneous continuations that saturate GetQueuedCompletionStatus under load.
+    private static readonly int UnlimitedPathMaxDop = Environment.ProcessorCount * 2;
+
     private readonly TUnitFrameworkLogger _logger;
     private readonly ITestGroupingService _groupingService;
     private readonly ITUnitMessageBus _messageBus;
@@ -321,10 +327,15 @@ private async Task ExecuteTestsAsync(
         {
 #if NET8_0_OR_GREATER
             // Use Parallel.ForEachAsync for bounded concurrency (eliminates unbounded Task.Run queue depth)
-            // This dramatically reduces ThreadPool contention and GetQueuedCompletionStatus waits
+            // Cap MaxDegreeOfParallelism to ProcessorCount * 2 even on the unlimited path so
+            // large test counts do not saturate the IOCP thread with simultaneous continuations.
             await Parallel.ForEachAsync(
                 tests,
-                new ParallelOptions { CancellationToken = cancellationToken },
+                new ParallelOptions
+                {
+                    MaxDegreeOfParallelism = UnlimitedPathMaxDop,
+                    CancellationToken = cancellationToken
+                },
                 async (test, ct) =>
                 {
                     test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask();

TUnit.Engine/Services/AfterHookPairTracker.cs

@@ -18,7 +18,6 @@ internal sealed class AfterHookPairTracker
     private readonly ThreadSafeDictionary<Assembly, Task<List<Exception>>> _afterAssemblyTasks = new();
     private Task<List<Exception>>? _afterTestSessionTask;
     private readonly Lock _testSessionLock = new();
-    private readonly Lock _classLock = new();
 
     // Ensure only the first call to RegisterAfterTestSessionHook registers a callback.
     // Subsequent calls (e.g. from per-test timeout tokens) are ignored so that
@@ -162,33 +161,31 @@ public ValueTask<List<Exception>> GetOrCreateAfterAssemblyTask(Assembly assembly
 
     /// <summary>
     /// Gets or creates the After Class task for the specified test class.
-    /// Thread-safe using double-checked locking.
+    /// Thread-safe via ThreadSafeDictionary's per-key Lazy initialization, which
+    /// guarantees single-execution without serializing unrelated classes behind a shared lock.
     /// Returns the exceptions from hook execution.
     /// </summary>
+    [UnconditionalSuppressMessage("Trimming", "IL2077",
+        Justification = "Type parameter is annotated at the method boundary and the closure invokes ExecuteAfterClassHooksAsync which requires the same annotation.")]
     public ValueTask<List<Exception>> GetOrCreateAfterClassTask(
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)]
         Type testClass,
         HookExecutor hookExecutor,
         CancellationToken cancellationToken)
     {
+        // Lock-free fast path avoids allocating a closure on the common cache-hit case.
         if (_afterClassTasks.TryGetValue(testClass, out var existingTask))
         {
             return new ValueTask<List<Exception>>(existingTask);
         }
 
-        lock (_classLock)
-        {
-            if (_afterClassTasks.TryGetValue(testClass, out existingTask))
-            {
-                return new ValueTask<List<Exception>>(existingTask);
-            }
-
-            // Call ExecuteAfterClassHooksAsync directly with the annotated testClass
-            // The factory ignores the key since we've already created the task with the annotated type
-            var newTask = hookExecutor.ExecuteAfterClassHooksAsync(testClass, cancellationToken).AsTask();
-            _afterClassTasks.GetOrAdd(testClass, _ => newTask);
-            return new ValueTask<List<Exception>>(newTask);
-        }
+        // ThreadSafeDictionary<,> internally uses Lazy<T> with ExecutionAndPublication,
+        // guaranteeing single-execution per key without serializing unrelated classes
+        // behind a shared lock.
+        var task = _afterClassTasks.GetOrAdd(
+            testClass,
+            _ => hookExecutor.ExecuteAfterClassHooksAsync(testClass, cancellationToken).AsTask());
+        return new ValueTask<List<Exception>>(task);
     }
 
     /// <summary>

TUnit.Engine/Services/BeforeHookTaskCache.cs

@@ -15,7 +15,6 @@ internal sealed class BeforeHookTaskCache
     private readonly ThreadSafeDictionary<Assembly, Task> _beforeAssemblyTasks = new();
     private Task? _beforeTestSessionTask;
     private readonly Lock _testSessionLock = new();
-    private readonly Lock _classLock = new();
 
     public ValueTask GetOrCreateBeforeTestSessionTask(Func<CancellationToken, ValueTask> taskFactory, CancellationToken cancellationToken)
     {
@@ -40,29 +39,26 @@ public ValueTask GetOrCreateBeforeAssemblyTask(Assembly assembly, Func<Assembly,
         return new ValueTask(task);
     }
 
+    [UnconditionalSuppressMessage("Trimming", "IL2077",
+        Justification = "Type parameter is annotated at the method boundary and the closure invokes ExecuteBeforeClassHooksAsync which requires the same annotation.")]
     public ValueTask GetOrCreateBeforeClassTask(
         [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)]
         Type testClass,
         HookExecutor hookExecutor,
         CancellationToken cancellationToken)
     {
+        // Lock-free fast path avoids allocating a closure on the common cache-hit case.
         if (_beforeClassTasks.TryGetValue(testClass, out var existingTask))
         {
             return new ValueTask(existingTask);
         }
 
-        lock (_classLock)
-        {
-            if (_beforeClassTasks.TryGetValue(testClass, out existingTask))
-            {
-                return new ValueTask(existingTask);
-            }
-
-            // Call ExecuteBeforeClassHooksAsync directly with the annotated testClass
-            // The factory ignores the key since we've already created the task with the annotated type
-            var newTask = hookExecutor.ExecuteBeforeClassHooksAsync(testClass, cancellationToken).AsTask();
-            _beforeClassTasks.GetOrAdd(testClass, _ => newTask);
-            return new ValueTask(newTask);
-        }
+        // ThreadSafeDictionary<,> internally uses Lazy<T> with ExecutionAndPublication,
+        // guaranteeing single-execution per key without serializing unrelated classes
+        // behind a shared lock.
+        var task = _beforeClassTasks.GetOrAdd(
+            testClass,
+            _ => hookExecutor.ExecuteBeforeClassHooksAsync(testClass, cancellationToken).AsTask());
+        return new ValueTask(task);
     }
 }