perf: cache per-render arrays/LINQ in DataGrid#669
Draft
azchohfi wants to merge 3 commits into
Draft
Conversation
The DataGrid rebuilt arrays and re-ran LINQ on every render even when the underlying sort/filter/column/width state was unchanged. On the data-grid stress workload (StressPerf.ReactorGrid, the grid being benchmarked) this dominated Renders/sec, reconcile, diff, and memory. DataGridState.cs: - Add SortVersion/FilterVersion/ColumnVersion counters bumped by every mutation path (ToggleSort, SetFilter/ClearFilter/ClearAllFilters, Resize/Hide/Show/Reorder/Pin), used as cache keys. - O(1) lookups via dictionaries kept in sync with the lists: GetSortDirection (_sortDirByField), GetFilter (_filterByField), GetColumnWidth/PinColumn (_columnIndexByName), replacing per-column LINQ FirstOrDefault/FindIndex. - Columns getter serves a cached _visibleColumns list (version-keyed) instead of Where+ToList on every access. - GetColumnLayout(...) caches the per-column widths + a single shared GridDefinition keyed on ColumnVersion + shape; the header and every data row reuse the one definition reference so the reconciler skips re-applying ColumnDefinitions. - SelectRangeByKeyCache scans the internal row-key string[] by index instead of materializing the whole cache into a List<RowKey> on shift-click. DataGridComponent.cs: - Memoize the DataRequest (also stabilizes UseDataSource deps, fixing a spurious per-render pagination restart) and the sort key (UseMemo). - Root grid + data/header rows build GridElements directly from cached GridDefinitions (static root-row-def cache; shared column layout) instead of allocating string[]/double[] + per-column double.ToString each render. - Cache the once-wired LostFocus setter array in a UseRef. The new perf-only state members are internal (InternalsVisibleTo Reactor.Tests), so the public API surface is unchanged. #126 (per-row Element?[] pooling) and #131B (validation-summary cache) are intentionally not done; see the issue for the correctness rationale. Adds DataGridPerfCacheTests covering cache invalidation, O(1)-lookup parity with the old LINQ, the shared layout cache's reference stability, the visible-columns cache, and SelectRangeByKeyCache parity. Closes #663 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Review-phase fixes for PR #669 (pr-review skill + multi-model cross-check): - GetColumnLayout: key the width/GridDefinition cache on the caller-supplied columns list (reference + count), not only ColumnVersion+shape. ColumnVersion only tracks internal mutations (resize/hide/show/reorder/pin); a swapped el.Columns/auto-columns list could otherwise be served a stale, wrong-sized layout (and index out of range in the row renderer). [HIGH, confirmed] - DataGridComponent: hoist both editable-grid UseRef hooks out of the if (el.Editable) branch so the hook call order is stable when Editable toggles between renders (avoids HookOrderException). [HIGH] - Columns getter: build a fresh visible-column snapshot on invalidation instead of returning the internal list or clearing the cached buffer in place, so a previously returned IReadOnlyList is never mutated by a later column change (restores the snapshot semantics of the old Where(...).ToList()). [LOW] Tests: add coverage for layout-cache invalidation on a columns-reference/count change, content invalidation on reorder/pin/toggle-visibility, the Columns snapshot guarantee, unknown-column lookup parity, and SelectRange reversed/ missing/empty parity. Full Reactor.Tests: 9688 passed / 0 failed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets DataGrid render-path allocation hotspots in src/Reactor/Controls/DataGrid/ by introducing version-keyed caches and O(1) lookups in DataGridState, and by memoizing / reusing stable layout objects in DataGridComponent. The intent is to reduce per-render LINQ/array churn without changing the public API surface.
Changes:
- Add sort/filter/column version counters and dictionary-based O(1) lookups in
DataGridState, plus cached visible-columns and cached column-layout (GridDefinition+ widths) keyed by versions. - Update
DataGridComponentto memoizeDataRequestand the legacysortKey, and to buildGridElements directly from cachedGridDefinitions (root grid + header/rows) to avoid per-render string/double arrays. - Add
DataGridPerfCacheTeststo validate cache invalidation, lookup parity with prior LINQ behavior, and range-selection parity.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Reactor.Tests/DataGridPerfCacheTests.cs | Adds focused unit tests validating the new caching/versioning behavior and ensuring output parity with previous LINQ-based logic. |
| src/Reactor/Controls/DataGrid/DataGridState.cs | Introduces version counters, lookup dictionaries, visible-columns caching, column-layout caching, and an allocation-free range selection path over the row-key cache. |
| src/Reactor/Controls/DataGrid/DataGridComponent.cs | Memoizes request/sort key and reuses cached GridDefinitions by constructing GridElement directly, reducing per-render allocations and helping reconciler skip definition reapplication. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Cache the search TextBox's onChanged handler in a UseRef so it keeps a stable reference across renders instead of allocating a fresh closure each time. It captures only the stable `state`, so a once-built handler is equivalent to rebuilding it every render. The ref is declared unconditionally (hooks must run in a stable order; ShowSearch can toggle), mirroring the #131A LostFocus-setter pattern. This is the only cell/toolbar-facing handler created in the component's hook context. Every other DataGrid handler (per-row click/expand/edit/select, header sort/resize/reorder/pin) is built inside the static, virtualized RenderRow / RenderHeaderRow methods, which run in VirtualList's renderItem factory with no hook context — UseCallback/UseRef cannot be called there, so they can't be stabilized without a separate state-backed delegate cache. On this branch the Element skip path (Element.CanSkipUpdate) compares callback *presence*, not delegate identity (ShallowEquals intentionally ignores delegate equality), so those fresh closures do not currently defeat the skip path; reference-stabilizing them only matters once the Core reference-comparison skip-path lands and is best done as a coordinated follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks
Collaborator
Author
|
/perf |
⚡ Reactor perf comparisonWorkload: Regression vs
|
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Renders/sec ↑ | 2.63 | 2.61 | -0.3% 95% CI [-3.1, +2.4] | ≈ within noise |
| Avg Reconcile (ms) ↓ | 145.4 | 146.9 | -0.3% 95% CI [-4.3, +3.7] | ≈ within noise |
| Avg Diff (ms) ↓ | 132.4 | 134.1 | +0.6% 95% CI [-3.7, +4.8] | ≈ within noise |
| Avg Memory (MB) ↓ | 294.1 | 293.6 | -0.7% 95% CI [-2.1, +0.6] | ≈ within noise |
Allocation (Reactor) — lower is better
| Metric | main (baseline) |
This PR | Δ (95% CI) | Status |
|---|---|---|---|---|
| Alloc bytes/render ↓ | 9605750 | n/a | — | — |
| Gen0 GC / 1k renders ↓ | 291.01 | n/a | — | — |
Cross-framework reference (same StocksGrid workload)
| Metric | vanilla WinUI3¹ | Rust windows-reactor² |
Reactor (this PR) |
|---|---|---|---|
| Renders/sec ↑ | 3.33 | 4.71 | 2.61 |
| Avg Reconcile (ms) ↓ | n/a | 20.4 | 146.9 |
| Avg Diff (ms) ↓ | n/a | 17.8 | 134.1 |
| Avg Memory (MB) ↓ | 264.6 | 196.3 | 293.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).
¹ 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 1042823130.
Generated by .github/workflows/perf-compare.yml · PR 9f640ba vs main 66d38dc · 2026-06-26T07:33:57Z · run log.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #663
Problem
The DataGrid (
src/Reactor/Controls/DataGrid/) — which is the grid measured by theStressPerf.ReactorGriddata-grid stress workload — rebuilt arrays and re-ran LINQ on every render, even when the underlying sort/filter/column/width state was unchanged. These per-render allocations were first-order contributors to the C# vs Rust gap (Renders/sec, Avg Reconcile, Avg Diff, Avg Memory).Changes
All edits are confined to
src/Reactor/Controls/DataGrid/. New perf-only state members areinternal(covered byInternalsVisibleTo Reactor.Tests), so the public API surface is unchanged.DataGridState.csSortVersion/FilterVersion/ColumnVersion) bumped by every mutation path —ToggleSort,SetFilter/ClearFilter/ClearAllFilters,ResizeColumn/HideColumn/ShowColumn/ToggleColumnVisibility/ReorderColumn/PinColumn— used as cache keys. No-op mutations don't bump (preserving the existingStateChangedsemantics).GetSortDirection(_sortDirByField),GetFilter(_filterByField),GetColumnWidth/PinColumn(_columnIndexByName) — replacing per-column LINQFirstOrDefault/FindIndex(fix(devtools): accept HTTP.SYS-owned ports in PortOwnership check #128).Columnsgetter serves a cached_visibleColumnslist (version-keyed) instead ofWhere + ToListon every access (ci: bump fast-xml-parser, @react-native-community/cli and react-native-windows in /tests/stress_perf_rn/StocksGrid #127).GetColumnLayout(...)caches the per-column widths + a single sharedGridDefinitionkeyed onColumnVersion+ shape. The header row and every data row reuse the one definition reference, so the reconciler skips re-applyingColumnDefinitions(Spec 035 — Demo Script Tool sample app design #125).SelectRangeByKeyCachescans the internal row-keystring[]by index instead of materializing the whole cache into aList<RowKey>on shift-click (boxed one struct per row — 100k+ on large client-fallback loads) (UseAnnounce.Announce should marshal to the UI thread automatically #130).DataGridComponent.csDataRequest(Spec 034 — Element allocation reduction (EX1+EX3+EX4) + investigation #124) and the sort key (ci: bump fast-xml-parser, @react-native-community/cli and react-native-windows in /tests/stress_perf_rn/VirtualList #123) withUseMemokeyed on the version counters. TheDataRequestmemo also stabilizesUseDataSource's deps, fixing a latent spurious per-render pagination restart (the oldSorts.ToList()/Filters.ToList()produced new list references every render).GridElements directly from cachedGridDefinitions — a static root-row-def cache (docs(devtools-skill): clarify lockfile auth and add scripted-loop guidance #129) and the shared column layout (Spec 035 — Demo Script Tool sample app design #125) — instead of allocatingstring[]+double[]+ per-columndouble.ToStringeach render.LostFocussetter array in aUseRef(#131A).Intentionally not done
Element?[]): the array becomesGridElement.Childrenand is retained by the reconciler/VirtualList across diffs — returning it to anArrayPoolwould corrupt a live array. Documented in perf: cache per-render arrays/LINQ in DataGrid #663._editingVersion; zero benchmark impact (the workload does no editing). Documented in perf: cache per-render arrays/LINQ in DataGrid #663.Behavior preservation
Sorting, filtering, pagination, column show/hide/pin/resize, and range selection are unchanged. Every mutation path bumps the matching version so caches invalidate correctly.
Validation
dotnet build src/Reactor/Reactor.csproj -c Release— 0 warnings, 0 errors (no new AOT/trim warnings).dotnet test tests/Reactor.Tests— 9681 passed, 0 failed, including 11 newDataGridPerfCacheTests(cache invalidation on sort/filter/column/width change; O(1) lookups match the old LINQ; shared layout cache reference stability; visible-columns cache;SelectRangeByKeyCacheparity with the explicit-visibleOrderpath).--self-test --filter DataGrid, real WinUI window) — 0 failures (paging, scroll, edit lifecycle, edit mutation, search+sort).