Improve GetChildren / GetChildKeys performance for large configurations - #130306
Improve GetChildren / GetChildKeys performance for large configurations#130306rosebyte wants to merge 3 commits into
GetChildren / GetChildKeys performance for large configurations#130306Conversation
GetChildren / GetChildKeys performance for large configurations
There was a problem hiding this comment.
Pull request overview
This PR refactors how configuration child keys are aggregated across providers, aiming to reduce allocations and improve correctness/consistency by de-duplicating keys during aggregation (rather than at the end), while keeping ordering via ConfigurationKeyComparer. It also adds targeted tests around de-duplication, large key sets, deep keys, and provider key-filtering behavior.
Changes:
- Introduces
SortedChildKeysas an internal accumulator to de-duplicate and lazily sort child keys. - Updates
GetChildrenImplementation(and chained-provider child key retrieval) to use the new accumulator-based aggregation. - Adds tests validating de-duplication and various edge cases for
GetChildren()/GetChildKeys()behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs | Adds test coverage for de-duplication and edge cases around child key aggregation. |
| src/libraries/Microsoft.Extensions.Configuration/src/SortedChildKeys.cs | New internal accumulator type for de-duplicating + lazily sorting child keys. |
| src/libraries/Microsoft.Extensions.Configuration/src/InternalConfigurationRootExtensions.cs | Reworks provider aggregation and section projection to use SortedChildKeys. |
| src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationProvider.cs | Updates base provider to add keys directly into SortedChildKeys when used as the seed. |
| src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs | Avoids unnecessary parsing work / allocations by gating TryParse attempts. |
| src/libraries/Microsoft.Extensions.Configuration/src/ChainedConfigurationProvider.cs | Uses the accumulator-based path to avoid per-child section allocations when chaining. |
| private static void ProcessProvider(IConfigurationProvider provider, SortedChildKeys accumulator, string? path) | ||
| { | ||
| IEnumerable<string> returned = provider.GetChildKeys(accumulator, path); | ||
| if (!ReferenceEquals(returned, accumulator)) | ||
| { | ||
| accumulator.Overwrite(returned); | ||
| } | ||
| } |
| public ChildKeysBag(SortedChildKeys accumulator) | ||
| { | ||
| _accumulator = accumulator; | ||
| } | ||
|
|
||
| public ChildKeysBag(List<string> fallback) | ||
| { | ||
| _fallback = fallback; | ||
| } |
| SortedChildKeys accumulator = earlierKeys is SortedChildKeys existing ? existing : new(earlierKeys); | ||
| if (_config is IConfigurationRoot root) | ||
| { | ||
| return root.GetChildKeysImplementation(parentPath, accumulator); | ||
| } |
| int count = list.Count; | ||
| if (count == 0) | ||
| { | ||
| return null; |
There was a problem hiding this comment.
When a chained IConfigurationRoot has zero providers this returns null, and the caller turns that into Array.Empty, discarding the seed accumulator that already holds the earlier providers' keys. The else branch below keeps the seed, so the two paths disagree.
Repro:
var empty = new ConfigurationBuilder().Build();
var config = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { { "A", "1" }, { "B", "2" } })
.AddConfiguration(empty)
.Build();
config.GetChildren(); // [] on this PR; main returns [A, B]Values via the indexer are fine; only child enumeration is affected. return seed; here (matching the else branch) keeps the accumulated keys.
| SortedChildKeys accumulator = earlierKeys is SortedChildKeys existing ? existing : new(earlierKeys); | ||
| if (_config is IConfigurationRoot root) | ||
| { | ||
| return root.GetChildKeysImplementation(parentPath, accumulator); |
There was a problem hiding this comment.
This threads the outer accumulator into the chained root's own providers as their earlierKeys. A provider inside the chained root that overrides GetChildKeys to filter earlierKeys will now drop keys contributed by the outer providers, which it never observed before.
Example: outer {a, b, c}, then AddConfiguration(innerRoot) where innerRoot has a provider that removes b from earlierKeys. main returns [a, b, c]; this PR returns [a, c]. It changes the chaining boundary semantics, so worth a deliberate decision or at least a note.
| internal void AddSegment(string key, int start, int length) | ||
| { | ||
| #if NET | ||
| if (!_lookup.Contains(key.AsSpan(start, length))) |
There was a problem hiding this comment.
First-added wins here, so for a case-insensitive duplicate child across providers the returned key now takes the first provider's casing instead of the last. Value resolution is unchanged.
Example: provider1 has Section:Abc, provider2 has Section:ABC. main returns child key ABC; this PR returns Abc. Minor, but observable to code that reads child.Key.
There was a problem hiding this comment.
I'm aware of this, but I don't see any harm it could cause, as the purpose of keys is to be used for value lookups, and all value lookups are case-insensitive, which should make the practical ramifications non-existent. Moreover, I don't think there is any contract that promises this behaviour. We only do it this way because it happens to be the default behaviour of ConfigurationProvider.
We could certainly either use a dictionary or implement a hash set that allows keys to be updated, but that would come at a cost. The quick benchmarks I ran showed at least a 17 % increase in both memory usage and execution time for duplicate heavy use-cases and 10 % in general, which I struggle to justify. Do you see a compelling reason to preserve the most recent casing?
9ed72c2 to
0a758f0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/libraries/Microsoft.Extensions.Configuration/src/ChildKeySorter.cs:22
- The XML doc comment has a malformed line combining two doc tags (
</para>and<para>) on the same line, which will render incorrectly in generated docs.
/// </para> /// <para>
src/libraries/Microsoft.Extensions.Configuration/src/SortedChildKeys.cs:14
- The SortedChildKeys summary says ordering is established lazily on first enumeration after a change, but the implementation also sorts when
Finalis set. Updating the doc comment would keep it aligned with the behavior.
/// sorted with <see cref="ConfigurationKeyComparer"/> lazily, at most once, the first time the accumulator is
src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs:1078
- This test comment claims the accumulator is unsorted until CopyTo runs, but GetChildren now establishes ordering when it sets
SortedChildKeys.Final = true, even if the returned section sequence is never enumerated.
// Run the fold without enumerating its result. GetChildren aggregates eagerly but projects lazily, so
// nothing has sorted the accumulator yet and CopyTo is the only thing that can.
0a758f0 to
6deb717
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs:1477
Capturedis a non-nullable auto-property but is never initialized in the constructor, which can trigger CS8618 (often treated as an error in the repo). Initialize it to an empty sequence (or make it nullable).
public IEnumerable<string> Captured { get; private set; }
src/libraries/Microsoft.Extensions.Configuration/src/Resources/Strings.resx:127
- The Error_ReadOnlyChildKeys resource message is used when mutating the
ICollection<IConfigurationSection>returned byGetChildren(), but the text mentions “child keys returned by a configuration provider”, which is misleading for callers.
<value>The child keys returned by a configuration provider are read-only.</value>
src/libraries/Microsoft.Extensions.Configuration/src/ChildKeySorter.cs:20
- The PR description says
ConfigurationKeyComparerno longer compares integer segments by subtraction, butConfigurationKeyComparer.Comparein the current code still doesvalue1 - value2(overflow/non-transitive). Either updateConfigurationKeyComparerto useCompareToor adjust the PR description to clarify that only the newChildKeySorteravoids the overflow.
/// The two orders agree except on a pair of numeric keys far enough apart that the general comparer's
/// <c>value1 - value2</c> overflows and reverses them. This sorter compares such a pair with <c>CompareTo</c>,
/// so it does not reproduce that.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/libraries/Microsoft.Extensions.Configuration/src/ChildKeysAggregator.cs:136
- ChildKeysAggregator.Overwrite clears only up to
end, so if the accumulator has been overwritten multiple times, stale references can remain in_itemsbeyondend. BecauseGetChildrenImplementationpasseskeys.Itemsdirectly intoChildSections, those stale references can keep many configuration key strings alive longer than needed (even thoughCountis smaller). Clearing the remainder of the array after rebuilding avoids this retention.
for (int i = start; i < end; i++)
{
Add(_items[i]);
}
Array.Clear(_items, _count, end - _count);
src/libraries/Microsoft.Extensions.Configuration/src/ChildKeySorter.cs:21
- PR description says
ConfigurationKeyComparerno longer compares integer segments by subtraction, but this change set leavesConfigurationKeyComparer.Compareusingvalue1 - value2(overflow-prone). The overflow fix appears to be implemented here inChildKeySorterinstead. Consider updating the PR description (or, if intended, separately fixingConfigurationKeyComparer).
/// The two orders agree except on a pair of numeric keys far enough apart that the general comparer's
/// <c>value1 - value2</c> overflows and reverses them. This sorter compares such a pair with <c>CompareTo</c>,
/// so it does not reproduce that.
Fixes #65885.
O(P²)in the provider count.GetChildKeysthreads the accumulated keys through every provider, and eachConfigurationProvidercopied them into a fresh list and re-sorted the lot. WithPproviders the same keys were sortedPtimes over.GetChildren(path)call rescans every key of every provider. Binding walks each section in turn, so a configuration ofNkeys costsO(N²·P)to bind.This PR removes the first entirely and cuts the constant on the second by roughly 2.5x.
Measurements
BenchmarkDotNet, in-process toolchain, .NET 10, Apple arm64. Both sides built from source and the loaded assembly checksum-verified per run. Absolute numbers are machine-specific, so the ratios are the point. A few cells at 20 000 keys carry error bars of ±8-27% and are marked; everything else is within ±5%.
Three key shapes are used throughout:
Root:Items:{i}:{Name,Value,Enabled,Meta:Kind}, the issue's shape.Service{s}:Endpoints:Group{g}:{leaf}, wide and shallow.Contoso:Platform:Features:Feature{f}:Providers:Provider{p}:Options:{leaf}, the options pattern.Provider-count scaling
One
GetChildrenover 100 distinct keys carried byPfully overlapping providers.main grows about 4x per doubling, which is
O(P²). This PR grows linearly, and its allocation is flat at 13.7 KB regardless of provider count.Binding a whole configuration
Recursive walk of every section, which is what the binder does.
A single wide section
Reading
section.Keyas well: 6.5x to 17.3x. Asking only.Any(): 8.7x to 23.5x, with allocation down up to 22x.Chained configuration
AddConfiguration, which the generic host uses to fold host configuration into app configuration. main re-aggregated and re-sorted at every nesting level.How
O(P²)→O(P)change.KeyValuePair, one character of content rejects most candidates before the ignore-case comparison, and consecutive keys belonging to the same child are skipped without hashing. Providers write keys in document order, soRoot:0:Name,Root:0:Value,Root:0:Enabledall yield the segment0back to back.Behaviour change
The aggregate is now always sorted. Previously the order of
GetChildren()was whatever the last provider returned, becauseConfigurationRootdid not sort. A provider that returned keys in its own order therefore decided the order for the whole configuration, including keys it did not own.That was reachable and unpleasant: a provider contributing no data at all, merely returning
earlierKeysreordered, changed which element a bound array put where.Root:0..Root:3could bind to[three, two, one, zero].Verified against the shipped 10.0.0 packages before changing anything, and reproduced identically on main. Sorting at the root converts an unenforced convention into a guarantee. A provider that deliberately returned a bespoke order will no longer have it honoured.
ConfigurationKeyCompareralso no longer compares integer segments by subtraction, which overflowed at opposite extremes and made the comparison non-transitive.What this does not change
A single
GetChildrenstill scans the whole of each provider'sData, so it remainsO(N·P)per call. That is the floor without derived state, because a provider may overrideGetChildKeysand never useDataat all.Making it sub-linear needs a per-provider key index. That was prototyped, along with replacing
Datawith a trie, and both were rejected: they are parallel structures with invalidation and retained memory, and the trie needs a CoreLib-internal hash to keep lookup at parity. Neither was necessary to resolve the reported cost.Compatibility
GetChildKeysare unaffected; a foreign result is absorbed by the accumulator.