A delegate object is created for each IOptionsMonitor<T>.Get call

[rayao]

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

Line 89 in df5b118

return _cache.GetOrAdd(name, () => _factory.Create(name));

It could be avoided if the delegate is a member, or simply Add a TryGetValue call before calling GetOrAdd.
The object allocation is much more expensive than the getting itself, when CurrentValue/Get is used in hot code path, the cost will be unexpected.

[ghost]

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

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

Line 89 in df5b118

return _cache.GetOrAdd(name, () => _factory.Create(name));

It could be avoided if the delegate is a member, or simply Add a TryGetValue call before calling GetOrAdd.
The object allocation is much more expensive than the getting itself, when CurrentValue/Get is used in hot code path, the cost will be unexpected.

Author:	rayao
Assignees:	-
Labels:	untriaged, area-Extensions-Options
Milestone:	-

[davidfowl]

We can fix this by not using the IOptionsCache. It's possible we can use the concrete type if it hasn't been overridden as an optimization.

[rayao]

@davidfowl I didn't get why we need the IOptionsMonitorCache service, if OptionsMonitor maintains the cache itself, everything will be much easier. I didn't find IOptionsMonitorCache is used anywhere else, could we just remove it or add some more methods?

[NickCraver]

For what it's worth, I came across this as a hot path allocation today in an application using .CurrentValue quite a bit on every request. Can we switch to the TArg overload here?

[stephentoub]

Can we switch to the TArg overload here?

There isn't one:

runtime/src/libraries/Microsoft.Extensions.Options/src/IOptionsMonitorCache.cs

Lines 13 to 43 in 8b1a0c8

	public interface IOptionsMonitorCache<[DynamicallyAccessedMembers(Options.DynamicallyAccessedMembers)] TOptions>
	where TOptions : class
	{
	/// <summary>
	/// Gets a named options instance, or adds a new instance created with <paramref name="createOptions"/>.
	/// </summary>
	/// <param name="name">The name of the options instance.</param>
	/// <param name="createOptions">The func used to create the new instance.</param>
	/// <returns>The options instance.</returns>
	TOptions GetOrAdd(string name, Func<TOptions> createOptions);
	/// <summary>
	/// Tries to adds a new option to the cache, will return false if the name already exists.
	/// </summary>
	/// <param name="name">The name of the options instance.</param>
	/// <param name="options">The options instance.</param>
	/// <returns>Whether anything was added.</returns>
	bool TryAdd(string name, TOptions options);
	/// <summary>
	/// Try to remove an options instance.
	/// </summary>
	/// <param name="name">The name of the options instance.</param>
	/// <returns>Whether anything was removed.</returns>
	bool TryRemove(string name);
	/// <summary>
	/// Clears all options instances from the cache.
	/// </summary>
	void Clear();
	}

[NinoFloris]

Isn't this a clear case where a new interface method with a default implementation would be perfect? It seems really simple to add a TArg version that would just allocate and call the existing GetOrAdd in the default implementation.