perf: improve lock contention and async overhead (#3693)

thomhurst · web-flow · commit bb263413d1aa · 2025-11-04T17:57:21.000Z
* perf: improve lock contention and async overhead

* refactor: streamline property injection logic by removing redundant comments
diff --git a/TUnit.Engine/Interfaces/ITestCoordinator.cs b/TUnit.Engine/Interfaces/ITestCoordinator.cs
@@ -9,5 +9,5 @@ namespace TUnit.Engine.Interfaces;
 /// </summary>
 internal interface ITestCoordinator
 {
-    Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken);
+    ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken);
 }
diff --git a/TUnit.Engine/Scheduling/TestRunner.cs b/TUnit.Engine/Scheduling/TestRunner.cs
@@ -39,14 +39,14 @@ internal TestRunner(
     private readonly ThreadSafeDictionary<string, Task> _executingTests = new();
     private Exception? _firstFailFastException;
 
-    public async Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
+    public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
         // Prevent double execution with a simple lock
-        var executionTask = _executingTests.GetOrAdd(test.TestId, _ => ExecuteTestInternalAsync(test, cancellationToken));
+        var executionTask = _executingTests.GetOrAdd(test.TestId, _ => ExecuteTestInternalAsync(test, cancellationToken).AsTask());
         await executionTask.ConfigureAwait(false);
     }
 
-    private async Task ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
+    private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
         try
         {
diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs
@@ -420,7 +420,7 @@ private async Task ExecuteWithGlobalLimitAsync(
 
                     try
                     {
-                        test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken);
+                        test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken).AsTask();
                         await test.ExecutionTask.ConfigureAwait(false);
                     }
                     finally
@@ -458,9 +458,7 @@ await Parallel.ForEachAsync(
 
                 try
                 {
-#pragma warning disable IL2026 // ExecuteTestAsync uses reflection, but caller (ExecuteWithGlobalLimitAsync) is already marked with RequiresUnreferencedCode
-                    test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct);
-#pragma warning restore IL2026
+                    test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask();
                     await test.ExecutionTask.ConfigureAwait(false);
                 }
                 finally
@@ -485,9 +483,7 @@ await Parallel.ForEachAsync(
             },
             async (test, ct) =>
             {
-#pragma warning disable IL2026 // ExecuteTestAsync uses reflection, but caller (ExecuteWithGlobalLimitAsync) is already marked with RequiresUnreferencedCode
-                test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct);
-#pragma warning restore IL2026
+                test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask();
                 await test.ExecutionTask.ConfigureAwait(false);
             }
         ).ConfigureAwait(false);
diff --git a/TUnit.Engine/Services/DataSourceInitializer.cs b/TUnit.Engine/Services/DataSourceInitializer.cs
@@ -12,8 +12,7 @@ namespace TUnit.Engine.Services;
 /// </summary>
 internal sealed class DataSourceInitializer
 {
-    private readonly Dictionary<object, Task> _initializationTasks = new();
-    private readonly object _lock = new();
+    private readonly ConcurrentDictionary<object, Lazy<Task>> _initializationTasks = new();
     private PropertyInjectionService? _propertyInjectionService;
 
     /// <summary>
@@ -42,30 +41,22 @@ public async Task<T> EnsureInitializedAsync<T>(
         }
 
         // Check if already initialized or being initialized
-        Task? existingTask;
-        lock (_lock)
-        {
-            if (_initializationTasks.TryGetValue(dataSource, out existingTask))
-            {
-                // Already initialized or being initialized
-            }
-            else
-            {
-                // Start initialization
-                existingTask = InitializeDataSourceAsync(dataSource, objectBag, methodMetadata, events, cancellationToken);
-                _initializationTasks[dataSource] = existingTask;
-            }
-        }
+        // Use Lazy<Task> to ensure only one initialization task is created per data source (thread-safe)
+        var lazyTask = _initializationTasks.GetOrAdd(
+            dataSource,
+            _ => new Lazy<Task>(() => InitializeDataSourceAsync(dataSource, objectBag, methodMetadata, events, cancellationToken)));
+
+        var task = lazyTask.Value;
 
         // Wait for initialization with cancellation support
         if (cancellationToken.CanBeCanceled)
         {
-            await existingTask.ConfigureAwait(false);
+            await task.ConfigureAwait(false);
             cancellationToken.ThrowIfCancellationRequested();
         }
         else
         {
-            await existingTask.ConfigureAwait(false);
+            await task.ConfigureAwait(false);
         }
 
         return dataSource;
@@ -221,9 +212,6 @@ private void CollectNestedObjects(
     /// </summary>
     public void ClearCache()
     {
-        lock (_lock)
-        {
-            _initializationTasks.Clear();
-        }
+        _initializationTasks.Clear();
     }
 }
diff --git a/TUnit.Engine/Services/PropertyInjectionService.cs b/TUnit.Engine/Services/PropertyInjectionService.cs
@@ -40,7 +40,6 @@ public async Task InjectPropertiesIntoArgumentsAsync(object?[] arguments, Concur
             return;
         }
 
-        // Fast path: check if any arguments need injection
         var injectableArgs = arguments
             .Where(argument => argument != null && PropertyInjectionCache.HasInjectableProperties(argument.GetType()))
             .ToArray();
@@ -50,7 +49,6 @@ public async Task InjectPropertiesIntoArgumentsAsync(object?[] arguments, Concur
             return;
         }
 
-        // Process arguments in parallel
         var argumentTasks = injectableArgs
             .Select(argument => InjectPropertiesIntoObjectAsync(argument!, objectBag, methodMetadata, events))
             .ToArray();
@@ -80,7 +78,6 @@ public Task InjectPropertiesIntoObjectAsync(object instance, ConcurrentDictionar
             throw new ArgumentNullException(nameof(events), "TestContextEvents must not be null. Each test permutation must have a unique TestContextEvents instance for proper disposal tracking.");
         }
 
-        // Start with an empty visited set for cycle detection
 #if NETSTANDARD2_0
         var visitedObjects = new ConcurrentDictionary<object, byte>();
 #else
@@ -96,8 +93,7 @@ internal async Task InjectPropertiesIntoObjectAsyncCore(object instance, Concurr
             return;
         }
 
-        // Prevent cycles - if we're already processing this object, skip it
-        // TryAdd returns false if the key already exists (thread-safe)
+        // Prevent cycles
         if (!visitedObjects.TryAdd(instance, 0))
         {
             return;
@@ -117,13 +113,11 @@ await PropertyInjectionCache.GetOrAddInjectionTask(instance, async _ =>
                 {
                     var plan = PropertyInjectionCache.GetOrCreatePlan(instance.GetType());
 
-                    // Use the orchestrator for property initialization
                     await _orchestrator.InitializeObjectWithPropertiesAsync(
                         instance, plan, objectBag, methodMetadata, events, visitedObjects);
                 });
             }
 
-            // After properties are initialized, recursively inject nested properties
             await RecurseIntoNestedPropertiesAsync(instance, objectBag, methodMetadata, events, visitedObjects);
         }
         catch (Exception ex)
diff --git a/TUnit.Engine/Services/TestExecution/HashSetPool.cs b/TUnit.Engine/Services/TestExecution/HashSetPool.cs
@@ -1,34 +1,26 @@
+using System.Collections.Concurrent;
+
 namespace TUnit.Engine.Services.TestExecution;
 
 /// <summary>
 /// Thread-safe object pool for HashSet instances used during test execution.
 /// Single Responsibility: Managing pooled HashSet objects to reduce allocations.
+/// Uses lock-free concurrent collections for high-performance parallel test execution.
 /// </summary>
 internal sealed class HashSetPool
 {
-    private readonly Dictionary<Type, object> _pools = new();
-    private readonly object _lock = new();
+    private readonly ConcurrentDictionary<Type, ConcurrentBag<object>> _pools = new();
 
     public HashSet<T> Rent<T>()
     {
         var type = typeof(T);
+        var bag = _pools.GetOrAdd(type, _ => new ConcurrentBag<object>());
 
-        lock (_lock)
+        if (bag.TryTake(out var pooledSet))
         {
-            if (!_pools.TryGetValue(type, out var poolObj))
-            {
-                poolObj = new Stack<HashSet<T>>();
-                _pools[type] = poolObj;
-            }
-
-            var pool = (Stack<HashSet<T>>)poolObj;
-
-            if (pool.Count > 0)
-            {
-                var set = pool.Pop();
-                set.Clear();
-                return set;
-            }
+            var set = (HashSet<T>)pooledSet;
+            set.Clear();
+            return set;
         }
 
         return [];
@@ -37,19 +29,9 @@ public HashSet<T> Rent<T>()
     public void Return<T>(HashSet<T> set)
     {
         var type = typeof(T);
+        set.Clear();
 
-        lock (_lock)
-        {
-            set.Clear();
-
-            if (!_pools.TryGetValue(type, out var poolObj))
-            {
-                poolObj = new Stack<HashSet<T>>();
-                _pools[type] = poolObj;
-            }
-
-            var pool = (Stack<HashSet<T>>)poolObj;
-            pool.Push(set);
-        }
+        var bag = _pools.GetOrAdd(type, _ => new ConcurrentBag<object>());
+        bag.Add(set);
     }
 }
diff --git a/TUnit.Engine/Services/TestExecution/TestCoordinator.cs b/TUnit.Engine/Services/TestExecution/TestCoordinator.cs
@@ -50,13 +50,13 @@ public TestCoordinator(
         _hashSetPool = hashSetPool;
     }
 
-    public async Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
+    public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
         await _executionGuard.TryStartExecutionAsync(test.TestId,
-            () => ExecuteTestInternalAsync(test, cancellationToken));
+            () => ExecuteTestInternalAsync(test, cancellationToken).AsTask());
     }
 
-    private async Task ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
+    private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
         try
         {
diff --git a/TUnit.Engine/TestExecutor.cs b/TUnit.Engine/TestExecutor.cs
@@ -51,7 +51,7 @@ await _beforeHookTaskCache.GetOrCreateBeforeTestSessionTask(() =>
     /// Creates a test executor delegate that wraps the provided executor with hook orchestration.
     /// Uses focused services that follow SRP to manage lifecycle and execution.
     /// </summary>
-    public async Task ExecuteAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)
+    public async ValueTask ExecuteAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)
     {
 
         var testClass = executableTest.Metadata.TestClassType;
@@ -148,7 +148,7 @@ await _eventReceiverOrchestrator.InvokeFirstTestInClassEventReceiversAsync(
         }
     }
 
-    private static async Task ExecuteTestAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)
+    private static async ValueTask ExecuteTestAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)
     {
         // Skip the actual test invocation for skipped tests
         if (executableTest.Context.Metadata.TestDetails.ClassInstance is SkippedTestInstance ||
diff --git a/TUnit.Engine/TestInitializer.cs b/TUnit.Engine/TestInitializer.cs
@@ -21,7 +21,7 @@ public TestInitializer(EventReceiverOrchestrator eventReceiverOrchestrator,
         _objectTracker = objectTracker;
     }
 
-    public async Task InitializeTest(AbstractExecutableTest test, CancellationToken cancellationToken)
+    public async ValueTask InitializeTest(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
         var testClassInstance = test.Context.Metadata.TestDetails.ClassInstance;
 

Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,5 @@ namespace TUnit.Engine.Interfaces;`
`9`	`9`	`/// </summary>`
`10`	`10`	`internal interface ITestCoordinator`
`11`	`11`	`{`
`12`		`- Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken);`
	`12`	`+ ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken);`
`13`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,14 +39,14 @@ internal TestRunner(`
`39`	`39`	`private readonly ThreadSafeDictionary<string, Task> _executingTests = new();`
`40`	`40`	`private Exception? _firstFailFastException;`
`41`	`41`
`42`		`- public async Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
	`42`	`+ public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
`43`	`43`	`{`
`44`	`44`	`// Prevent double execution with a simple lock`
`45`		`- var executionTask = _executingTests.GetOrAdd(test.TestId, _ => ExecuteTestInternalAsync(test, cancellationToken));`
	`45`	`+ var executionTask = _executingTests.GetOrAdd(test.TestId, _ => ExecuteTestInternalAsync(test, cancellationToken).AsTask());`
`46`	`46`	`await executionTask.ConfigureAwait(false);`
`47`	`47`	`}`
`48`	`48`
`49`		`- private async Task ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
	`49`	`+ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
`50`	`50`	`{`
`51`	`51`	`try`
`52`	`52`	`{`
Original file line number	Diff line number	Diff line change
`@@ -420,7 +420,7 @@ private async Task ExecuteWithGlobalLimitAsync(`
`420`	`420`
`421`	`421`	`try`
`422`	`422`	`{`
`423`		`- test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken);`
	`423`	`+ test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken).AsTask();`
`424`	`424`	`await test.ExecutionTask.ConfigureAwait(false);`
`425`	`425`	`}`
`426`	`426`	`finally`
`@@ -458,9 +458,7 @@ await Parallel.ForEachAsync(`
`458`	`458`
`459`	`459`	`try`
`460`	`460`	`{`
`461`		`-#pragma warning disable IL2026 // ExecuteTestAsync uses reflection, but caller (ExecuteWithGlobalLimitAsync) is already marked with RequiresUnreferencedCode`
`462`		`- test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct);`
`463`		`-#pragma warning restore IL2026`
	`461`	`+ test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask();`
`464`	`462`	`await test.ExecutionTask.ConfigureAwait(false);`
`465`	`463`	`}`
`466`	`464`	`finally`
`@@ -485,9 +483,7 @@ await Parallel.ForEachAsync(`
`485`	`483`	`},`
`486`	`484`	`async (test, ct) =>`
`487`	`485`	`{`
`488`		`-#pragma warning disable IL2026 // ExecuteTestAsync uses reflection, but caller (ExecuteWithGlobalLimitAsync) is already marked with RequiresUnreferencedCode`
`489`		`- test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct);`
`490`		`-#pragma warning restore IL2026`
	`486`	`+ test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask();`
`491`	`487`	`await test.ExecutionTask.ConfigureAwait(false);`
`492`	`488`	`}`
`493`	`489`	`).ConfigureAwait(false);`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,6 @@ public async Task InjectPropertiesIntoArgumentsAsync(object?[] arguments, Concur`
`40`	`40`	`return;`
`41`	`41`	`}`
`42`	`42`
`43`		`- // Fast path: check if any arguments need injection`
`44`	`43`	`var injectableArgs = arguments`
`45`	`44`	`.Where(argument => argument != null && PropertyInjectionCache.HasInjectableProperties(argument.GetType()))`
`46`	`45`	`.ToArray();`
`@@ -50,7 +49,6 @@ public async Task InjectPropertiesIntoArgumentsAsync(object?[] arguments, Concur`
`50`	`49`	`return;`
`51`	`50`	`}`
`52`	`51`
`53`		`- // Process arguments in parallel`
`54`	`52`	`var argumentTasks = injectableArgs`
`55`	`53`	`.Select(argument => InjectPropertiesIntoObjectAsync(argument!, objectBag, methodMetadata, events))`
`56`	`54`	`.ToArray();`
`@@ -80,7 +78,6 @@ public Task InjectPropertiesIntoObjectAsync(object instance, ConcurrentDictionar`
`80`	`78`	`throw new ArgumentNullException(nameof(events), "TestContextEvents must not be null. Each test permutation must have a unique TestContextEvents instance for proper disposal tracking.");`
`81`	`79`	`}`
`82`	`80`
`83`		`- // Start with an empty visited set for cycle detection`
`84`	`81`	`#if NETSTANDARD2_0`
`85`	`82`	`var visitedObjects = new ConcurrentDictionary<object, byte>();`
`86`	`83`	`#else`
`@@ -96,8 +93,7 @@ internal async Task InjectPropertiesIntoObjectAsyncCore(object instance, Concurr`
`96`	`93`	`return;`
`97`	`94`	`}`
`98`	`95`
`99`		`- // Prevent cycles - if we're already processing this object, skip it`
`100`		`- // TryAdd returns false if the key already exists (thread-safe)`
	`96`	`+ // Prevent cycles`
`101`	`97`	`if (!visitedObjects.TryAdd(instance, 0))`
`102`	`98`	`{`
`103`	`99`	`return;`
`@@ -117,13 +113,11 @@ await PropertyInjectionCache.GetOrAddInjectionTask(instance, async _ =>`
`117`	`113`	`{`
`118`	`114`	`var plan = PropertyInjectionCache.GetOrCreatePlan(instance.GetType());`
`119`	`115`
`120`		`- // Use the orchestrator for property initialization`
`121`	`116`	`await _orchestrator.InitializeObjectWithPropertiesAsync(`
`122`	`117`	`instance, plan, objectBag, methodMetadata, events, visitedObjects);`
`123`	`118`	`});`
`124`	`119`	`}`
`125`	`120`
`126`		`- // After properties are initialized, recursively inject nested properties`
`127`	`121`	`await RecurseIntoNestedPropertiesAsync(instance, objectBag, methodMetadata, events, visitedObjects);`
`128`	`122`	`}`
`129`	`123`	`catch (Exception ex)`
Original file line number	Diff line number	Diff line change
`@@ -50,13 +50,13 @@ public TestCoordinator(`
`50`	`50`	`_hashSetPool = hashSetPool;`
`51`	`51`	`}`
`52`	`52`
`53`		`- public async Task ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
	`53`	`+ public async ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
`54`	`54`	`{`
`55`	`55`	`await _executionGuard.TryStartExecutionAsync(test.TestId,`
`56`		`- () => ExecuteTestInternalAsync(test, cancellationToken));`
	`56`	`+ () => ExecuteTestInternalAsync(test, cancellationToken).AsTask());`
`57`	`57`	`}`
`58`	`58`
`59`		`- private async Task ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
	`59`	`+ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, CancellationToken cancellationToken)`
`60`	`60`	`{`
`61`	`61`	`try`
`62`	`62`	`{`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ await _beforeHookTaskCache.GetOrCreateBeforeTestSessionTask(() =>`
`51`	`51`	`/// Creates a test executor delegate that wraps the provided executor with hook orchestration.`
`52`	`52`	`/// Uses focused services that follow SRP to manage lifecycle and execution.`
`53`	`53`	`/// </summary>`
`54`		`- public async Task ExecuteAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)`
	`54`	`+ public async ValueTask ExecuteAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)`
`55`	`55`	`{`
`56`	`56`
`57`	`57`	`var testClass = executableTest.Metadata.TestClassType;`
`@@ -148,7 +148,7 @@ await _eventReceiverOrchestrator.InvokeFirstTestInClassEventReceiversAsync(`
`148`	`148`	`}`
`149`	`149`	`}`
`150`	`150`
`151`		`- private static async Task ExecuteTestAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)`
	`151`	`+ private static async ValueTask ExecuteTestAsync(AbstractExecutableTest executableTest, CancellationToken cancellationToken)`
`152`	`152`	`{`
`153`	`153`	`// Skip the actual test invocation for skipped tests`
`154`	`154`	`if (executableTest.Context.Metadata.TestDetails.ClassInstance is SkippedTestInstance \|\|`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ public TestInitializer(EventReceiverOrchestrator eventReceiverOrchestrator,`
`21`	`21`	`_objectTracker = objectTracker;`
`22`	`22`	`}`
`23`	`23`
`24`		`- public async Task InitializeTest(AbstractExecutableTest test, CancellationToken cancellationToken)`
	`24`	`+ public async ValueTask InitializeTest(AbstractExecutableTest test, CancellationToken cancellationToken)`
`25`	`25`	`{`
`26`	`26`	`var testClassInstance = test.Context.Metadata.TestDetails.ClassInstance;`
`27`	`27`