Looks simple enough.

Can you remind me what costs this is trying to address?

The current implementation is shared between "named" and "unnamed" Options. When using "named" options, we need to use a cache (ConcurrentDictionary):

In the simple case where you never use a named option, we still create the dictionary, and check it to get the value.

Here, when you never use a named option, we can simply use a Lazy, saving the dictionary alloc and access.

saving the dictionary alloc

This is why I was asking though. If the goal is to make this cheaper when it's never used, and we're going to be creating a lot of these so that it matters, there are cheaper ways. Lazy still allocates, and in the current usage, there's not only the allocation for the Lazy itself, there's the internal LazyHelper that'll be allocated to serve as the lock object for the double-checked locking it employs by default, and there's the closure allocation to capture the factory, and there's the delegate allocation for the method on the closure. If it's never accessed, yeah, it's a tad cheaper, but not as much as you might think (and if it is accessed, you've basically doubled the allocation cost).

[Benchmark]
public object CreateLazy()
{
    return CreateLazyWithClosure(s => new ConcurrentDictionary<string, string>(1, 0));

    static Lazy<T> CreateLazyWithClosure<T>(Func<string, T> factory) =>
        new Lazy<T>(() => factory(""));
}

[Benchmark]
public object CreateDict() => new ConcurrentDictionary<string, string>(1, 0);

[Benchmark]
public object CreateLazyAndAccess() => ((Lazy<ConcurrentDictionary<string, string>>)CreateLazy()).Value;

Is it better to use LazyInitializer and keep the state in the implementation?

Ah I re-read your statement, I thought you were saying the ConcurrentDictionary was cheaper. You're just saying they are cheaper ways to avoid the lazy overhead. I thought about it a little but figured I'd let you comment and tell me what the optimal way was 😉

#49883

I really hope people aren't doing this, but I gotta ask: do we know if people get IOptions<T>, and then cast/as it either to OptionsManager<T> or IOptionsSnapshot<T>?

I see OptionsManager<> is public, which made me think of this.

Something like this:

https://github.com/KevinDockx/HttpCacheHeaders/blob/7c7f0fe94b70e4273f0a8011c8daf78adb220c13/test/Marvin.Cache.Headers.Test/Extensions/ServiceExtensionFacts.cs#L75-L81

        private static void ValidateServiceOptions<T>(TestServer testServer, Func<OptionsManager<T>, bool> validOptions) where T : class, new()
        {
            var options = testServer.Host.Services.GetService(typeof(IOptions<T>));
            Assert.NotNull(options);
            var manager = (OptionsManager<T>)options;
            Assert.True(validOptions(manager));
        }

I know this is test code, but I can imagine other doing this unnatural thing.

This is an acceptable breaking change IMO. It's unfortunate that OptionsManager is public but I don't think it's reasonable to tie our hands as a result. This is one of the benefits of using DI (swapping the implementation).

Good find though!

I did a query and found some packages using OptionsManager directly but none were casting the default implementation.