U/petrhouska/improve-default-case-of-options-cache

[petrroll]

Fixes Issue #109446

Description

Special cases most common (IMHO by super vast majority) case of IOtionsMonitor for default options name, i.e. "". Most importantly changes .CurrentValue access from concurentdictionary read to a field access.

Customer Impact

Small perf.

Regression

No.

Testing

I'll add tests if it's deamed feasible and worthy of being merged.

Risk

Substentially increases complexity of originally super simple class. The complecity is still very limited and IMHO perfectly managable but it went from super duper simple to somewhat simple.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

[petrroll]

Method	Mean	Error	StdDev	Allocated
Baseline	10.0452 ns	0.2823 ns	0.4944 ns	-
Better	0.6969 ns	0.0287 ns	0.0254 ns	-

    [Benchmark]
    public string Baseline()
    {
        var opt = optionsCache.GetOrAdd(null, () => new MyOptions { Name = "Default", Value = 42 });
        return opt.Name;
    }

    [Benchmark]
    public string Better()
    {
        var opt = optionsCacheBetter.GetOrAdd(null, () => new MyOptions { Name = "Default", Value = 42 });
        return opt.Name;
    }

https://pastebin.com/Vf5XRC8K

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

[petrroll]

Could it be reopened @huoyaoyuan ? I'd still be fully up for merging it :)

[tarekgh]

The bar for .NET 10 is now quite high—similar to the servicing bar. Let’s wait until the main branch opens for the next release, and then we can reopen and merge. This should happen in a few weeks from now. Thanks!

[tarekgh]

I’ve skimmed through your PR, and it seems to add some complexity without offering much benefit. The performance provided measurement is at the micro level, which I don’t believe will have a significant impact in the full stack — especially since heavier operations are handled through options. A few nanoseconds won’t be noticeable in that context.
The added complexity comes from introducing new caching fields that must be guaranteed thread-safe. In the current implementation, that’s not fully the case and would require careful handling. For example, the new cache field should be marked volatile; otherwise, it could end up in a torn state even if updated with Interlocked. Also, when using this field, it’s important to first store it in a local variable, since other code clears and sets it to null.

I am closing this PR but feel free to reply if you have any more feedback. Thanks!