Skip to content

Commit 98d760b

Browse files
bloveclaude
andauthored
perf(row-model): scalar aggregate cells + selected-root-only aggregation (#500 cycle 2) (#519)
* docs: grouping-apply insert-phase design (#500 cycle 2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(row-model): scalar accumulator cells + selected-root-only aggregation (#500 cycle 2) Decision A: sum/avg/count columns ride O(1) scalar accumulator cells behind the aggregate-root interface — insert accumulates inline (charging no seal units), remove applies the exact inverse from the originally-inserted leaf (bigint superaccumulator subtraction; ±Infinity flags become counters so mixed-infinity finalize semantics survive removals; NaN admission symmetric). min/max and custom aggregators keep the ordered tree. Decision C1: only the selected population root (aggregateFilteredRows) is built; the unselected root was write-only. representativeRowId falls back to whichever root exists. Micro: normalizedLeaf returns an already-canonical frozen leaf instead of allocating a copy per insert. Node repro 50k×10avg: 3658ms/798MB → ~540ms/~135MB. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(row-model): draft-path C1 probe + final-totalRows seal-charge pin (#500 cycle 2) Both fixes came out of mutation testing: the seal-charge pin sampled totalRows between slices and missed a charge consumed inside one slice (read the monotonic final value instead), and the cooperative-draft C1 probe used a fixture with a custom aggregator, which routes to the incremental path — the draft only runs builtin-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bench): bound the group-expand setup scan to the visible prefix waitForGroupedRowModel scanned the ENTIRE still-flat snapshot (50k rowAt calls per rAF frame at target scale) looking for a group row, stuffing every frame with harness work and starving the cooperative transition it was waiting for — the setup then blew its 120-frame budget on its own overhead and reported partial. A grouped model's sorted-first root group is the first visible row, so a 64-row prefix scan is sufficient. The measurement window is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: changeset for the grouping-apply insert phase (#500 cycle 2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: correct the C1 selected-root sentence (review nit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5ee56a1 commit 98d760b

11 files changed

Lines changed: 1237 additions & 77 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@pretable-internal/row-model": patch
3+
---
4+
5+
Grouping-apply insert phase (#500 cycle 2): `sum`/`avg`/`count` columns now
6+
aggregate through O(1) scalar accumulator cells with exact inverses instead
7+
of per-row ordered trees, only the selected `aggregateFilteredRows`
8+
population root is built (the other was write-only), and canonical frozen
9+
aggregate leaves are no longer copied on every insert. Applying grouping at
10+
50k rows × 10 `avg` columns drops from ~3.7s / ~800MB of transient heap to
11+
~0.55s / ~130MB; `min`/`max` and custom aggregators keep the ordered tree and
12+
their exact fold semantics.

‎apps/bench/src/bench-app.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,15 @@ export function BenchApp({ search, browserVersion }: BenchAppProps) {
218218
}
219219

220220
const snapshot = grid.rowModel.getState().snapshot;
221+
// A grouped model's sorted-first ROOT group is the first visible row,
222+
// so a bounded prefix scan finds it. Scanning the ENTIRE still-flat
223+
// model (50k `rowAt` calls per frame at target scale) stuffed every
224+
// frame with harness work and starved the cooperative transition this
225+
// wait exists to await — the setup then blew its frame budget on its
226+
// own overhead (#500 cycle 2).
227+
const scanLimit = Math.min(snapshot.visibleRowCount, 64);
221228
let firstGroupRow: BenchGroupRow | null = null;
222-
for (let index = 0; index < snapshot.visibleRowCount; index += 1) {
229+
for (let index = 0; index < scanLimit; index += 1) {
223230
const row = snapshot.rowAt(index);
224231
if (row?.kind === "group") {
225232
firstGroupRow = row;
@@ -263,7 +270,8 @@ export function BenchApp({ search, browserVersion }: BenchAppProps) {
263270
// run that says so beats a completed run measuring the wrong thing.
264271
const snapshot = pretableGridRef.current?.rowModel.getState().snapshot;
265272
if (!snapshot || countPaintedGroupRows() === 0) return null;
266-
for (let index = 0; index < snapshot.visibleRowCount; index += 1) {
273+
const limit = Math.min(snapshot.visibleRowCount, 64);
274+
for (let index = 0; index < limit; index += 1) {
267275
const row = snapshot.rowAt(index);
268276
if (row?.kind === "group") return row;
269277
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Grouping-Apply Insert Phase (#500 cycle 2) — Design
2+
3+
**Status:** approved direction, specced in full.
4+
**Parent:** `2026-08-29-grouping-apply-cooperative-cost.md` (cycle 1: seal
5+
units + clock — merged as PR #518). Cycle 1 fixed the seal phase; this fixes
6+
the insert phase, which measurement showed dominates: **~2.9s of the ~3.2s**
7+
at 50k rows × 10 `avg` columns.
8+
9+
## The cost, measured (investigation 2026-08-29; scripts in session scratchpad)
10+
11+
Per row, per grouping level, per aggregated column, the build inserts into
12+
TWO ordered AVL aggregate trees (`all` + `filtered` roots): ~5.8µs and
13+
~1.4KB of heap per row·column — perfectly linear at ~289ms/72MB per column on
14+
a 314ms zero-aggregate base. Three structural facts make this waste, not
15+
work:
16+
17+
1. **The comparator never reads values.** `compareAggregateLeaves` orders by
18+
row sort keys only, so a group's 20 aggregate trees are isomorphic — the
19+
engine builds 20 identical AVL shapes per group and shares nothing.
20+
2. **Scalar kinds don't need order.** `sum`/`avg`/`count` are commutative
21+
with **exact inverses** (bigint superaccumulator subtraction; count
22+
decrement; ±Infinity as counters), and streaming updates already arrive
23+
as remove-then-insert with the originally-inserted value on the removal
24+
record. Only `min`/`max` (no inverse) and **custom aggregator specs**
25+
(associativity is the only validated law — the fold order is observable)
26+
need a removal structure. The real builtin `avg` measures ~0.11µs per
27+
scalar accumulate — ~50× under the tree path.
28+
3. **One population root is write-only.** `aggregateFilteredRows` is a
29+
model-lifetime option; `aggregateRecord` finalizes only the selected
30+
root. The other root is built and never read (its `firstId` serves only a
31+
vestigial error-context path).
32+
33+
## Decisions
34+
35+
1. **(A) Kind-aware aggregation.** `sum`/`avg`/`count` columns get a scalar
36+
accumulator cell behind the same root interface
37+
(`insertOrReplace`/`remove`/`finalize`/`firstId`): insert = accumulate,
38+
remove = exact inverse (bigint units subtraction, count decrement,
39+
±Infinity flags become counters so `Inf + (−Inf) → NaN` finalize
40+
semantics survive; NaN admission applied symmetrically on both sides).
41+
`min`/`max` and custom specs keep the ordered tree unchanged. Scalar
42+
columns charge **no seal units** (accumulate is O(1) inline at insert);
43+
the cooperative resumable-unit contract is untouched for the kinds that
44+
still seal. `firstId` on a builtin-only scalar group may return
45+
undefined — its only consumer is custom-finalizer error context.
46+
2. **(C1) Build only the selected population root.** The unselected root
47+
(`filtered` when `aggregateFilteredRows` is on — the option selects the
48+
`all` population — and `all` otherwise) is
49+
write-only today; stop building it. Halves the remaining tree cost for
50+
min/max/custom AND the scalar-cell writes. `representativeRowId` falls
51+
back to the root that exists. The option is model-lifetime, so nothing
52+
can request the dropped root later.
53+
3. **(micro) `normalizedLeaf` stops copying frozen leaves** — it currently
54+
allocates a fresh frozen copy of an already-frozen leaf on every insert
55+
(~157ms + GC at 50k×10). Return the input when it is already canonical.
56+
4. **Rejected: bulk tree construction at seal (shape B)** — the O(n)
57+
`createOrderStatisticTreeFromSortedEntries` primitive exists, but B keeps
58+
the full allocation/GC bill, needs non-resumable per-group sorts inside
59+
the cooperative seal, and under A+C1 the residual tree count is too small
60+
to repay that; **rejected: multi-measure shared trees (C2)** — mostly
61+
redundant once A lands, large typing/instrumentation churn.
62+
5. **Streaming stays the CONTROL and should strictly improve** — scalar
63+
cells make per-touch group updates O(1) where trees were O(log n). The
64+
S5 group-updates 20k p95 (10.0ms) must not regress; any improvement is
65+
welcome but not claimed in advance.
66+
67+
## Invariants that must be test-pinned (the risk register, made checkable)
68+
69+
- **Inverse exactness**: insert/remove fuzz (randomized adds/removes incl.
70+
NaN, ±Infinity, duplicates) comparing the scalar cell against a tree
71+
oracle for sum/avg/count — final values identical, intermediate finalize
72+
identical at every step.
73+
- **Remove-then-insert invariant**: streaming updates must keep arriving as
74+
remove-then-insert (scalar cells cannot detect replace); pin it with a
75+
test that fails if an update path skips the remove.
76+
- **±Infinity finalize semantics**: `Inf`, `−Inf`, and mixed populations
77+
finalize identically to the tree path, including after removals that
78+
clear one flag.
79+
- **Custom specs untouched**: an order-sensitive custom aggregator (one the
80+
associativity law permits but commutativity would break) produces the
81+
SAME fold as before — proving customs still ride the ordered tree.
82+
- **C1 twins**: both `aggregateFilteredRows` configs run the full aggregate
83+
fixture set; a filter making the two populations differ proves the
84+
SELECTED root is the one surviving.
85+
- **Cycle-1 pins hold**: unit-accounting and slice bounds re-derived for
86+
scalar columns charging zero seal units (the existing pins' fixtures use
87+
aggregated columns — update the derivations honestly, don't loosen).
88+
89+
## Verification
90+
91+
- The pins above; the full row-model + react suites untouched-green.
92+
- Node repro (50k×10avg): time-to-grouped from ~3.2s to **≤ 600ms** (target
93+
~400-450ms; the gate at 600 leaves loaded-box headroom), heap Δ from
94+
~780MB to ≤ 200MB.
95+
- Bench, same-window controls: S2 `group`/`group-expand` at target scale
96+
return **`completed`** (the #500 headline); settle reported against the
97+
~400ms pre-#321 number; S5 `group-updates` 20k p95 ≤ 10.0ms (control);
98+
3k/750 settles reported. Isolated port, full output to files.
99+
- Changeset: `@pretable-internal/row-model` patch.
100+
- #500 closes only if S2 target completes; else it stays open with the
101+
residual named.
102+
103+
## Out of scope
104+
105+
The renderer-dom height-index cost (filed, task chip); shape B / C2
106+
(rejected above, revisit only with new evidence); any change to budget or
107+
slicing constants; the jsdom setDerivations stall.

‎packages/row-model/src/__tests__/expansion.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,8 +447,11 @@ function failingOverrideRoot(): {
447447
leaves: { size: 1, entryAt: () => ({ rowId: 1 }) },
448448
filteredCount: 1,
449449
allCount: 1,
450+
// Under C1 (#500 cycle 2) only the SELECTED population root exists —
451+
// `filtered` for this root's `aggregateFilteredRows: false` — so the
452+
// representative-row fallback must find `firstId` there, not in `all`.
450453
aggregateRoots: {
451-
all: new Map([["score", aggregateTree]]),
454+
all: new Map(),
452455
filtered: new Map([["score", aggregateTree]]),
453456
},
454457
aggregates: Object.freeze({ score: 1 }),

‎packages/row-model/src/__tests__/grouping-apply-cost.test.ts‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,17 @@ function groupAggregates(model: {
116116

117117
describe("grouping-apply cooperative cost (#500)", () => {
118118
test("the grouped candidate charges O(R) units, not R × columns × roots", () => {
119-
// R = 600 rows over 8 aggregated columns and 4 groups. Charged units:
119+
// R = 600 rows over 8 aggregated columns and 4 groups. The fixture is
120+
// deliberately MIXED — 4 scalar (sum/avg) + 4 tree (min/max) columns —
121+
// so both accounting regimes are pinned at once. Charged units:
120122
// insert phase: exactly R (one row evaluated per unit), then
121-
// seal phase: one unit per ROW (all aggregated columns and both
122-
// population roots sealed together) plus 2 node units
123-
// per group (finalize + parent/root edge).
124-
// The old accounting charged R × 8 columns × 2 roots = 16R seal units,
125-
// which cannot fit under R + C for any small constant C.
123+
// seal phase: one unit per ROW for the TREE-backed columns (all of
124+
// a node's still-pending trees seal together; #500 cycle
125+
// 2 builds only the selected population root and scalar
126+
// sum/avg/count columns charge nothing) plus 2 node
127+
// units per group (finalize + parent/root edge).
128+
// The original accounting charged R × 8 columns × 2 roots = 16R seal
129+
// units, which cannot fit under R + C for any small constant C.
126130
const rowCount = 600;
127131
const scheduler = new ManualScheduler();
128132
const model = createLocalRowModel({
@@ -166,6 +170,62 @@ describe("grouping-apply cooperative cost (#500)", () => {
166170
expect(AGGREGATED_COLUMN_COUNT * rowCount).toBeGreaterThan(rowCount + 64);
167171
});
168172

173+
test("scalar-only aggregate columns charge zero seal units (#500 cycle 2)", () => {
174+
// Every aggregated column here is sum/avg/count — scalar cells that
175+
// accumulate inline at insert. The seal phase therefore has ONLY node
176+
// structure to do: 2 units per group node (finalize + parent/root edge)
177+
// and completion bookkeeping — nothing proportional to R. A scalar
178+
// column falling back to the tree path re-adds ~R units and fails the
179+
// constant bound.
180+
const rowCount = 600;
181+
const scheduler = new ManualScheduler();
182+
const model = createLocalRowModel({
183+
rows: makeRows(rowCount),
184+
columns: [
185+
helper.accessor("team", { type: "text" }),
186+
helper.accessor("score", { type: "number" }),
187+
helper.accessor("m0", { type: "number", aggregate: "sum" }),
188+
helper.accessor("m1", { type: "number", aggregate: "avg" }),
189+
helper.accessor("m2", { type: "number", aggregate: "count" }),
190+
helper.accessor("m3", { type: "number", aggregate: "sum" }),
191+
helper.accessor("m4", { type: "number", aggregate: "avg" }),
192+
helper.accessor("m5", { type: "number", aggregate: "count" }),
193+
helper.accessor("m6", { type: "number", aggregate: "sum" }),
194+
helper.accessor("m7", { type: "number", aggregate: "avg" }),
195+
] as const,
196+
getRowId: (row) => row.id,
197+
query: { filters: [], sort: [], rowGroups: [] },
198+
transitionScheduler: scheduler,
199+
transitionClock: () => 0,
200+
});
201+
model.setQuery({
202+
filters: [],
203+
sort: [],
204+
rowGroups: [{ columnId: "team", direction: "asc" }],
205+
});
206+
const candidate = getLocalRowModelActiveTransitionCandidateForTesting(
207+
model,
208+
) as { readonly totalRows: number } | undefined;
209+
expect(candidate).toBeDefined();
210+
let flushed = 0;
211+
while (model.getState().status.kind !== "ready") {
212+
if (!scheduler.flushOne()) break;
213+
flushed += 1;
214+
if (flushed > 100_000) throw new Error("Transition did not settle.");
215+
}
216+
expect(model.getState().status).toEqual({ kind: "ready" });
217+
// `totalRows` is monotonic and folds in the builder's charged seal units
218+
// when the insert phase ends, so the FINAL reading exposes the full
219+
// charge even when sealing itself finishes inside one slice (a
220+
// between-slice max can miss it entirely — verified by mutation).
221+
// Derivation: 4 groups × 2 node units + completion bookkeeping — the
222+
// bound (64) is R-independent; the mixed-fixture test above proves the
223+
// same machinery charges ~R when tree columns are present.
224+
const sealCharge = candidate!.totalRows - rowCount;
225+
expect(sealCharge).toBeGreaterThanOrEqual(0);
226+
expect(sealCharge).toBeLessThanOrEqual(64);
227+
});
228+
169229
for (const aggregateFilteredRows of [false, true]) {
170230
test(`aggregates survive the per-row seal (aggregateFilteredRows: ${aggregateFilteredRows})`, async () => {
171231
// 41 rows (not a multiple of the team count) with per-column-distinct
@@ -238,8 +298,10 @@ describe("grouping-apply cooperative cost (#500)", () => {
238298
}
239299

240300
test("a full 8k-row grouped rebuild completes within the derived slice bound", () => {
241-
// Charged units ≈ R inserts + R row-seals + 2 units per group node +
242-
// completion bookkeeping ≈ 2R + small. With the clock frozen, every
301+
// Charged units ≈ R inserts + R row-seals (the fixture's min/max
302+
// columns keep the ordered tree; its scalar sum/avg columns charge
303+
// nothing — #500 cycle 2) + 2 units per group node + completion
304+
// bookkeeping ≈ 2R + small. With the clock frozen, every
243305
// slice runs exactly the 256-unit cap, so slices ≈ ceil((2 × 8_000 +
244306
// small) / 256) ≈ 63. The bound below allows ~1.5× headroom; the old
245307
// per-(row × column × root) accounting needed ~530 slices here and

0 commit comments

Comments
 (0)