Skip to content

Commit 1c66315

Browse files
Brian Loveclaude
authored andcommitted
perf(row-model): the adopted identity sweep answers verdict and keys from one cache read
Under adoption (every warm keystroke — a filter-only set-query above the sync gate), carryRecord paid two evaluation-cache WeakMap gets on the same key per survivor: filterVerdict looked the entry up, failed the verdictPlan guard by design, and discarded it; fillSortKeysFromPrevious then looked the same key up in the same (adopted) map to hit its early return, behind two instanceof revalidations. ~500k redundant lookups per 50k keystroke settle. The fused sortKeysIfPasses answers both questions from ONE get: the verdict is still recomputed under the new plan (the memo guard still fails for adopted entries), the keys are the same unguarded sortKeys read the fill's early return performed, and a lineage-miss falls back to the fill's accessor arm with a keys-only seed. The un-adopted lane is untouched. Certified by a new evaluationCacheLookups work counter (counts the identity-carry path's cache reads only): the adopted-lane pin asserts ONE lookup per swept row (read 10_050 for 10_000 sweeps + 50 survivors before the fusion; mutation-verified — disabling the fused arm fails it at exactly 10_050), and the un-adopted control pins 10_100 so a fusion that merely stopped counting cannot pass. Verdict correctness is pinned by the flipped visible set (the new filter keeps exactly the 50 rows the old one rejected). Zero public report drift (pnpm api clean). row-model 719, grid-core 169, core 7, renderer-dom 168, react 1570 — all green. The ms payoff is unmeasured (load 18 machine) and not claimed; it rides with the owed #490 confirmation round. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c2ca7ba commit 1c66315

7 files changed

Lines changed: 273 additions & 3 deletions

File tree

packages/row-model/src/__tests__/filter-fast-path.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ function testInstrumentation(): LocalRowModelInstrumentation {
180180
bulkOrderVerificationsSkipped: 0,
181181
evaluationCacheAdoptions: 0,
182182
slotChunksTouched: 0,
183+
evaluationCacheLookups: 0,
183184
sortKeyCarries: 0,
184185
sortKeyEvaluations: 0,
185186
schedulerSliceDurations: [],

packages/row-model/src/__tests__/order-statistic-tree.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,7 @@ describe("bulk-build byId routing", () => {
10281028
bulkOrderVerificationsSkipped: 0,
10291029
evaluationCacheAdoptions: 0,
10301030
slotChunksTouched: 0,
1031+
evaluationCacheLookups: 0,
10311032
sortKeyCarries: 0,
10321033
sortKeyEvaluations: 0,
10331034
snapshotOutputRowsRead: 0,

packages/row-model/src/__tests__/sort-fast-path.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ function testInstrumentation(): LocalRowModelInstrumentation {
189189
bulkOrderVerificationsSkipped: 0,
190190
evaluationCacheAdoptions: 0,
191191
slotChunksTouched: 0,
192+
evaluationCacheLookups: 0,
192193
sortKeyCarries: 0,
193194
sortKeyEvaluations: 0,
194195
schedulerSliceDurations: [],

packages/row-model/src/__tests__/work.test.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,118 @@ describe("instrumented local row-model work", () => {
234234
},
235235
);
236236

237+
test(
238+
"carries a flat filter-only set-query with ONE evaluation-cache lookup per swept row",
239+
{ timeout: 30_000 },
240+
async () => {
241+
const scheduled: Array<() => void> = [];
242+
const instrumented = createInstrumentedLocalRowModel({
243+
rows: rows(10_000),
244+
columns,
245+
query: {
246+
filters: [{ columnId: "filterValue", operator: "gte", value: 1_000 }],
247+
sort: [{ columnId: "score", direction: "asc" }],
248+
rowGroups: [],
249+
},
250+
transitionScheduler: {
251+
schedule(task) {
252+
scheduled.push(task);
253+
return () => undefined;
254+
},
255+
},
256+
transitionClock: () => 0,
257+
// Test-forcing direction of #488's gate: a filter-only change at 10k
258+
// would take the synchronous rebuild; production trips this lane at
259+
// >15k resident rows (the warm-keystroke path at 50k).
260+
ɵfilterFastPathRowLimit: 0,
261+
});
262+
263+
instrumented.diagnostics.resetWork();
264+
// Filter changes ALONE: `isFilterOnlyChange` holds, so the identity
265+
// lane adopts the previous plan's evaluation cache wholesale and every
266+
// carried record's entry is already in the shared map.
267+
const transition = instrumented.model.setQuery({
268+
filters: [{ columnId: "filterValue", operator: "lte", value: 1_500 }],
269+
sort: [{ columnId: "score", direction: "asc" }],
270+
rowGroups: [],
271+
});
272+
while (scheduled.length > 0) scheduled.shift()!();
273+
await transition.finished;
274+
275+
const work = instrumented.diagnostics.read().work;
276+
expect(work.evaluationCacheAdoptions).toBe(1);
277+
expect(work.transitionRows).toBe(10_000);
278+
expect(work.rowsEvaluated).toBe(0);
279+
// The adopted-lane budget: ONE evaluation-cache read per swept row,
280+
// total. Before the fused reader the sweep paid two on every SURVIVOR
281+
// (`filterVerdict` looked the entry up and discarded it, then
282+
// `fillSortKeysFromPrevious` looked the same key up again to hit its
283+
// early return) — this read 10_050 with 50 survivors, and at the
284+
// 50k/5-commit warm-keystroke scale that was ~500k redundant lookups.
285+
expect(work.evaluationCacheLookups).toBe(10_000);
286+
// Keys came from the adopted entries, not from accessor re-runs, and
287+
// not from the per-row carry fill.
288+
expect(work.sortKeyCarries).toBe(0);
289+
expect(work.sortKeyEvaluations).toBe(0);
290+
// The verdict itself must still be recomputed under the NEW plan (the
291+
// adopted entries memo the OLD plan's verdict): the new filter keeps
292+
// exactly the 50 rows the old one rejected, in the unchanged sort.
293+
const snapshot = instrumented.model.getState().snapshot;
294+
expect(snapshot.visibleRowCount).toBe(50);
295+
expect(snapshot.range(0, 1)[0]).toMatchObject({
296+
kind: "data",
297+
rowId: 900,
298+
});
299+
instrumented.model.dispose();
300+
},
301+
);
302+
303+
test(
304+
"counts the un-adopted carry fill's evaluation-cache lookups (counter control)",
305+
{ timeout: 30_000 },
306+
async () => {
307+
const scheduled: Array<() => void> = [];
308+
const instrumented = createInstrumentedLocalRowModel({
309+
rows: rows(10_000),
310+
columns,
311+
query: {
312+
filters: [{ columnId: "filterValue", operator: "gte", value: 1_000 }],
313+
sort: [{ columnId: "score", direction: "asc" }],
314+
rowGroups: [],
315+
},
316+
transitionScheduler: {
317+
schedule(task) {
318+
scheduled.push(task);
319+
return () => undefined;
320+
},
321+
},
322+
transitionClock: () => 0,
323+
});
324+
325+
instrumented.diagnostics.resetWork();
326+
// Filter AND sort change: not a filter-only change, so no adoption —
327+
// the identity lane pays the verdict lookup per swept row plus the
328+
// carry fill's two reads (fresh-cache miss, then the previous plan's
329+
// store) per SURVIVOR. This pin is the control proving the counter
330+
// observes every read site: if the fused reader ever "wins" by simply
331+
// not counting, this expectation catches it.
332+
const transition = instrumented.model.setQuery({
333+
filters: [{ columnId: "filterValue", operator: "lte", value: 1_500 }],
334+
sort: [{ columnId: "score", direction: "desc" }],
335+
rowGroups: [],
336+
});
337+
while (scheduled.length > 0) scheduled.shift()!();
338+
await transition.finished;
339+
340+
const work = instrumented.diagnostics.read().work;
341+
expect(work.evaluationCacheAdoptions).toBe(0);
342+
expect(work.transitionRows).toBe(10_000);
343+
expect(work.evaluationCacheLookups).toBe(10_100);
344+
expect(work.sortKeyCarries).toBe(50);
345+
instrumented.model.dispose();
346+
},
347+
);
348+
237349
test(
238350
"builds a flat set-derivations candidate without persistent per-row path copying",
239351
{ timeout: 30_000 },

packages/row-model/src/compiled-query.ts

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,18 @@ export interface SortKeyFillInstrumentation {
103103
readonly work: {
104104
sortKeyCarries: number;
105105
sortKeyEvaluations: number;
106+
evaluationCacheLookups: number;
107+
};
108+
}
109+
110+
/**
111+
* Structural slice consumed by `filterVerdict`'s lookup counting — the
112+
* verdict-only callers that thread instrumentation (the flat identity-carry
113+
* sweep) count their cache reads; everyone else passes nothing.
114+
*/
115+
export interface EvaluationCacheLookupInstrumentation {
116+
readonly work: {
117+
evaluationCacheLookups: number;
106118
};
107119
}
108120

@@ -1999,11 +2011,14 @@ class CompiledQueryPlan<TColumns>
19992011
static filterVerdict<TColumns, TRowId extends PretableRowId>(
20002012
plan: unknown,
20012013
input: CompiledRowInput<RowForColumns<TColumns>, TRowId>,
2014+
instrumentation?: EvaluationCacheLookupInstrumentation,
20022015
): boolean {
20032016
if (!(plan instanceof CompiledQueryPlan)) {
20042017
throw new TypeError("Filter verdicts require a compiled query plan.");
20052018
}
20062019
const compiled = plan as CompiledQueryPlan<TColumns>;
2020+
if (instrumentation !== undefined)
2021+
instrumentation.work.evaluationCacheLookups += 1;
20072022
const cached = compiled.#evaluationCache.get(input.row);
20082023
if (
20092024
cached !== undefined &&
@@ -2024,6 +2039,85 @@ class CompiledQueryPlan<TColumns>
20242039
);
20252040
}
20262041

2042+
/**
2043+
* Fused verdict + sort-key resolution for the ADOPTED identity-carry
2044+
* sweep: ONE evaluation-cache read answers both questions
2045+
* `filterVerdict` + `fillSortKeysFromPrevious` used to pay two reads for
2046+
* (the verdict lookup discarded its entry, then the fill looked the same
2047+
* key up again to hit its early return — one redundant get per survivor).
2048+
*
2049+
* Returns the row's sort keys when the row passes THIS plan's filters and
2050+
* `undefined` when it does not; a rejected row's keys are never read, so
2051+
* key resolution rides the verdict's single lookup for free.
2052+
*
2053+
* Precondition (CALLER-OWNED, exactly `adoptEvaluationCache`'s): this
2054+
* plan's cache was adopted from the plan whose lineage evaluated
2055+
* `input.row` — a filter-only change — so the cached `sortKeys` are the
2056+
* ones this plan's accessors would produce (same orderings, same
2057+
* accessors; see the adoption proof). The verdict is still recomputed
2058+
* under this plan whenever the memo's `verdictPlan` is not this plan —
2059+
* adopted entries always miss that guard, exactly as before the fusion.
2060+
*/
2061+
static sortKeysIfPasses<TColumns, TRowId extends PretableRowId>(
2062+
plan: unknown,
2063+
input: CompiledRowInput<RowForColumns<TColumns>, TRowId>,
2064+
instrumentation?: SortKeyFillInstrumentation,
2065+
): readonly CompiledSortKey<TColumns>[] | undefined {
2066+
if (!(plan instanceof CompiledQueryPlan)) {
2067+
throw new TypeError("Filter verdicts require a compiled query plan.");
2068+
}
2069+
const compiled = plan as CompiledQueryPlan<TColumns>;
2070+
if (instrumentation !== undefined)
2071+
instrumentation.work.evaluationCacheLookups += 1;
2072+
const cached = compiled.#evaluationCache.get(input.row);
2073+
const passes =
2074+
cached !== undefined &&
2075+
cached.metadata !== undefined &&
2076+
cached.filterPasses !== undefined &&
2077+
cached.verdictPlan === compiled &&
2078+
Object.is(cached.rowId, input.rowId) &&
2079+
cached.sourceOrder === input.sourceOrder
2080+
? cached.filterPasses
2081+
: compiled.#filterVerdict((columnId) =>
2082+
compiled.#readColumnValue(
2083+
compiled.#byId.get(columnId)!,
2084+
input.row,
2085+
input.rowId,
2086+
),
2087+
);
2088+
if (!passes) return undefined;
2089+
if (cached !== undefined) {
2090+
// The same UNGUARDED read the carry fill's early return performs:
2091+
// sort keys depend only on the row object and the (unchanged) sort
2092+
// columns, so an entry from anywhere in the adopted lineage answers.
2093+
return cached.sortKeys as readonly CompiledSortKey<TColumns>[];
2094+
}
2095+
// A row the adopted lineage never evaluated: resolve keys by accessor
2096+
// and seed a keys-only entry, mirroring the carry fill's miss arm with
2097+
// nothing to carry (`metadata` absent, so a later `evaluate` upgrades).
2098+
const sortKeys = Object.freeze(
2099+
compiled.#runtimeQuery.sort.map((entry) => {
2100+
const value = compiled.#readColumnValue(
2101+
compiled.#byId.get(entry.columnId)!,
2102+
input.row,
2103+
input.rowId,
2104+
);
2105+
if (instrumentation !== undefined)
2106+
instrumentation.work.sortKeyEvaluations += 1;
2107+
return Object.freeze({ columnId: entry.columnId, value });
2108+
}),
2109+
) as readonly CompiledSortKey<TColumns>[];
2110+
compiled.#evaluationCache.set(input.row, {
2111+
rowId: input.rowId,
2112+
sourceOrder: input.sourceOrder,
2113+
metadata: undefined,
2114+
filterPasses: undefined,
2115+
verdictPlan: undefined,
2116+
sortKeys,
2117+
});
2118+
return sortKeys;
2119+
}
2120+
20272121
/*
20282122
* The single comparison loop behind `compareRecordRows`: per-ordering
20292123
* `compareValues` over store-resolved keys, then the `sourceOrder`
@@ -2159,11 +2253,15 @@ class CompiledQueryPlan<TColumns>
21592253
}
21602254
const next = nextPlan as CompiledQueryPlan<TColumns>;
21612255
const previous = previousPlan as CompiledQueryPlan<TColumns>;
2256+
if (instrumentation !== undefined)
2257+
instrumentation.work.evaluationCacheLookups += 1;
21622258
const existing = next.#evaluationCache.get(input.row);
21632259
if (existing !== undefined) {
21642260
return existing.sortKeys as readonly CompiledSortKey<TColumns>[];
21652261
}
21662262

2263+
if (instrumentation !== undefined)
2264+
instrumentation.work.evaluationCacheLookups += 1;
21672265
const carried = previous.#evaluationCache.get(input.row)?.sortKeys as
21682266
readonly CompiledSortKey<TColumns>[] | undefined;
21692267
const sortKeys = Object.freeze(
@@ -2538,8 +2636,33 @@ export function adoptEvaluationCache<TColumns>(
25382636
export function filterVerdict<TColumns, TRowId extends PretableRowId>(
25392637
plan: CompiledQuery<TColumns>,
25402638
input: CompiledRowInput<RowForColumns<TColumns>, TRowId>,
2639+
instrumentation?: EvaluationCacheLookupInstrumentation,
25412640
): boolean {
2542-
return CompiledQueryPlan.filterVerdict<TColumns, TRowId>(plan, input);
2641+
return CompiledQueryPlan.filterVerdict<TColumns, TRowId>(
2642+
plan,
2643+
input,
2644+
instrumentation,
2645+
);
2646+
}
2647+
2648+
/**
2649+
* Fused verdict + sort-key resolution under an ADOPTED evaluation cache —
2650+
* one cache read instead of `filterVerdict` + `fillSortKeysFromPrevious`'s
2651+
* two. Returns the keys when the row passes `plan`'s filters, `undefined`
2652+
* when it does not. Valid ONLY after `adoptEvaluationCache(plan, previous)`
2653+
* for the plan lineage that evaluated the row (a filter-only change); see
2654+
* `CompiledQueryPlan.sortKeysIfPasses` for the semantics proof.
2655+
*/
2656+
export function sortKeysIfPasses<TColumns, TRowId extends PretableRowId>(
2657+
plan: CompiledQuery<TColumns>,
2658+
input: CompiledRowInput<RowForColumns<TColumns>, TRowId>,
2659+
instrumentation?: SortKeyFillInstrumentation,
2660+
): readonly CompiledSortKey<TColumns>[] | undefined {
2661+
return CompiledQueryPlan.sortKeysIfPasses<TColumns, TRowId>(
2662+
plan,
2663+
input,
2664+
instrumentation,
2665+
);
25432666
}
25442667

25452668
export function compileQuery<const TColumns>(

packages/row-model/src/diagnostics.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ export interface LocalRowModelWorkDiagnostics {
6161
* plus table copies per commit rather than per-row.
6262
*/
6363
readonly slotChunksTouched: number;
64+
/**
65+
* Evaluation-cache WeakMap reads issued by the flat identity-carry path
66+
* (the cooperative sweep's verdict check and sort-key resolution,
67+
* including replay inserts). The adopted-lane budget is ONE read per
68+
* swept row; `work.test.ts` pins it. Other verdict/evaluate callers do
69+
* not count here.
70+
*/
71+
readonly evaluationCacheLookups: number;
6472
/** Sort-key entries carried from a previous plan's store, per (row, column). */
6573
readonly sortKeyCarries: number;
6674
/** Sort-key entries produced by running an accessor, per (row, column). */
@@ -140,6 +148,7 @@ function newInstrumentation(): LocalRowModelInstrumentation {
140148
bulkOrderVerificationsSkipped: 0,
141149
evaluationCacheAdoptions: 0,
142150
slotChunksTouched: 0,
151+
evaluationCacheLookups: 0,
143152
sortKeyCarries: 0,
144153
sortKeyEvaluations: 0,
145154
snapshotOutputRowsRead: 0,
@@ -171,6 +180,7 @@ function resetWork(instrumentation: LocalRowModelInstrumentation): void {
171180
"bulkOrderVerificationsSkipped",
172181
"evaluationCacheAdoptions",
173182
"slotChunksTouched",
183+
"evaluationCacheLookups",
174184
"sortKeyCarries",
175185
"sortKeyEvaluations",
176186
"snapshotOutputRowsRead",

packages/row-model/src/flat-cooperative-candidate.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
adoptEvaluationCache,
33
fillSortKeysFromPrevious,
4+
sortKeysIfPasses,
45
filterVerdict,
56
isFilterOnlyChange,
67
type CompiledQuery,
@@ -92,7 +93,11 @@ export function createFlatCooperativeCandidate<
9293
* per-row `fillSortKeysFromPrevious` below carries what it can instead.
9394
* Pure perf lever either way: nothing below depends on the adoption.
9495
*/
95-
if (isFilterOnlyChange(options.captured.queryPlan, options.queryPlan)) {
96+
const adopted = isFilterOnlyChange(
97+
options.captured.queryPlan,
98+
options.queryPlan,
99+
);
100+
if (adopted) {
96101
adoptEvaluationCache(options.queryPlan, options.captured.queryPlan);
97102
if (instrumentation !== undefined) {
98103
instrumentation.work.evaluationCacheAdoptions += 1;
@@ -253,7 +258,24 @@ export function createFlatCooperativeCandidate<
253258
// on this lane — that zero is the dense claim `work.test.ts` pins.
254259
instrumentation.work.transitionRows += 1;
255260
}
256-
if (filterVerdict(state.queryPlan, record as never)) {
261+
if (adopted) {
262+
// Fused: the adopted cache answers verdict AND keys from ONE read —
263+
// `work.test.ts` pins the one-lookup-per-swept-row budget, and the
264+
// un-fused arm below stays pinned as the counter's control.
265+
const keys = sortKeysIfPasses(
266+
state.queryPlan,
267+
record as never,
268+
instrumentation,
269+
) as readonly CompiledSortKey<TColumns>[] | undefined;
270+
if (keys !== undefined) {
271+
state.transientFlatRows!.insertOrReplace(
272+
Object.freeze({ record, keys }),
273+
);
274+
setMembershipBit(state.membership!, record.slot);
275+
}
276+
return;
277+
}
278+
if (filterVerdict(state.queryPlan, record as never, instrumentation)) {
257279
const keys = fillSortKeysFromPrevious(
258280
state.queryPlan,
259281
state.captured.queryPlan,

0 commit comments

Comments
 (0)