diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsCache.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsCache.cs index 869fd75b4b6e2..902f314bf8124 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/OptionsCache.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsCache.cs @@ -31,9 +31,9 @@ public class OptionsCache<[DynamicallyAccessedMembers(Options.DynamicallyAccesse public virtual TOptions GetOrAdd(string? name, Func createOptions!!) { name ??= Options.DefaultName; - Lazy? value; + Lazy value; -#if NETSTANDARD2_1 +#if NET || NETSTANDARD2_1 value = _cache.GetOrAdd(name, static (name, createOptions) => new Lazy(createOptions), createOptions); #else if (!_cache.TryGetValue(name, out value)) @@ -45,6 +45,28 @@ public virtual TOptions GetOrAdd(string? name, Func createOptions!!) return value.Value; } + internal TOptions GetOrAdd(string? name, Func createOptions, TArg factoryArgument) + { + // For compatibility, fall back to public GetOrAdd() if we're in a derived class. + // For simplicity, we do the same for older frameworks that don't support the factoryArgument overload of GetOrAdd(). +#if NET || NETSTANDARD2_1 + if (GetType() != typeof(OptionsCache)) +#endif + { + // copying captured variables to locals avoids allocating a closure if we don't enter the if + string? localName = name; + Func localCreateOptions = createOptions; + TArg localFactoryArgument = factoryArgument; + return GetOrAdd(name, () => localCreateOptions(localName ?? Options.DefaultName, localFactoryArgument)); + } + +#if NET || NETSTANDARD2_1 + return _cache.GetOrAdd( + name ?? Options.DefaultName, + static (name, arg) => new Lazy(arg.createOptions(name, arg.factoryArgument)), (createOptions, factoryArgument)).Value; +#endif + } + /// /// Gets a named options instance, if available. /// @@ -72,7 +94,7 @@ internal bool TryGetValue(string? name, [MaybeNullWhen(false)] out TOptions opti public virtual bool TryAdd(string? name, TOptions options!!) { return _cache.TryAdd(name ?? Options.DefaultName, new Lazy( -#if !NETSTANDARD2_1 +#if !(NET || NETSTANDARD2_1) () => #endif options)); diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsMonitor.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsMonitor.cs index d75bc70b1e51e..3f1c02900f629 100644 --- a/src/libraries/Microsoft.Extensions.Options/src/OptionsMonitor.cs +++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsMonitor.cs @@ -85,8 +85,17 @@ public TOptions CurrentValue /// public virtual TOptions Get(string? name) { - name = name ?? Options.DefaultName; - return _cache.GetOrAdd(name, () => _factory.Create(name)); + if (_cache is not OptionsCache optionsCache) + { + // copying captured variables to locals avoids allocating a closure if we don't enter the if + string localName = name ?? Options.DefaultName; + IOptionsFactory localFactory = _factory; + return _cache.GetOrAdd(localName, () => localFactory.Create(localName)); + } + + // non-allocating fast path + return optionsCache.GetOrAdd(name, static (name, factory) => factory.Create(name), _factory); + } /// diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsMonitorTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsMonitorTest.cs index 858b90dccd7ee..88c032a133077 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsMonitorTest.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsMonitorTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; @@ -418,5 +419,70 @@ public ChangeTokenSource(IChangeToken changeToken) public IChangeToken GetChangeToken() => _changeToken; } + + [Fact] + public void CallsPublicGetOrAddForCustomOptionsCache() + { + DerivedOptionsCache derivedOptionsCache = new(); + CreateMonitor(derivedOptionsCache).Get(null); + Assert.Equal(1, derivedOptionsCache.GetOrAddCalls); + + ImplementedOptionsCache implementedOptionsCache = new(); + CreateMonitor(implementedOptionsCache).Get(null); + Assert.Equal(1, implementedOptionsCache.GetOrAddCalls); + + static OptionsMonitor CreateMonitor(IOptionsMonitorCache cache) => + new OptionsMonitor( + new OptionsFactory(Enumerable.Empty>(), Enumerable.Empty>()), + Enumerable.Empty>(), + cache); + } + + private sealed class DerivedOptionsCache : OptionsCache + { + public int GetOrAddCalls { get; private set; } + + public override FakeOptions GetOrAdd(string? name, Func createOptions) + { + GetOrAddCalls++; + return base.GetOrAdd(name, createOptions); + } + } + + private sealed class ImplementedOptionsCache : IOptionsMonitorCache + { + public int GetOrAddCalls { get; private set; } + + public void Clear() => throw new NotImplementedException(); + + public FakeOptions GetOrAdd(string? name, Func createOptions) + { + GetOrAddCalls++; + return createOptions(); + } + + public bool TryAdd(string? name, FakeOptions options) => throw new NotImplementedException(); + + public bool TryRemove(string? name) => throw new NotImplementedException(); + } + +#if NET // need GC.GetAllocatedBytesForCurrentThread() + /// + /// Tests the fix for https://github.com/dotnet/runtime/issues/61086 + /// + [Fact] + public void TestCurrentValueDoesNotAllocateOnceValueIsCached() + { + var monitor = new OptionsMonitor( + new OptionsFactory(Enumerable.Empty>(), Enumerable.Empty>()), + Enumerable.Empty>(), + new OptionsCache()); + Assert.NotNull(monitor.CurrentValue); // populate the cache + + long initialBytes = GC.GetAllocatedBytesForCurrentThread(); + _ = monitor.CurrentValue; + Assert.Equal(0, GC.GetAllocatedBytesForCurrentThread() - initialBytes); + } +#endif } }