Skip to content

Commit 3368e82

Browse files
authored
perf: use GetOrAdd args overload to eliminate closure allocations in event receivers (thomhurst#5222)
Replace TryGetValue + GetOrAdd(closure) pattern with GetOrAdd<TArg>(static lambda, args) in First* event receiver methods. This removes redundant TryGetValue pre-checks (GetOrAdd already does this internally) and avoids closure allocation by passing context, sessionContext/assemblyContext/classContext, and cancellationToken as explicit tuple args to a static lambda.
1 parent 08a86f8 commit 3368e82

6 files changed

Lines changed: 37 additions & 99 deletions

TUnit.Core/Data/ThreadSafeDictionary.cs

Lines changed: 27 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Collections.Concurrent;
1+
using System.Collections.Concurrent;
22
using System.Diagnostics.CodeAnalysis;
33

44
namespace TUnit.Core.Data;
@@ -7,111 +7,69 @@ namespace TUnit.Core.Data;
77
/// Provides a thread-safe dictionary with lazy value initialization.
88
/// </summary>
99
/// <typeparam name="TKey">The type of keys in the dictionary. Must be non-null.</typeparam>
10-
/// <typeparam name="TValue">The type of values in the dictionary. Must have a public parameterless constructor.</typeparam>
10+
/// <typeparam name="TValue">The type of values in the dictionary.</typeparam>
1111
/// <remarks>
12-
/// <para>
1312
/// This class provides a concurrent dictionary where values are lazily initialized on first access.
1413
/// Each value is created exactly once per key, even when accessed concurrently from multiple threads.
15-
/// </para>
16-
/// <para>
17-
/// The lazy initialization is thread-safe and uses <see cref="LazyThreadSafetyMode.ExecutionAndPublication"/>
18-
/// to ensure that the factory function is executed only once per key, even under concurrent access.
19-
/// </para>
20-
/// <para>
21-
/// This class is marked as hidden in the editor browsable state in non-debug builds as it's primarily
22-
/// intended for internal use within the TUnit framework.
23-
/// </para>
2414
/// </remarks>
25-
/// <example>
26-
/// <code>
27-
/// var cache = new ThreadSafeDictionary&lt;string, ExpensiveResource&gt;();
28-
///
29-
/// // Values are created lazily on first access
30-
/// var resource1 = cache.GetOrAdd("key1", k => new ExpensiveResource(k));
31-
///
32-
/// // Same key returns the same instance (thread-safe)
33-
/// var resource2 = cache.GetOrAdd("key1", k => new ExpensiveResource(k));
34-
/// // resource1 == resource2
35-
/// </code>
36-
/// </example>
3715
#if !DEBUG
3816
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
3917
#endif
40-
public class ThreadSafeDictionary<TKey,
41-
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TValue>
18+
public class ThreadSafeDictionary<TKey, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TValue>
4219
where TKey : notnull
4320
{
4421
private readonly ConcurrentDictionary<TKey, Lazy<TValue>> _innerDictionary = new();
4522

4623
/// <summary>
4724
/// Gets a collection containing the keys in the dictionary.
4825
/// </summary>
49-
/// <value>A collection of keys present in the dictionary.</value>
5026
public ICollection<TKey> Keys => _innerDictionary.Keys;
5127

5228
/// <summary>
5329
/// Gets an enumerable collection of values in the dictionary.
5430
/// </summary>
55-
/// <value>An enumerable of initialized values.</value>
56-
/// <remarks>
57-
/// Accessing this property will force initialization of all lazy values in the dictionary.
58-
/// </remarks>
5931
public IEnumerable<TValue> Values => _innerDictionary.Values.Select(static lazy => lazy.Value);
6032

6133
/// <summary>
6234
/// Gets the value associated with the specified key, or creates it if it doesn't exist.
35+
/// The factory is guaranteed to run at most once per key even under concurrent access.
6336
/// </summary>
64-
/// <param name="key">The key of the value to get or create.</param>
65-
/// <param name="func">The factory function to create the value if the key doesn't exist.</param>
66-
/// <returns>
67-
/// The value for the key. This will be either the existing value or a newly created value from the factory function.
68-
/// </returns>
6937
/// <remarks>
70-
/// This method is thread-safe. If multiple threads call this method simultaneously with the same key,
71-
/// the factory function will be executed only once, and all threads will receive the same instance.
72-
/// This implementation uses a two-phase approach: TryGetValue for the fast path (when key exists),
73-
/// and GetOrAdd with a pre-created Lazy for the slow path (new key). This prevents the factory
74-
/// from being invoked multiple times during concurrent access.
38+
/// Uses a fast-path TryGetValue to avoid allocating a Lazy on repeated calls,
39+
/// then falls back to GetOrAdd with a pre-created Lazy to prevent the factory
40+
/// running multiple times during a race.
7541
/// </remarks>
7642
public TValue GetOrAdd(TKey key, Func<TKey, TValue> func)
7743
{
78-
// Fast path: Check if key already exists (lock-free read)
7944
if (_innerDictionary.TryGetValue(key, out var existingLazy))
8045
{
8146
return existingLazy.Value;
8247
}
8348

84-
return GetOrAddSlow(key, func);
49+
var newLazy = new Lazy<TValue>(() => func(key), LazyThreadSafetyMode.ExecutionAndPublication);
50+
var winning = _innerDictionary.GetOrAdd(key, newLazy);
51+
return winning.Value;
8552
}
8653

87-
private TValue GetOrAddSlow(TKey key, Func<TKey, TValue> func)
54+
/// <summary>
55+
/// Gets the value associated with the specified key, or creates it using the factory and arg if it doesn't exist.
56+
/// Avoids closure allocation at the call site by accepting the factory argument explicitly.
57+
/// </summary>
58+
public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> func, TArg arg)
8859
{
89-
// Slow path: Key not found, need to create
90-
// Create Lazy instance OUTSIDE of GetOrAdd to prevent factory from running during race
91-
var newLazy = new Lazy<TValue>(() => func(key), LazyThreadSafetyMode.ExecutionAndPublication);
92-
93-
// Use GetOrAdd with VALUE (not factory) - atomic operation that either:
94-
// 1. Adds our newLazy if key still doesn't exist
95-
// 2. Returns existing Lazy if another thread just added one
96-
var winningLazy = _innerDictionary.GetOrAdd(key, newLazy);
60+
if (_innerDictionary.TryGetValue(key, out var existingLazy))
61+
{
62+
return existingLazy.Value;
63+
}
9764

98-
// CRITICAL: Always return value from the Lazy that's actually in the dictionary
99-
// This ensures only ONE factory execution even if multiple Lazy instances were created
100-
return winningLazy.Value;
65+
var newLazy = new Lazy<TValue>(() => func(key, arg), LazyThreadSafetyMode.ExecutionAndPublication);
66+
var winning = _innerDictionary.GetOrAdd(key, newLazy);
67+
return winning.Value;
10168
}
10269

10370
/// <summary>
10471
/// Tries to get the value associated with the specified key.
10572
/// </summary>
106-
/// <param name="key">The key of the value to get.</param>
107-
/// <param name="value">
108-
/// When this method returns, contains the value associated with the specified key if found;
109-
/// otherwise, the default value for the type.
110-
/// </param>
111-
/// <returns><see langword="true"/> if the dictionary contains an element with the specified key; otherwise, <see langword="false"/>.</returns>
112-
/// <remarks>
113-
/// If the key exists, accessing the value will trigger lazy initialization if it hasn't occurred yet.
114-
/// </remarks>
11573
public bool TryGetValue(TKey key, [NotNullWhen(true)] out TValue? value)
11674
{
11775
if (_innerDictionary.TryGetValue(key, out var lazy))
@@ -120,40 +78,28 @@ public bool TryGetValue(TKey key, [NotNullWhen(true)] out TValue? value)
12078
return true;
12179
}
12280

123-
value = default!;
81+
value = default;
12482
return false;
12583
}
12684

12785
/// <summary>
12886
/// Removes the value with the specified key from the dictionary.
87+
/// Returns the value only if it had already been initialized; otherwise returns default.
12988
/// </summary>
130-
/// <param name="key">The key of the value to remove.</param>
131-
/// <returns>
132-
/// The value that was removed, or the default value for the type if the key was not found.
133-
/// </returns>
134-
/// <remarks>
135-
/// If the key exists and the value has been lazily initialized, that instance is returned.
136-
/// Otherwise, the default value for <typeparamref name="TValue"/> is returned.
137-
/// </remarks>
13889
public TValue? Remove(TKey key)
13990
{
140-
if (_innerDictionary.TryRemove(key, out var lazy))
91+
if (_innerDictionary.TryRemove(key, out var lazy) && lazy.IsValueCreated)
14192
{
14293
return lazy.Value;
14394
}
14495

145-
return default(TValue?);
96+
return default;
14697
}
14798

14899
/// <summary>
149100
/// Gets the value associated with the specified key.
150101
/// </summary>
151-
/// <param name="key">The key of the value to get.</param>
152-
/// <returns>The value associated with the specified key.</returns>
153-
/// <exception cref="KeyNotFoundException">Thrown when the specified key is not found in the dictionary.</exception>
154-
/// <remarks>
155-
/// Accessing this indexer will trigger lazy initialization of the value if it hasn't occurred yet.
156-
/// </remarks>
102+
/// <exception cref="KeyNotFoundException">Thrown when the specified key is not found.</exception>
157103
public TValue this[TKey key] => _innerDictionary.TryGetValue(key, out var lazy)
158104
? lazy.Value
159105
: throw new KeyNotFoundException($"Key '{key}' not found in dictionary");

TUnit.Engine/Services/EventReceiverOrchestrator.cs

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,9 @@ public ValueTask InvokeFirstTestInSessionEventReceiversAsync(
321321
return default;
322322
}
323323

324-
if (_firstTestInSessionTasks.TryGetValue("session", out var existingTask))
325-
{
326-
return new ValueTask(existingTask);
327-
}
328-
329324
var task = _firstTestInSessionTasks.GetOrAdd("session",
330-
_ => InvokeFirstTestInSessionEventReceiversCoreAsync(context, sessionContext, cancellationToken));
325+
static (_, args) => args.self.InvokeFirstTestInSessionEventReceiversCoreAsync(args.context, args.sessionContext, args.cancellationToken),
326+
(self: this, context, sessionContext, cancellationToken));
331327
return new ValueTask(task);
332328
}
333329

@@ -357,13 +353,9 @@ public ValueTask InvokeFirstTestInAssemblyEventReceiversAsync(
357353

358354
var assemblyName = assemblyContext.Assembly.GetName().FullName ?? "";
359355

360-
if (_firstTestInAssemblyTasks.TryGetValue(assemblyName, out var existingTask))
361-
{
362-
return new ValueTask(existingTask);
363-
}
364-
365356
var task = _firstTestInAssemblyTasks.GetOrAdd(assemblyName,
366-
_ => InvokeFirstTestInAssemblyEventReceiversCoreAsync(context, assemblyContext, cancellationToken));
357+
static (_, args) => args.self.InvokeFirstTestInAssemblyEventReceiversCoreAsync(args.context, args.assemblyContext, args.cancellationToken),
358+
(self: this, context, assemblyContext, cancellationToken));
367359
return new ValueTask(task);
368360
}
369361

@@ -393,13 +385,9 @@ public ValueTask InvokeFirstTestInClassEventReceiversAsync(
393385

394386
var classType = classContext.ClassType;
395387

396-
if (_firstTestInClassTasks.TryGetValue(classType, out var existingTask))
397-
{
398-
return new ValueTask(existingTask);
399-
}
400-
401388
var task = _firstTestInClassTasks.GetOrAdd(classType,
402-
_ => InvokeFirstTestInClassEventReceiversCoreAsync(context, classContext, cancellationToken));
389+
static (_, args) => args.self.InvokeFirstTestInClassEventReceiversCoreAsync(args.context, args.classContext, args.cancellationToken),
390+
(self: this, context, classContext, cancellationToken));
403391
return new ValueTask(task);
404392
}
405393

TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet10_0.verified.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,7 @@ namespace .Data
17871787
public .<TKey> Keys { get; }
17881788
public .<TValue> Values { get; }
17891789
public TValue GetOrAdd(TKey key, <TKey, TValue> func) { }
1790+
public TValue GetOrAdd<TArg>(TKey key, <TKey, TArg, TValue> func, TArg arg) { }
17901791
public TValue? Remove(TKey key) { }
17911792
public bool TryGetValue(TKey key, [.(true)] out TValue? value) { }
17921793
}

TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet8_0.verified.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,7 @@ namespace .Data
17871787
public .<TKey> Keys { get; }
17881788
public .<TValue> Values { get; }
17891789
public TValue GetOrAdd(TKey key, <TKey, TValue> func) { }
1790+
public TValue GetOrAdd<TArg>(TKey key, <TKey, TArg, TValue> func, TArg arg) { }
17901791
public TValue? Remove(TKey key) { }
17911792
public bool TryGetValue(TKey key, [.(true)] out TValue? value) { }
17921793
}

TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet9_0.verified.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,7 @@ namespace .Data
17871787
public .<TKey> Keys { get; }
17881788
public .<TValue> Values { get; }
17891789
public TValue GetOrAdd(TKey key, <TKey, TValue> func) { }
1790+
public TValue GetOrAdd<TArg>(TKey key, <TKey, TArg, TValue> func, TArg arg) { }
17901791
public TValue? Remove(TKey key) { }
17911792
public bool TryGetValue(TKey key, [.(true)] out TValue? value) { }
17921793
}

TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.Net4_7.verified.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,6 +1733,7 @@ namespace .Data
17331733
public .<TKey> Keys { get; }
17341734
public .<TValue> Values { get; }
17351735
public TValue GetOrAdd(TKey key, <TKey, TValue> func) { }
1736+
public TValue GetOrAdd<TArg>(TKey key, <TKey, TArg, TValue> func, TArg arg) { }
17361737
public TValue? Remove(TKey key) { }
17371738
public bool TryGetValue(TKey key, [.(true)] out TValue? value) { }
17381739
}

0 commit comments

Comments
 (0)