Skip to content

Commit 9f800da

Browse files
authored
Special case default Options in OptionsCache for better perf
1 parent d4c1aa9 commit 9f800da

File tree

2 files changed

+79
-7
lines changed

2 files changed

+79
-7
lines changed

src/libraries/Microsoft.Extensions.Options/src/OptionsCache.cs

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@ public class OptionsCache<[DynamicallyAccessedMembers(Options.DynamicallyAccesse
1616
where TOptions : class
1717
{
1818
private readonly ConcurrentDictionary<string, Lazy<TOptions>> _cache = new ConcurrentDictionary<string, Lazy<TOptions>>(concurrencyLevel: 1, capacity: 31, StringComparer.Ordinal); // 31 == default capacity
19+
private Lazy<TOptions>? _defaultOptions = null;
1920

2021
/// <summary>
2122
/// Clears all options instances from the cache.
2223
/// </summary>
23-
public void Clear() => _cache.Clear();
24+
public void Clear()
25+
{
26+
_defaultOptions = null;
27+
_cache.Clear();
28+
}
2429

2530
/// <summary>
2631
/// Gets a named options instance, or adds a new instance created with <paramref name="createOptions"/>.
@@ -35,6 +40,21 @@ public virtual TOptions GetOrAdd(string? name, Func<TOptions> createOptions)
3540
name ??= Options.DefaultName;
3641
Lazy<TOptions> value;
3742

43+
if (name == Options.DefaultName)
44+
{
45+
if (_defaultOptions is null)
46+
{
47+
// We need a reference to the new instance to be able to return it. Usage of `return _defaultOptions.Value`
48+
// could technically save us some allocations but it would have a risk of sneaky race condition of .Clear
49+
// being called between the Interlocked.CompareExchange call assigning new value and the return, leading to NRE.
50+
var newDefaultOptions = new Lazy<TOptions>(createOptions);
51+
var result = Interlocked.CompareExchange(ref _defaultOptions, newDefaultOptions, null);
52+
53+
return result is not null ? result.Value : newDefaultOptions.Value;
54+
}
55+
return _defaultOptions.Value;
56+
}
57+
3858
#if NET || NETSTANDARD2_1
3959
value = _cache.GetOrAdd(name, static (name, createOptions) => new Lazy<TOptions>(createOptions), createOptions);
4060
#else
@@ -51,6 +71,22 @@ internal TOptions GetOrAdd<TArg>(string? name, Func<string, TArg, TOptions> crea
5171
{
5272
// For compatibility, fall back to public GetOrAdd() if we're in a derived class.
5373
// For simplicity, we do the same for older frameworks that don't support the factoryArgument overload of GetOrAdd().
74+
name ??= Options.DefaultName;
75+
if (name == Options.DefaultName)
76+
{
77+
if (_defaultOptions is null)
78+
{
79+
// We need a reference to the new instance to be able to return it. Usage of `return _defaultOptions.Value`
80+
// could technically save us some allocations but it would have a risk of sneaky race condition of .Clear
81+
// being called between the Interlocked.CompareExchange call assigning new value and the return, leading to NRE.
82+
var newDefaultOptions = new Lazy<TOptions>(() => createOptions(Options.DefaultName, factoryArgument));
83+
var result = Interlocked.CompareExchange(ref _defaultOptions, newDefaultOptions, null);
84+
85+
return result is not null ? result.Value : newDefaultOptions.Value;
86+
}
87+
return _defaultOptions.Value;
88+
}
89+
5490
#if NET || NETSTANDARD2_1
5591
if (GetType() != typeof(OptionsCache<TOptions>))
5692
#endif
@@ -59,7 +95,7 @@ internal TOptions GetOrAdd<TArg>(string? name, Func<string, TArg, TOptions> crea
5995
string? localName = name;
6096
Func<string, TArg, TOptions> localCreateOptions = createOptions;
6197
TArg localFactoryArgument = factoryArgument;
62-
return GetOrAdd(name, () => localCreateOptions(localName ?? Options.DefaultName, localFactoryArgument));
98+
return GetOrAdd(name, () => localCreateOptions(localName, localFactoryArgument));
6399
}
64100

65101
#if NET || NETSTANDARD2_1
@@ -77,7 +113,19 @@ internal TOptions GetOrAdd<TArg>(string? name, Func<string, TArg, TOptions> crea
77113
/// <returns><see langword="true"/> if the options were retrieved; otherwise, <see langword="false"/>.</returns>
78114
internal bool TryGetValue(string? name, [MaybeNullWhen(false)] out TOptions options)
79115
{
80-
if (_cache.TryGetValue(name ?? Options.DefaultName, out Lazy<TOptions>? lazy))
116+
name ??= Options.DefaultName;
117+
if (name == Options.DefaultName)
118+
{
119+
if (_defaultOptions is { } defaultOptions)
120+
{
121+
options = defaultOptions.Value;
122+
return true;
123+
}
124+
options = default;
125+
return false;
126+
}
127+
128+
if (_cache.TryGetValue(name, out Lazy<TOptions>? lazy))
81129
{
82130
options = lazy.Value;
83131
return true;
@@ -97,7 +145,18 @@ public virtual bool TryAdd(string? name, TOptions options)
97145
{
98146
ArgumentNullException.ThrowIfNull(options);
99147

100-
return _cache.TryAdd(name ?? Options.DefaultName, new Lazy<TOptions>(
148+
name ??= Options.DefaultName;
149+
if (name == Options.DefaultName)
150+
{
151+
if (_defaultOptions is not null)
152+
{
153+
return false; // Default options already exist
154+
}
155+
var result = Interlocked.CompareExchange(ref _defaultOptions, new Lazy<TOptions>(() => options), null);
156+
return result is null;
157+
}
158+
159+
return _cache.TryAdd(name, new Lazy<TOptions>(
101160
#if !(NET || NETSTANDARD2_1)
102161
() =>
103162
#endif
@@ -109,7 +168,20 @@ public virtual bool TryAdd(string? name, TOptions options)
109168
/// </summary>
110169
/// <param name="name">The name of the options instance.</param>
111170
/// <returns><see langword="true"/> if anything was removed; otherwise, <see langword="false"/>.</returns>
112-
public virtual bool TryRemove(string? name) =>
113-
_cache.TryRemove(name ?? Options.DefaultName, out _);
171+
public virtual bool TryRemove(string? name)
172+
{
173+
name ??= Options.DefaultName;
174+
if (name == Options.DefaultName)
175+
{
176+
if (_defaultOptions is not null)
177+
{
178+
var result = Interlocked.Exchange(ref _defaultOptions, null);
179+
return result is not null;
180+
}
181+
return false;
182+
}
183+
184+
return _cache.TryRemove(name, out _);
185+
}
114186
}
115187
}

src/libraries/Microsoft.Extensions.Options/src/OptionsMonitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public void Dispose()
127127
_registrations.Clear();
128128
}
129129

130-
internal sealed class ChangeTrackerDisposable : IDisposable
130+
private sealed class ChangeTrackerDisposable : IDisposable
131131
{
132132
private readonly Action<TOptions, string> _listener;
133133
private readonly OptionsMonitor<TOptions> _monitor;

0 commit comments

Comments
 (0)