A large set of configuration keys provided by the built-in .NET IConfigurationProvider's negatively affect performance significantly

[LeoHexspoor]

Description

A large set of configuration keys provided by the built-in .NET IConfigurationProvider's negatively affect performance significantly.

I've created a benchmark project here: https://github.com/NetMatch/dotnet-config-performance-issue

Configuration

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.19044.1526 (21H2)
AMD Ryzen 9 5900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK=6.0.102
  [Host]     : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT
  DefaultJob : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT

Regression?

We encountered this issue in .NET Core 3.1, the benchmark project shows it still reproduces in .NET 6.

Data

|                     Method |        ConfigFile | UseCache |      Mean |     Error |    StdDev |
|--------------------------- |------------------ |--------- |----------:|----------:|----------:|
| GetObjectFromConfiguration | /largeconfig.json |    False | 89.062 us | 0.8921 us | 0.8345 us |
| GetObjectFromConfiguration | /largeconfig.json |     True |  1.945 us | 0.0093 us | 0.0087 us |
| GetObjectFromConfiguration | /smallconfig.json |    False |  1.910 us | 0.0077 us | 0.0072 us |
| GetObjectFromConfiguration | /smallconfig.json |     True |  2.015 us | 0.0075 us | 0.0070 us |

Analysis

We have a large web application in which we have a significant amount of configuration, this adds up to about 20k configuration keys. We have a non-trivial amount of IOptionsSnapshot instances that are resolved per request via DI. We noticed that as configuration got added, performance and CPU usage went up across the board. Digging deeper we found that this is due to how the Microsoft.Extensions.Configuration.ConfigurationProvider and Microsoft.Extensions.Configuration.ConfigurationBinder work.

Every request the IOptionsSnapshots are constructed via the ConfigurationBinder. The ConfigurationBinder is passed an IConfiguration which is actually a IConfigurationSection instance that is scoped to a certain path in the configuration. Based on the public API, using GetSection, one expects that the IConfigurationSection instance is optimized to access the child keys of a certain section. But this is not the case. Whenever the ConfigurationBinder accesses the configuration and calls GetChildren() it ends up going to the ConfigurationRoot and calls into each registered IConfigurationProvider to fetch child keys for the path of the ConfigurationSection.

All of the default IConfigurationProvider implementations inherit from ConfigurationProvider which has a GetChildKeys method where things get troublesome. This methods does a loop over ALL registered keys in the provider to find a prefix match for the given path. This means that for every call of GetChildren() on an IConfiguration instance, it scans every possible key in all the providers. On top of that various code paths in the binder lead it to call GetChildren() multiple times on the same IConfiguration. All of this combined adds up to a lot of scans.

When you have a lot of configuration and a non-trivial amount of Options that get bound per request you end up with a serious amount of scanning over keys for no reason. And that gave us big performance issues. After identifying this issue, we added a new IConfigurationSource and IConfigurationProvider that caches calls into GetChildKeys before going to the underlying IConfigurationProvider. The implementation in the linked repo is a simplified combined version of this which should not be used in production. However for the benchmark it shows its effects well enough, it's used when the UseCache option is set to true. With this deployed in our application, our 95 percentile response times were 4 times lower and we are using 10 times(!) less CPU during load, at the cost of slightly higher memory usage.

IMO the problem is twofold. The ConfigurationBinder is suboptimal by calling into the GetChildren() method so often. But most important is the fact that the IConfiguration(Section) implementation seems flawed. ConfigurationSection's are not really optimized to access the keys in their section, it always goes to the root and scans all keys without any caching or optimizations.

[ghost]

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

Issue Details

---

Description

A large set of configuration keys provided by the built-in .NET IConfigurationProvider's negatively affect performance significantly.

I've created a benchmark project here: https://github.com/NetMatch/dotnet-config-performance-issue

Configuration

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.19044.1526 (21H2)
AMD Ryzen 9 5900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK=6.0.102
  [Host]     : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT
  DefaultJob : .NET 6.0.2 (6.0.222.6406), X64 RyuJIT

Regression?

We encountered this issue in .NET Core 3.1, the benchmark project shows it still reproduces in .NET 6.

Data

|                     Method |        ConfigFile | UseCache |      Mean |     Error |    StdDev |
|--------------------------- |------------------ |--------- |----------:|----------:|----------:|
| GetObjectFromConfiguration | /largeconfig.json |    False | 89.062 us | 0.8921 us | 0.8345 us |
| GetObjectFromConfiguration | /largeconfig.json |     True |  1.945 us | 0.0093 us | 0.0087 us |
| GetObjectFromConfiguration | /smallconfig.json |    False |  1.910 us | 0.0077 us | 0.0072 us |
| GetObjectFromConfiguration | /smallconfig.json |     True |  2.015 us | 0.0075 us | 0.0070 us |

Analysis

We have a large web application in which we have a significant amount of configuration, this adds up to about 20k configuration keys. We have a non-trivial amount of IOptionsSnapshot instances that are resolved per request via DI. We noticed that as configuration got added, performance and CPU usage went up across the board. Digging deeper we found that this is due to how the Microsoft.Extensions.Configuration.ConfigurationProvider and Microsoft.Extensions.Configuration.ConfigurationBinder work.

Every request the IOptionsSnapshots are constructed via the ConfigurationBinder. The ConfigurationBinder is passed an IConfiguration which is actually a IConfigurationSection instance that is scoped to a certain path in the configuration. Based on the public API, using GetSection, one expects that the IConfigurationSection instance is optimized to access the child keys of a certain section. But this is not the case. Whenever the ConfigurationBinder accesses the configuration and calls GetChildren() it ends up going to the ConfigurationRoot and calls into each registered IConfigurationProvider to fetch child keys for the path of the ConfigurationSection.

All of the default IConfigurationProvider implementations inherit from ConfigurationProvider which has a GetChildKeys method where things get troublesome. This methods does a loop over ALL registered keys in the provider to find a prefix match for the given path. This means that for every call of GetChildren() on an IConfiguration instance, it scans every possible key in all the providers. On top of that various code paths in the binder lead it to call GetChildren() multiple times on the same IConfiguration. All of this combined adds up to a lot of scans.

When you have a lot of configuration and a non-trivial amount of Options that get bound per request you end up with a serious amount of scanning over keys for no reason. And that gave us big performance issues. After identifying this issue, we added a new IConfigurationSource and IConfigurationProvider that caches calls into GetChildKeys before going to the underlying IConfigurationProvider. The implementation in the linked repo is a simplified combined version of this which should not be used in production. However for the benchmark it shows its effects well enough, it's used when the UseCache option is set to true. With this deployed in our application, our 95 percentile response times were 4 times lower and we are using 10 times(!) less CPU during load, at the cost of slightly higher memory usage.

IMO the problem is twofold. The ConfigurationBinder is suboptimal by calling into the GetChildren() method so often. But most important is the fact that the IConfiguration(Section) implementation seems flawed. ConfigurationSection's are not really optimized to access the keys in their section, it always goes to the root and scans all keys without any caching or optimizations.

Author:	LeoHexspoor
Assignees:	-
Labels:	tenet-performance, untriaged, area-Extensions-Configuration
Milestone:	-

[rjgotten]

The design of passing earlierKeys into each GetChildKeys call is kind of bad as well: nothing more than a leaky abstraction of the LINQ Aggregate that's internally being used to pull up the full set of child keys across all providers. The earlierKeys being passed in places burden on each individual configuration provider to combine child-key results of a previous provider with the current's.

And because the final combined result is internally not sorted by InternalConfigurationRootExtensions.GetChildrenImplementation, it requires each individual provider to combine its results with the previous set, and re-sort the running total list as it is being grown out. N providers means N times resorting that list.

That design could be improved by moving the sort operation into GetChildrenImplementation and removing that burden from the individual providers. Same for the responsibility of having to combine earlierKeys and create additional intermediate List<> instances.

It could just be a basic IEnumerable<>.Concat which chains together enumerators for the lists returned by the various providers; runs Distinct over them; shoves it into one big List<> with ToList(); and then sorts that in-place -- once.

Wonder if it shouldn't actually use an array instead of a list though. Given that classes consuming the result are not expected to actually go and add/remove elements here? And that an array iterator is cheaper than a list iterator?

[maryamariyan]

Related to #62510

[wangyuantao]

Add a similar benchmark at dotnet/performance#2572 and hope the performance can be improved in net7.0

[davidfowl]

.NET 7 is done.

[wangyuantao]

@davidfowl you are right. If not planned, I would like to contribute to improve it in net8.0.

[davidfowl]

@wangyuantao I think the first change would be to get this into our performance tests.

cc @eerhardt

[kwaazaar]

So that PR #74736 was never completed?

Our application is already performance bad because of this. Now we want to make it multi-tenant, makeing the problem even worse :-(

[rjgotten]

@kwaazaar
Have a look at the benchmark application mentioned in the initial issue report.
It contains a ConfigurationSourceCacheDecorator that can be used to mitigate the problem somewhat.

[julealgon]

Reading this issue makes me super skeptical about introducing more IOptionsSnapshot usages in our solution...

We also have a fairly large monolith with centralized configuration coming from a custom db source and Azure AppConfiguration in addition to the other standard sources, and we have been ramping up usages of IOptionsSnapshot as we migrate from legacy code that was not using the Options pattern.

Seems like this missed .NET9 too... oh well 😞

I might have to start recommending developers to not use IOptionsSnapshot while these problems are not resolved and live with the need for a full application deployment to refresh some of the settings. It would degrade our dev experience quite a bit though.

[kwaazaar]

You could still use IOptionSnapshot in all code and use your own implementation for DI. That way your code still adheres to a standard approach and the impact is minimal in the future to swap back to the improved implementation in .NET 10 (or later :-()

[rjgotten]

@kwaazaar
Have a look at the benchmark application mentioned in the initial issue report. It contains a ConfigurationSourceCacheDecorator that can be used to mitigate the problem somewhat.

Oh I know it helps mitigating the issue. I helped write it to do just that.
(@LeoHexspoor and I are colleagues. 😉 )

And indeed: _mitigates - It's not an end solution.
I'm hoping still that at some point Microsoft.Extensions.Configuration itself will be fixed, regardless of what it will mean for backwards compatibility, which is likely to be on some minor edge cases only - not the major usage surface of the API.

[Falco20019]

I just spent 2 hours finding out why importing a big JSON file takes 14 minutes... We need multiple calls to config.GetChildren().Any() during binding and since GetChildren() is ALWAYS creating the full list, even though Any() is only interested in finding out if it's a leaf or not, this sums up pretty hugely... So looking forward to this getting fixed.

We sadly need to keep using it, since it's not distinguishable if a section is not existing or if it was only empty. Only GetChildren will list it with null as value. When using GetSection(...).Value, both cases are null :(

[Falco20019]

I was able to improve my loading from 14 minutes to 675 milliseconds. The main bottleneck is this:

runtime/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationProvider.cs

Lines 61 to 93 in dda0d4e

	public virtual IEnumerable<string> GetChildKeys(
	IEnumerable<string> earlierKeys,
	string? parentPath)
	{
	var results = new List<string>();
	if (parentPath is null)
	{
	foreach (KeyValuePair<string, string?> kv in Data)
	{
	results.Add(Segment(kv.Key, 0));
	}
	}
	else
	{
	Debug.Assert(ConfigurationPath.KeyDelimiter == ":");
	foreach (KeyValuePair<string, string?> kv in Data)
	{
	if (kv.Key.Length > parentPath.Length &&
	kv.Key.StartsWith(parentPath, StringComparison.OrdinalIgnoreCase) &&
	kv.Key[parentPath.Length] == ':')
	{
	results.Add(Segment(kv.Key, parentPath.Length + 1));
	}
	}
	}
	results.AddRange(earlierKeys);
	results.Sort(ConfigurationKeyComparer.Comparison);
	return results;

runtime/src/libraries/Microsoft.Extensions.Configuration/src/InternalConfigurationRootExtensions.cs

Lines 21 to 41 in dda0d4e

	internal static IEnumerable<IConfigurationSection> GetChildrenImplementation(this IConfigurationRoot root, string? path)
	{
	using ReferenceCountedProviders? reference = (root as ConfigurationManager)?.GetProvidersReference();
	IEnumerable<IConfigurationProvider> providers = reference?.Providers ?? root.Providers;
	IEnumerable<IConfigurationSection> children = providers
	.Aggregate(Enumerable.Empty<string>(),
	(seed, source) => source.GetChildKeys(seed, path))
	.Distinct(StringComparer.OrdinalIgnoreCase)
	.Select(key => root.GetSection(path == null ? key : path + ConfigurationPath.KeyDelimiter + key));
	if (reference is null)
	{
	return children;
	}
	else
	{
	// Eagerly evaluate the IEnumerable before releasing the reference so we don't allow iteration over disposed providers.
	return children.ToList();
	}
	}

It is basically adding one string for each entry that contains the substring. For our big config file, this were 53k of stations (top level of an array with 1.7k stations having each a couple data points to it). Those are getting reduced to one stations entry by Distinct(StringComparer.OrdinalIgnoreCase).

For testing purposes, I changed the IDictionary<string, string?> Data to a simple data tree. I used ConfigurationPath.KeyDelimiter to split the levels as there was a hard assumption in the existing code for it anyway. I also replaced it with a real IEnumerable implementation that uses yield return and drops sorting at each GetChildKeys since the xmldocs never stated that it must be sorted in the first place. So it is a breaking change, but fixed my problems. I did the whole thing just through extension methods and added a cache to avoid re-processing the Data for a provider unless reloaded.

In case someone also has problems and is interested in the workaround, let me know and I will extract it into a MCVE. The better solution would be to fix the original implementation and maybe take the hit for the sorting to not take place. An feasable alternative would be to just add new methods that return the GetChildKeys sorted as before but also offer a property GetChildKeysUnsorted method that is used by the old one for compatability but offers proper usage to anyone who wants/needs it fast.

Sadly any change to Data in ConfigurationProvider is a breaking change. JsonConfigurationProvider could overwrite everything accordingly, but since even that is technically not sealed, anyone could rely on the current implementation as-well.