From 487f673400c1e301c5d62bf12176bf3237f455b5 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 28 May 2026 18:08:38 +0100 Subject: [PATCH] perf: pool HashSet in WaitingTestIndex.GetCandidatesForReleasedKeys (#6054) GetCandidatesForReleasedKeys allocated a fresh HashSet on every constraint-key release and the caller iterated then discarded it. Change the method to fill a caller-provided set, and have the single caller (ConstraintKeyScheduler) rent/return the set from the existing HashSetPool, eliminating the per-release allocation on a hot scheduling path. --- .../Framework/TUnitServiceProvider.cs | 3 +- .../Scheduling/ConstraintKeyScheduler.cs | 80 +++++++++++-------- TUnit.Engine/Scheduling/WaitingTestIndex.cs | 13 ++- 3 files changed, 55 insertions(+), 41 deletions(-) diff --git a/TUnit.Engine/Framework/TUnitServiceProvider.cs b/TUnit.Engine/Framework/TUnitServiceProvider.cs index e444900c78..a3be2e2ba8 100644 --- a/TUnit.Engine/Framework/TUnitServiceProvider.cs +++ b/TUnit.Engine/Framework/TUnitServiceProvider.cs @@ -256,7 +256,8 @@ public TUnitServiceProvider(IExtension extension, var constraintKeyScheduler = Register(new ConstraintKeyScheduler( testRunner, - Logger)); + Logger, + hashSetPool)); var staticPropertyHandler = Register(new StaticPropertyHandler(Logger, objectTracker, trackableObjectGraphProvider, disposer, lazyPropertyInjector, objectGraphDiscoveryService)); diff --git a/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs b/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs index d845cb82b6..5cddc0040f 100644 --- a/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs +++ b/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs @@ -9,13 +9,16 @@ internal sealed class ConstraintKeyScheduler : IConstraintKeyScheduler { private readonly TestRunner _testRunner; private readonly TUnitFrameworkLogger _logger; + private readonly HashSetPool _hashSetPool; public ConstraintKeyScheduler( TestRunner testRunner, - TUnitFrameworkLogger logger) + TUnitFrameworkLogger logger, + HashSetPool hashSetPool) { _testRunner = testRunner; _logger = logger; + _hashSetPool = hashSetPool; } #if NET8_0_OR_GREATER @@ -152,52 +155,63 @@ private async Task ExecuteTestAndReleaseKeysAsync( // Release the constraint keys and check if any waiting tests can now run var testsToStart = new List(); - lock (lockObject) + // Rent a pooled HashSet to collect candidates; returned immediately after we + // copy out the (priority-sorted) elements so we avoid allocating a fresh set + // per constraint-key release on hot scheduling paths. + var candidates = _hashSetPool.Rent(); + try { - // Release all constraint keys for this test - foreach (var key in constraintKeys) + lock (lockObject) { - lockedKeys.Remove(key); - } + // Release all constraint keys for this test + foreach (var key in constraintKeys) + { + lockedKeys.Remove(key); + } - // Only examine tests that are waiting on the keys we just released (O(k) lookup) - var candidates = waitingTestIndex.GetCandidatesForReleasedKeys(constraintKeys); + // Only examine tests that are waiting on the keys we just released (O(k) lookup) + waitingTestIndex.GetCandidatesForReleasedKeys(constraintKeys, candidates); - // Sort candidates by priority to respect ordering - // Use a simple list + sort rather than a SortedSet to avoid per-element allocation - var sortedCandidates = new List(candidates.Count); - sortedCandidates.AddRange(candidates); - sortedCandidates.Sort(static (a, b) => a.Priority.CompareTo(b.Priority)); + // Sort candidates by priority to respect ordering + // Use a simple list + sort rather than a SortedSet to avoid per-element allocation + var sortedCandidates = new List(candidates.Count); + sortedCandidates.AddRange(candidates); + sortedCandidates.Sort(static (a, b) => a.Priority.CompareTo(b.Priority)); - foreach (var candidate in sortedCandidates) - { - // Check if all constraint keys are available for this candidate - var canStart = true; - var waitingKeyCount = candidate.ConstraintKeys.Count; - for (var i = 0; i < waitingKeyCount; i++) + foreach (var candidate in sortedCandidates) { - if (lockedKeys.Contains(candidate.ConstraintKeys[i])) + // Check if all constraint keys are available for this candidate + var canStart = true; + var waitingKeyCount = candidate.ConstraintKeys.Count; + for (var i = 0; i < waitingKeyCount; i++) { - canStart = false; - break; + if (lockedKeys.Contains(candidate.ConstraintKeys[i])) + { + canStart = false; + break; + } } - } - if (canStart) - { - // Lock the keys for this test - for (var i = 0; i < waitingKeyCount; i++) + if (canStart) { - lockedKeys.Add(candidate.ConstraintKeys[i]); + // Lock the keys for this test + for (var i = 0; i < waitingKeyCount; i++) + { + lockedKeys.Add(candidate.ConstraintKeys[i]); + } + + // Remove from the index and mark for starting + waitingTestIndex.Remove(candidate); + testsToStart.Add(candidate); } - - // Remove from the index and mark for starting - waitingTestIndex.Remove(candidate); - testsToStart.Add(candidate); + // If can't start, leave it in the index for future key releases } - // If can't start, leave it in the index for future key releases } } + finally + { + _hashSetPool.Return(candidates); + } // Log and signal tests to start outside the lock if (_logger.IsTraceEnabled) diff --git a/TUnit.Engine/Scheduling/WaitingTestIndex.cs b/TUnit.Engine/Scheduling/WaitingTestIndex.cs index 84e1639e00..197975a0be 100644 --- a/TUnit.Engine/Scheduling/WaitingTestIndex.cs +++ b/TUnit.Engine/Scheduling/WaitingTestIndex.cs @@ -77,13 +77,14 @@ public void Remove(WaitingTest waitingTest) } /// - /// Returns a deduplicated set of waiting tests that are associated with any of the released keys. - /// These are candidates that might be unblocked (but still need to be checked against locked keys). + /// Populates the supplied set with a deduplicated collection of waiting tests + /// that are associated with any of the released keys. These are candidates that + /// might be unblocked (but still need to be checked against locked keys). + /// The caller owns the lifetime of (typically rented + /// from a pool) so this method avoids allocating a fresh set per key release. /// - public HashSet GetCandidatesForReleasedKeys(IReadOnlyList releasedKeys) + public void GetCandidatesForReleasedKeys(IReadOnlyList releasedKeys, HashSet candidates) { - var candidates = new HashSet(); - var keyCount = releasedKeys.Count; for (var i = 0; i < keyCount; i++) { @@ -93,7 +94,5 @@ public HashSet GetCandidatesForReleasedKeys(IReadOnlyList r candidates.UnionWith(tests); } } - - return candidates; } }