Reuse reflection argument arrays in ConfigurationBinder BindDictionary/BindCollection#130626
Reuse reflection argument arrays in ConfigurationBinder BindDictionary/BindCollection#130626artl93 wants to merge 2 commits into
Conversation
The reflection-based ConfigurationBinder allocated a fresh single-element object[] for every successfully bound dictionary child (for PropertyInfo.SetValue) and every collection item (for MethodInfo.Invoke). Following the existing BindSet pattern, allocate the argument array once before the loop and overwrite slot 0 per child. The reused private array is never observable because reflection invocation is synchronous and by-value. Behavior is preserved: configuration-child order, dictionary overwrite semantics, null item values, custom/interface collection dispatch, and ErrorOnUnknownConfiguration wrapping/exception timing are unchanged. In BindCollection the one-slot array is allocated only when an Add method exists, keeping the null-addMethod path a no-op as before. Adds targeted reflection-mode tests verifying Add/indexer call order, per-item values, dictionary overwrite, and exception behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 133a456b-9d7d-4f2c-b1aa-17639ba65277
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
There was a problem hiding this comment.
Pull request overview
This PR updates the reflection-based ConfigurationBinder to reuse a single object[] argument array when invoking dictionary indexers and collection Add via reflection, reducing per-element allocations during binding. It also adds targeted tests to validate binding order/value semantics and error behavior for custom collection/dictionary implementations.
Changes:
- Reuse a single
object?[1]array forPropertyInfo.SetValue(..., indexArgs)inBindDictionary. - Reuse a single
object?[1]array forMethodInfo.Invoke(..., args)inBindCollection. - Add new test helper types and tests that validate per-item call order/values and
ErrorOnUnknownConfigurationbehavior in non-source-gen mode.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs | Hoists/reuses reflection invocation argument arrays in BindDictionary/BindCollection. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.Collections.cs | Adds custom recording collection/dictionary types used by new tests. |
| src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Collections.cs | Adds non-source-gen tests validating call order/values and strict vs default error handling. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
| MethodInfo tryGetValue = dictionaryType.GetMethod("TryGetValue", DeclaredOnlyLookup)!; | ||
| PropertyInfo indexerProperty = dictionaryType.GetProperty("Item", DeclaredOnlyLookup)!; | ||
|
|
||
| object?[] indexerArguments = new object?[1]; |
There was a problem hiding this comment.
Correct that indexerArguments is allocated before the loop. This is measured and disclosed as the change's one negative case: +32 B/op, once, and only when the route is entered but zero children bind successfully (all values fail conversion and are swallowed under the default ErrorOnUnknownConfiguration=false, or a present-but-empty section). An absent section stays +0 because the route is never entered. At 100 all-invalid children it is ~0.02% of the ~166 KB the swallowed-conversion path already allocates.
The eager allocation is deliberate: it mirrors the sibling BindSet (line 928), which has allocated its object?[1] before its loop for years — keeping the three collection-binding methods uniform.
Your lazy suggestion (object?[]? indexerArguments = null; → indexerArguments ??= new object?[1]; inside the HasNewValue block) would eliminate even that +32 B while preserving the full win (still exactly one allocation per bind that sets ≥1 item). The tradeoff is eager-consistency-with-BindSet vs zero-regression. Flagging it as an open decision for review rather than silently picking one.
Note
This reply was generated with the assistance of GitHub Copilot.
There was a problem hiding this comment.
Adopted the suggested lazy allocation. Both paths now use ??= new object?[1] on the first successful bind instead of allocating the array eagerly, so a route that is entered but binds zero items (all values swallowed as invalid, or an empty/absent section) allocates nothing.
Fresh exact-parent Release A/B (candidate 3888f0b vs parent 7aa830a) confirms the zero-success case is now +0 B/op (List all-invalid @100: 165,986 → 165,986; Dict: 190,198 → 190,198; missing section: +0), while the per-item win is preserved (List/Dict −288/−3.2K/−32K at N=10/100/1000). There is no longer any measured regression in any scenario.
Note
This reply was generated with the assistance of GitHub Copilot.
| MethodInfo? addMethod = collectionType.GetMethod("Add", DeclaredOnlyLookup); | ||
| object?[]? addArguments = addMethod is not null ? new object?[1] : null; | ||
|
|
||
| foreach (IConfigurationSection section in config.GetChildren()) | ||
| { |
There was a problem hiding this comment.
Same tradeoff as the BindDictionary thread, and already guarded here: addArguments is only allocated when an Add method exists (the no-Add interface path stays allocation-neutral). The residual cost is +32 B/op once only when the route is entered and zero items bind successfully (all-invalid/swallowed or present-but-empty); absent sections are +0.
The eager pattern matches the sibling BindSet (line 928) for consistency. The lazy variant — addArguments ??= new object?[1]; inside the if (itemBindingPoint.HasNewValue && addMethod is not null) block — would remove even the +32 B while keeping one allocation per non-empty bind. Deferred as an open decision (eager consistency vs zero-regression), noted on the dictionary thread too.
Note
This reply was generated with the assistance of GitHub Copilot.
There was a problem hiding this comment.
Adopted the suggested lazy allocation. Both paths now use ??= new object?[1] on the first successful bind instead of allocating the array eagerly, so a route that is entered but binds zero items (all values swallowed as invalid, or an empty/absent section) allocates nothing.
Fresh exact-parent Release A/B (candidate 3888f0b vs parent 7aa830a) confirms the zero-success case is now +0 B/op (List all-invalid @100: 165,986 → 165,986; Dict: 190,198 → 190,198; missing section: +0), while the per-item win is preserved (List/Dict −288/−3.2K/−32K at N=10/100/1000). There is no longer any measured regression in any scenario.
Note
This reply was generated with the assistance of GitHub Copilot.
|
SUPERSEDED — do not action this request. The trigger below used an invalid arg (
Original benchmark (retained for reference)using System.Collections.Generic;
using BenchmarkDotNet.Attributes;
using Microsoft.Extensions.Configuration;
public class ConfigBinderBench
{
private IConfigurationSection _list = null!, _dict = null!, _listAllInvalid = null!;
[GlobalSetup]
public void Setup()
{
var list = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) list[$"Root:{i}"] = i.ToString();
_list = new ConfigurationBuilder().AddInMemoryCollection(list).Build().GetSection("Root");
var dict = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) dict[$"Root:k{i}"] = i.ToString();
_dict = new ConfigurationBuilder().AddInMemoryCollection(dict).Build().GetSection("Root");
var bad = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) bad[$"Root:{i}"] = "not_an_int";
_listAllInvalid = new ConfigurationBuilder().AddInMemoryCollection(bad).Build().GetSection("Root");
}
[Benchmark] public List<int>? BindList() => _list.Get<List<int>>();
[Benchmark] public Dictionary<string, int>? BindDict() => _dict.Get<Dictionary<string, int>>();
[Benchmark] public List<int>? BindList_AllInvalid() => _listAllInvalid.Get<List<int>>();
}Note This comment was generated with the assistance of GitHub Copilot. |
…ection Allocate the reused object?[1] argument array on the first successful bind instead of eagerly before the child loop. Routes that enter but bind zero items (all-invalid values swallowed, or a present-but-empty section) now allocate nothing, eliminating the +32 B/op zero-successful-binds regression while preserving the per-item argument-array reuse win. BindCollection now gates the lazy allocation on addMethod directly rather than on a pre-built array, keeping the no-Add interface path allocation-neutral. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 133a456b-9d7d-4f2c-b1aa-17639ba65277
Consolidated evidence (canonical: BenchmarkDotNet exact-parent A/B)Harness: BenchmarkDotNet 0.14.0, Runtime-verified binder substitution (loaded-assembly SHA-256)The harness runs benchmarks in-process (
The two arms load different binders while every shared dependency is byte-identical — so the measured delta is attributable to the binder change alone. (An earlier run silently loaded a released 2018 Scaling — Allocated bytes/op, Δ = candidate − baseline
Model Δ ≈ −32 × (N − 1) bytes/op (32 B = one Fixed shapes @n=100 & scenarios — Allocated bytes/op
Every changed route improves by ≈ −32 × (N − 1); every control and every zero-success negative is byte-identical (no regression, and lazy-init confirmed allocation-neutral when a route binds zero items). Corroboration (secondary)A custom in-process Tests
Lineage
Note This comment was generated with the assistance of GitHub Copilot. |
|
@EgorBot -amd -osx_arm64 Supersedes the earlier EgorBot request on this PR (which used an invalid using System.Collections.Generic;
using BenchmarkDotNet.Attributes;
using Microsoft.Extensions.Configuration;
public class ConfigBinderBench
{
private IConfigurationSection _list = null!, _dict = null!, _listAllInvalid = null!;
[GlobalSetup]
public void Setup()
{
var list = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) list[$"Root:{i}"] = i.ToString();
_list = new ConfigurationBuilder().AddInMemoryCollection(list).Build().GetSection("Root");
var dict = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) dict[$"Root:k{i}"] = i.ToString();
_dict = new ConfigurationBuilder().AddInMemoryCollection(dict).Build().GetSection("Root");
var bad = new Dictionary<string, string?>();
for (int i = 0; i < 100; i++) bad[$"Root:{i}"] = "not_an_int";
_listAllInvalid = new ConfigurationBuilder().AddInMemoryCollection(bad).Build().GetSection("Root");
}
[Benchmark] public List<int>? BindList() => _list.Get<List<int>>();
[Benchmark] public Dictionary<string, int>? BindDict() => _dict.Get<Dictionary<string, int>>();
[Benchmark] public List<int>? BindList_AllInvalid() => _listAllInvalid.Get<List<int>>(); // zero-success negative control
}Interpretation caveat. Note This comment was generated with the assistance of GitHub Copilot. |
Status: still WIP / draft — do not review or merge yetThis PR remains an experiment in draft. Current state:
Keeping this in draft until independent confirmation lands. Feedback on the approach is welcome; please hold formal review. Note This comment was generated with the assistance of GitHub Copilot. |
Note
Draft / experiment — please do not merge yet. Kept in draft while independent hardware A/B (@EgorBot) is pending. Contact @artl93.
Motivation
Configuration binding runs at app startup and again on every options reload (an
IOptionsMonitorchange token re-binds the options graph). In the reflection binder, every bound dictionary entry and every bound collection item allocates a throwaway one-elementobject[]to carry the reflection argument. Apps that bind large or numerous collections/dictionaries (feature-flag maps, endpoint lists, per-tenant settings) pay that per-item allocation on every bind, adding steady-state Gen0 pressure in long-running services.Change
Hoist a single
object?[1]argument array out of the child loop inBindDictionaryandBindCollectionand overwrite slot 0 each iteration, instead of allocating a fresh array per child. This mirrors the pattern the siblingBindSetmethod has used for years. The array is allocated lazily on the first successful bind (indexerArguments ??= new object?[1]/addArguments ??= new object?[1]), so routes that are entered but bind zero items (all-invalid values swallowed, or an empty/absent section) allocate nothing — there is no zero-success regression. InBindCollectionthe lazy allocation is gated on the presence of anAddmethod, so the no-Addinterface path stays allocation-neutral. Source generator is untouched.Why the reuse is safe:
PropertyInfo.SetValue/MethodInfo.Invokeare synchronous and copy the argument array by value into the target frame before returning (MethodBaseInvoker.CheckArgumentscopies each element into separate invocation storage;PropertyInfo.SetValueallocates its own setter-argument array and copies the index into it). The private, method-local array never escapes, is never read after the call, and is distinct per (recursive) invocation, so overwriting slot 0 on the next iteration is unobservable. Exception type/timing/order is unchanged: the array is only touched on the success path after conversion and recursion have already completed.Benchmarks (canonical: BenchmarkDotNet exact-parent A/B) + EgorBot
BenchmarkDotNet 0.14.0,
MemoryDiagnoser,InProcessEmitToolchain, WarmupCount=4, IterationCount=12. macOS 26.5.2, Apple M5 Pro, .NET SDK 11.0.100-preview.6, .NET 11.0.0 Arm64 RyuJIT. Each arm force-loads the freshly-built binder for that revision; the harness records the SHA-256 of the loadedMicrosoft.Extensions.Configuration.Binder.dllat runtime and asserts the two arms differ whileConfiguration/Abstractions/Primitivesare byte-identical:1e173d8b…(this PR,3888f0b) vs baseline binderb389076e…(exact parent7aa830a)4f4f167e…, abstractionsda91bc1f…, primitivesd3fcdb36…Δ = candidate − baseline, Allocated bytes/op:
Model: Δ ≈ −32 × (N − 1) bytes/op for N ≥ 1 (one
object[1]per successfully bound child eliminated; the single reused array is still allocated once, lazily, so N=1 nets zero). Mean times are within run-to-run noise — this is purely an allocation reduction with no throughput change.Fixed-shape @n=100 (Allocated Δ):
Dict<string,int>−3,169 ·Dict<string,Color>(enum) −3,169 ·HashSet<int>−3,168 · nestedDict<string,List<int>>−3,172. Options-reload ×10 @100: List −31,691, Dict −31,688.Controls — Δ = 0 (byte-identical):
int[]andIReadOnlyList<int>(BindArray),ISet<int>andIReadOnlySet<int>(BindSet), scalar POCO (BindProperties). The change is confined to the two touched routes.Zero-success negative controls — Δ = 0 (no regression): when a changed route is entered but binds zero items (all values fail conversion and are swallowed, or a missing/absent section), lazy allocation means the array is never created:
List<int>@100 all-invalid,Dict<string,int>@100 all-invalid, and List/Dict missing-section are all 0. (An earlier eager-allocation variant showed +32 B/op here; lazy-init eliminates it.)A custom in-process
GC.GetAllocatedBytesForCurrentThreadallocation probe (warmup=300, iters=4000) corroborates the same deltas within ±0.5 %.Independent hardware confirmation via
@EgorBot -amd -osx_arm64(targeting head3888f0b) is requested in a comment below. Blocked-gate caveat:Microsoft.Extensions.Configuration.Binderis an out-of-band NuGet package, not part of the shared framework. EgorBot's runtime-diff harness diffs the shared framework between two commits, so it will load the released binder in both arms and cannot exercise this change — an expected ~0 delta there means "not measured", not "no effect". The authoritative measurement is the local exact-parent binder-swap BDN above, which is proven to load distinct binders per arm.Validation
Microsoft.Extensions.Configuration.Binder.Tests: 304 passed / 0 failed / 0 skipped, including 3 addedNotSourceGenModetests (custom-collection add order; custom-dictionary set order + overwrite;ErrorOnUnknownConfigurationtiming).Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests: 326 passed / 0 failed / 31 skipped (generator untouched; matches baseline).Risks
Lineage
7aa830a03599a8255c2c4abf2947afc5b346cc6f3888f0bea6f492ebfd76baba37349832f9332a8eNote
This PR description was generated with the assistance of GitHub Copilot.