Resolve the config section once per type (P5)

.github/workflows/benchmark.yml

@@ -63,7 +63,7 @@ jobs:
           dotnet run -c Release
           --project performance/ExistForAll.SimpleSettings.Benchmark
           --
-          --filter "*ScanBenchmark*" "*EnumerateBenchmark*" "*EnvBinderBenchmark*" "*GenerateTypeBenchmark*" "*PlanPopulateBenchmark*"
+          --filter "*ScanBenchmark*" "*EnumerateBenchmark*" "*EnvBinderBenchmark*" "*GenerateTypeBenchmark*" "*PlanPopulateBenchmark*" "*ConvertArrayBenchmark*" "*ConfigBinderBenchmark*"
           --job short
           --exporters json
           --artifacts ${{ github.workspace }}/bdn

FIX-PLAN.md

@@ -6,8 +6,9 @@ _Derived from the 2026-07-10 three-part review (architecture · tests · perform
 - **Done & merged:** B1, B2, B4, B5, B9 + T1, T2 (PR #8) · BindingContext test (#10) · D3 namespace typo (#11) · T3 DI integration tests (#12) · solution rename (#13) · **A2 naming → ExistForAll (#15)** · **P0 benchmark harness (#16)** · **P1 provider cache + C3 decided/implemented (#17)** · **P2 memoize `ExtractTypeProperties` + `HashSet` dedup (#18)** · **docs tutorials refresh (#20)** · **Q1–Q4 perf quick wins + M1 collision fix + micro-benchmarks (#21)**.
 - **Q1–Q4 proven** via isolated micro-benchmarks (macro `ScanBenchmark` can't resolve them): Q1 2.7× / 64 KB→88 B · Q3 2.65× / 152 B→0 · Q4 32× / 224 B→0. **Q5 was already resolved by B4.** **M1** (code-review finding): namespace-qualify the generated impl name in the generator only — `GetNormalizeInterfaceName` also backs the section name. Suite → **56 per TFM**.
 - **Merged since:** **benchmark-tracking CI (#22)** — BDN on push/PR, gates PRs on **allocation** regressions (>10%) via github-action-benchmark on `gh-pages`; time informational. · **session-wrap docs (#23)** · **P3 — cached "settings plan" (#24)** — `SettingsPlan` per type (section name once+lazy, key/default/converter precomputed, `[SettingsProperty]` read once). Warm re-populate **−55–61%** (50 props 15,681→6,816 B); gated `ScanBenchmark` **≈flat (−0.4%)**. Reviewed by code+perf agents; emitted/compiled setter reverted (regressed the gated cold scan for **no** warm gain — net10 `SetValue` is already alloc-free). New gated `PlanPopulateBenchmark` tracks the warm path.
-- **In flight:** **P4 — de-reflect + DRY the array/enumerable converters** = **PR #25 open, code-reviewed clean** (branch `perf/p4-dereflect-converters`; awaiting CI + merge). New shared `CollectionTypeConverter` builds results via `Array.CreateInstance` + indexed fill and selects the element converter by walking the concrete `LinkedList` (struct enumerator) — no `List<T>`+reflected `Enumerable.ToArray`, no `First` closure. `TypeConverter.CreateNullResult` de-reflected too (`Array.CreateInstance(t,0)` for `Enumerable.Empty<T>()`). Proven via new gated `ConvertArrayBenchmark`: **1.33 KB→688 B (−49%), 1,247→219 ns (5.7×)**. Suite **68 per TFM** (+12 collection-converter parity tests). Branch also carries the pre-P4 doc refresh + post-P3 style tweaks to `TypeConverter.cs`/`TypeExtensions.cs`.
-- **Next:** P5 (resolve config section once per type) → optional P3b (tiered/lazy compiled setter, only if set *time* shows in a profile).
+- **P4 merged (#25)** — de-reflect + DRY the array/enumerable converters: shared `CollectionTypeConverter` (`Array.CreateInstance` + indexed fill, manual `LinkedList` walk); `CreateNullResult` de-reflected. Gated `ConvertArrayBenchmark`: **1.33 KB→688 B (−49%), 5.7×**. Suite 68/TFM. Included a user modernization pass (collection expressions across ~19 files).
+- **In flight:** **P5 — resolve config section once per type** = **implemented + reviewed clean on branch `perf/p5-resolve-section-once`** (awaiting commit/PR at time of writing). `ConfigurationBinder` caches the `IConfigurationSection` per section name (`ConcurrentDictionary`, zero-capture `GetOrAdd`). Plan reviewed by architect+perf+security (chose internal cache over a contract change — layering); code reviewed via `/code-review`. Gated `ConfigBinderBenchmark`: **BindNoRoot 80→40 B (−50%), BindWithRoot 144→56 B (−61%)**. Suite **71/TFM** (+3 parity/live-view tests). Also gated P4's `ConvertArrayBenchmark` (was never in the CI filter). **Security review surfaced a pre-existing leak → new item S1.**
+- **Next:** S1 (redact secret from exception message) · optional P3b (tiered/lazy compiled setter, only if set *time* shows in a profile).
 - **C3 — DECIDED (option 2):** cache in the provider only; Core `SettingsBuilder.GetSettings` unchanged; no reload. See #17.
 - **Held — do NOT delete (feature work coming):** D1 Validations (reconcile with the `validate-settings` branch) · D2 EqualityCompererCreator.
 - Running status lives in `SESSION-HANDOFF.md`.
@@ -58,8 +59,9 @@ _Derived from the 2026-07-10 three-part review (architecture · tests · perform
 - [x] P2 · Memoize `ExtractTypeProperties` + fix O(n²) dedup · Sev High · Eff S
 - [x] P3 · Cached “settings plan” — hoist section (lazy) + keys, precompute/cache converters, plan per type. Reflective `SetValue` kept; compiled setter deferred (regressed the gated cold scan for no warm gain) · Sev High · Eff L
 - [x] Q1–Q5 · Quick wins (GetEnumerator, OrdinalIgnoreCase, env-binder, type-cache; Q5 dead ctor checks already done by B4)
-- [x] P4 · De-reflect + DRY array/enumerable converters (shared `CollectionTypeConverter`; `Array.CreateInstance` + manual converter walk; `CreateNullResult` de-reflected) — **1.33 KB→688 B, 5.7×**; branch `perf/p4-dereflect-converters` · Sev Med · Eff M
-- [ ] P5 · Resolve config section once per type, not per property · Sev Med · Eff M
+- [x] P4 · De-reflect + DRY array/enumerable converters (shared `CollectionTypeConverter`; `Array.CreateInstance` + manual converter walk; `CreateNullResult` de-reflected) — **1.33 KB→688 B, 5.7×**; merged #25 · Sev Med · Eff M
+- [x] P5 · Resolve config section once per type — `ConfigurationBinder` caches the `IConfigurationSection` per section name (`ConcurrentDictionary`, zero-capture `GetOrAdd`) — **BindNoRoot 80→40 B (−50%), BindWithRoot 144→56 B (−61%)**; branch `perf/p5-resolve-section-once` · Sev Med · Eff M
+- [ ] S1 · Redact secret values from `SettingsPropertyValueException` message (`Resources.cs:34-36` interpolates the raw bound value → leaks secrets into logs); log type/length, not the value · Sev Med · Eff S · **found in P5 security review**
 
 **Phase 6 — Architecture strategy**
 - [ ] A1 · Decide AOT/trim story; annotate `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` and/or plan a source generator · Sev High · Eff M–L
@@ -224,8 +226,9 @@ The populate loop (`src/Core/ExistForAll.SimpleSettings/ValuesPopulator.cs:36-55
 Was: `TypeConverter.cs` (empty enumerable via `Enumerable.Empty` `MakeGenericMethod().Invoke()`), `ArrayTypeConverter` (`Activator.CreateInstance(List<>)` + reflected `Enumerable.ToArray` `Invoke`), `EnumerableTypeConverter` (`Activator.CreateInstance(List<>)`), both selecting the element converter with LINQ `First` (boxes the `LinkedList` enumerator + a closure).
 Now: a shared `CollectionTypeConverter` base implements `Convert` once — normalize the value to an array (split delimited string / passthrough / wrap scalar), select the element converter by walking the concrete `LinkedList` (struct enumerator, no boxing/closure), then fill an `Array.CreateInstance(elementType, n)` by index. `ArrayTypeConverter`/`EnumerableTypeConverter` are now thin subclasses differing only in `CanConvert` + element-type extraction; both return `T[]` (safe — `IsEnumerable()` matches only `IEnumerable<T>`, which a `T[]` satisfies). `CreateNullResult` uses `Array.CreateInstance(t,0)` instead of the `Enumerable.Empty<T>()` reflection. **Proof (`ConvertArrayBenchmark`, gated): 1.33 KB→688 B (−49%), 1,247→219 ns (5.7×).** 12 parity tests in `Conversion/CollectionConversionTests.cs` (int/string/enum/DateTime/Uri elements, empty-entry removal, custom delimiter, default passthrough, unbound→empty `T[]`, `T[]`-not-`List` guard, bad-element negative). Residual 688 B = irreducible split-substrings + element boxing + result array (shared by old & new). **Code-reviewed clean** (dotnet code-reviewer verified all parity claims; the enum/DateTime/Uri + null-path + negative tests were its suggestions — partially closes T6).
 
-### P5 · Resolve config section once per type  — Sev Med · Eff M
-`ConfigurationBinder.BindPropertySettings` (`ConfigurationBinder.cs:25-33`) calls `_configuration.GetSection(...)` **per property**; the section is constant per type. Resolve the `IConfigurationSection` once per (type, section) — pass section context via the plan (P3) or cache per section string. Touches the binder/context contract.
+### P5 · Resolve config section once per type  — Sev Med · Eff M · **DONE (branch `perf/p5-resolve-section-once`)**
+Was: `ConfigurationBinder.BindPropertySettings` called `_configuration.GetSection(...)` per property (a fresh `ConfigurationSection` alloc each time) + a `$"{RootSection}:{Section}"` interpolation per property when a root is set — though the section is constant per type.
+Now: the binder caches the resolved `IConfigurationSection` per section name in a `private readonly ConcurrentDictionary<string, IConfigurationSection>`, via a **zero-capture** `GetOrAdd(context.Section, static (name, self) => self.ResolveSection(name), this)` (a capturing lambda would allocate a 64 B delegate per call — measured). Kept the internal-cache approach (Option 2), **not** a contract change: the three-specialist plan review found threading `IConfigurationSection` through the Core `ISectionBinder`/`BindingContext` contract to be a layering violation (Core must not reference `Microsoft.Extensions.Configuration`), and the optimization is single-implementer (env/cmdline/in-memory binders are flat lookups). Reload-safe: `GetSection` returns a live view, so the cached section re-reads providers on each access (locked by a test). Dropped the dead `?.` (`GetSection` never returns null); stripped a stray BOM. **Proof (`ConfigBinderBenchmark`, gated): BindNoRoot 80→40 B (−50%), BindWithRoot 144→56 B (−61%)** — matched the perf review's predicted −40/−88 B deltas. 3 parity/live-view tests; also wired P4's `ConvertArrayBenchmark` into the CI filter (it was never gated). Reviewed clean (`/code-review`, after the `dotnet-claude-kit:code-reviewer` agent misfired 3×).
 
 ---