Conversation
…ed caption (P3) UpdateDefaultAutomationName ran a UIA GetName read + SetName write on every changed cell that goes through Update, even when the caption was unchanged - where the resulting Name write is a value no-op (or hits the author-override guard the GetName already protects). Add a caption-only fast-path that returns before touching the DP when the new caption is empty/whitespace or equals the old caption, and factor the remaining decision into a pure, DP-free ResolveDefaultAutomationNameUpdate helper so the caption/override policy is unit-testable headlessly. On a changed caption the original GetName + author-override + SetName path runs unchanged, so a genuine caption change still flows to UIA and author-set names are never clobbered. Scope is Reconciler.cs only (file-disjoint from the P1 PR #692). New headless tests pin the decision, including a teeth case: reverting the unchanged-caption skip makes the empty-live-Name assertion return the caption instead of null and fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…guard Addresses Copilot review on #695. The first cut skipped UpdateDefaultAutomationName whenever oldCaption == newCaption. That is unsafe: ApplyModifiers runs BEFORE this (Update.cs:188-200) and can clear AutomationProperties.Name when an explicit .AutomationName() override is removed - even though the caption is unchanged. The blanket skip then left UIA Name empty instead of restoring the caption-derived default (which main does). Replace the unchanged-caption skip with an idempotent-write guard: keep main's GetName + author-override logic verbatim, compute the trimmed target, and skip only the SetName when the live Name already equals it (a value no-op). This still WRITES when the default must be (re)applied - including the cleared-Name-unchanged-caption restore case - so behavior is identical to main minus the redundant same-value write. Tests rewritten: the teeth now pin the idempotent guard (revert it -> the skip test sees a redundant 'X' write and fails) AND the restore-default case (a blanket unchanged-caption skip -> the restore test returns null and fails). Added an author-override-survives-unchanged-caption case. Full Reactor.Tests 9714 passed / 0 failed / 64 skipped; core-lib Release AOT 0W/0E. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetName still runs for non-whitespace captions; the P3 saving is the skipped redundant SetName inside the helper. Reword the comment so it no longer implies the GetName read is removed for unchanged captions (Copilot review on #695). Comment-only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pr-review (test-coverage L1) flagged that the P3 idempotent-write guard's live UIA seam was only pinned headlessly via the pure helper ResolveDefaultAutomationNameUpdate. The subtle restore-default branch — a removed .AutomationName() override clears the live Name (ApplyModifiers ClearValue) and the guard must re-apply the caption default even though the caption is unchanged — was not proven end-to-end through a real control. Extend the CoreCov_AccessibilityModifiers selftest with a third phase that drops the .AutomationName() override (caption unchanged) and asserts AutomationProperties.GetName restores the caption-derived default, plus assertions that the author override wins at mount and a changed override still flows through. Exercises UpdateDefaultAutomationName via a live WinUI control. Teeth: a blanket unchanged-caption skip (or dropping the live SetName) leaves the Name cleared at phase 2 -> A11y_Name_RestoredToCaptionDefault flips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reword the UpdateDefaultAutomationName comment so it attributes the conditional DP access to the SetName *write*, not a vague "touch the DP" — the GetName read always runs for a non-whitespace caption; only the SetName write is skipped when the helper returns null. Comment-only; no behavior change. Addresses the Copilot review thread at Reconciler.cs:2693. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/perf |
⚡ Reactor perf comparisonWorkload: Regression vs
|
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Renders/sec ↑ | 2.67 | 2.58 | -3.4% 95% CI [-11.6, +4.8] | ≈ within noise |
| Avg Reconcile (ms) ↓ | 133.6 | 132.5 | +0.9% 95% CI [-7.4, +9.1] | ≈ within noise |
| Avg Diff (ms) ↓ | 121.2 | 119.7 | +1.5% 95% CI [-7.0, +10.1] | ≈ within noise |
| Avg Memory (MB) ↓ | 286.4 | 284.6 | +0.1% 95% CI [-0.8, +0.9] | ≈ within noise |
Low-mutation skip-floor (--percent 0)
At --percent 0 the workload mutates few cells per tick (always at least one), so reconcile/diff isolate the O(n) per-tick child skip-walk floor that higher mutation rates dilute — ChildReconciler re-walks every child each tick even when nothing moved. The closer --percent is to 0, the more this floor is the signal, so a structural-skip optimization shows up cleanly where the headline table above buries it. Δ is the mean paired change with a 95% CI.
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Renders/sec ↑ | 15.83 | 16.78 | +5.2% 95% CI [-1.9, +12.3] | ≈ within noise |
| Avg Reconcile (ms) ↓ | 36.2 | 36.7 | +1.4% 95% CI [-4.2, +7.0] | ≈ within noise |
| Avg Diff (ms) ↓ | 34.1 | 34.6 | +1.7% 95% CI [-4.6, +8.1] | ≈ within noise |
| Avg Memory (MB) ↓ | 267.3 | 267.6 | +0.1% 95% CI [-0.1, +0.3] | ≈ within noise |
Allocation (Reactor) — lower is better
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Alloc bytes/render ↓ | 5779177 | 5775695 | +0.2% 95% CI [-1.4, +1.7] | ≈ within noise |
| Gen0 GC / 1k renders ↓ | 230.77 | 230.77 | +1.3% 95% CI [-12.7, +15.4] | ≈ within noise |
Keyed-list workload (StressPerf.KeyedList, --percent 50)
A separate macro workload: a ~500-row stably keyed list whose rows are reordered / inserted / removed each tick. Because every child carries a key, the child reconciler takes its keyed arm (ReconcileKeyed → ReconcileKeyedMiddle, the LIS-based minimal-move pass) instead of the positional re-walk the StocksGrid tables above measure — so this is the sensitive macro signal for keyed-diff work the positional cells can never reach. Same interleaved paired-Δ 95% CI as the headline table.
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Renders/sec ↑ | 20.55 | 20.45 | +0.7% 95% CI [-1.4, +2.9] | ≈ within noise |
| Avg Reconcile (ms) ↓ | 16.3 | 16.4 | +0.6% 95% CI [-1.2, +2.5] | ≈ within noise |
| Avg Diff (ms) ↓ | 16.1 | 16.2 | +0.6% 95% CI [-1.2, +2.4] | ≈ within noise |
| Avg Memory (MB) ↓ | 168.5 | 169.0 | 0.0% 95% CI [-0.8, +0.8] | ≈ within noise |
Allocation (keyed-list) — lower is better
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Alloc bytes/render ↓ | 313916 | 314068 | +0.1% 95% CI [-0.1, +0.4] | ≈ within noise |
| Gen0 GC / 1k renders ↓ | 18.65 | 19.05 | +7.9% 95% CI [-5.5, +21.4] | ≈ within noise |
Reconciler micro-benchmarks (PerfBench.ControlModel)
Production --variant Reactor control-model path, ns-resolution and WinUI-undiluted (spec-047 M1–M13) — ↓ lower is better. Status tracks allocated bytes/op, the authoritative signal here; it is deterministic for structurally-fixed benches, while dispatcher / background-thread benches carry a small process-to-process offset, so a bench is flagged only when its 95% CI clears a ±3% minimum-effect band (real structural alloc changes are several percent to many-x). ns/op is shown for context but is not auto-flagged (its paired CI is rep-interleaved but the flag remains dormant pending a real-CI identical-binary band calibration). Δ is the mean paired change with a 95% CI.
| Bench | main ns/op |
Δ ns (95% CI) | main B/op |
Δ alloc (95% CI) | Status |
|---|---|---|---|---|---|
M1 Mount_Leaf_NoCallback |
147705.4 | +0.1% 95% CI [-2.4, +2.6] | 1140.9 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M2 Mount_Leaf_OneCallback |
110063.9 | -0.5% 95% CI [-5.7, +4.8] | 3383.3 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M3 Mount_Leaf_ThreeCallbacks |
217649.4 | +1.3% 95% CI [-2.0, +4.6] | 8423.4 | +0.4% 95% CI [-1.7, +2.5] | ≈ within noise |
M4 Dispatch_Switch_Cold |
103978.6 | +1.1% 95% CI [-4.6, +6.9] | 1767.8 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M5 Dispatch_Switch_Warm |
106960.2 | -0.2% 95% CI [-4.0, +3.7] | 1766.0 | -1.1% 95% CI [-2.8, +0.7] | ≈ within noise |
M6 Dispatch_ExternalType |
90863.3 | +1.2% 95% CI [-0.6, +2.9] | 987.6 | -1.9% 95% CI [-5.0, +1.2] | ≈ within noise |
M7 Update_NoChange |
55006.2 | -0.7% 95% CI [-2.9, +1.6] | 452.1 | +5.9% 95% CI [-2.4, +14.2] | ≈ within noise |
M8 Update_OneLeafChanged |
41305.6 | +0.9% 95% CI [-0.2, +2.1] | 536.0 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M9 Update_AllChanged |
2846690.6 | +1.2% 95% CI [+0.1, +2.3] | 184278.1 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M10 EventHandlerState_Alloc |
85230.6 | +0.3% 95% CI [-1.8, +2.3] | 3095.2 | 0.0% 95% CI [-0.1, 0.0] | ≈ within noise |
M11 ModifierEHS_Frequency |
46332.0 | -0.8% 95% CI [-2.5, +1.0] | 638.9 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M12 Pool_Rent_HotPath |
117123.5 | +0.1% 95% CI [-0.7, +0.9] | 1099.9 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M13 Setters_Suppression_Scope |
108.6 | -4.0% 95% CI [-14.3, +6.4] | 26.7 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
M14 Dsl_Rebuild_Cascade |
1540531.0 | -12.9% 95% CI [-13.9, -12.0] | 2231828.9 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
C207 ChangeHandler_DpRead_Coalesce |
1326.0 | -3.0% 95% CI [-8.2, +2.2] | 0.6 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
OAlloc Optional_Element_Alloc |
213.1 | +5.6% 95% CI [-4.7, +15.9] | 528.0 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
OUpdate Optional_Reconciler_Update |
12438.3 | -2.7% 95% CI [-4.5, -0.8] | 2772.3 | 0.0% 95% CI [0.0, 0.0] | ≈ within noise |
Cross-framework reference (same StocksGrid workload)
| Metric | vanilla WinUI3¹ | Rust windows-reactor² |
Reactor (this PR) |
|---|---|---|---|
| Renders/sec ↑ | 3.30 | 4.90 | 2.58 |
| Avg Reconcile (ms) ↓ | n/a | 19.3 | 132.5 |
| Avg Diff (ms) ↓ | n/a | 17.6 | 119.7 |
| Avg Memory (MB) ↓ | 264.4 | 196.6 | 284.6 |
↑ higher is better · ↓ lower is better. Within noise = the 95% confidence interval of the paired Δ includes 0 (no change resolvable at this sample size); ✅ improvement /
Allocation metrics (alloc bytes/render, Gen0 GC) are the sensitive signal for allocation-reduction work, where the mean-ms / memory figures are largely flat. They read n/a for a harness built from a revision that predates them (rebase the PR onto main to populate them).
Reconciler micro-benchmarks run PerfBench.ControlModel --variant Reactor (M1–M13) as a headless loop bracketed by per-thread alloc + GC counters — ns-resolution and free of WinUI render / working-set dilution, so they resolve Core/Reconciler allocation deltas the macro StocksGrid workload cannot. main and PR each link their own src/Reactor build and are rep-interleaved (a fresh alternated process per rep); Δ is the paired 95% CI over per-rep means. The Status column tracks allocated bytes/op (deterministic for identical code); ns/op is informational — its paired CI is now unbiased but the flag stays dormant pending a real-CI identical-binary band calibration.
¹ vanilla WinUI3 = StressPerf.Direct (imperative; no virtual-DOM, so it has no reconcile/diff phase — those cells read n/a). Measured live on this runner.
² Rust = test_reactor_perf from microsoft/windows-rs — a port of this harness (same StocksGrid, same --percent/--duration CLI). Built from source and measured live on this runner.
Absolute numbers are runner-dependent — trust the Δ vs main, not the absolute values. Memory (working set) is the noisiest metric.
Runner: CPU: AMD EPYC 7763 64-Core Processor · 4 logical cores · 16 GB RAM · runner: GitHub Actions 1043000949.
Generated by .github/workflows/perf-compare.yml · PR 4227b49 vs main e8572a0 · 2026-06-27T05:41:45Z · run log.
1 participant
Throwaway re-validation of #695 on origin/main b9ace1e (=#692+M14+#665+#649). DO NOT MERGE; origin #695 pristine.