diff --git a/apps/bench/src/__tests__/bench-runtime.test.ts b/apps/bench/src/__tests__/bench-runtime.test.ts index f4ed9742..923c70a1 100644 --- a/apps/bench/src/__tests__/bench-runtime.test.ts +++ b/apps/bench/src/__tests__/bench-runtime.test.ts @@ -464,6 +464,106 @@ describe("bench runtime", () => { } }); + test("main-thread long tasks are measured from the trigger, not before it", async () => { + // #458: the interaction path measured no main-thread blocking at all, so a + // synchronous engine that blocks for its whole sort and a cooperative one + // that never blocks reported indistinguishable results. The observer must + // attach AT the trigger — a long task during the pre-trigger quiet wait is + // mount tail, not the interaction's. + const { layoutRow, root, viewport } = createDataUpdateHarness(); + const rows = [ + ...viewport.querySelectorAll("[data-pretable-row]"), + ]; + const pending: { + frames: number; + apply: () => void; + onFrame?: (frame: number) => void; + } = { + frames: 3, + apply: () => { + for (const [index, row] of rows.entries()) { + layoutRow(row, index - 1); + } + }, + }; + const restore = installFrameStub(pending); + + // jsdom has no PerformanceObserver; the stub records the callbacks the + // harness registers for `longtask` so the test can play entries into them + // at controlled moments. + const longTaskCallbacks: Array<(list: unknown) => void> = []; + const previousObserver = (globalThis as { PerformanceObserver?: unknown }) + .PerformanceObserver; + class StubObserver { + static supportedEntryTypes = ["longtask"]; + #callback: (list: unknown) => void; + constructor(callback: (list: unknown) => void) { + this.#callback = callback; + } + observe() { + longTaskCallbacks.push(this.#callback); + } + disconnect() { + const index = longTaskCallbacks.indexOf(this.#callback); + if (index >= 0) longTaskCallbacks.splice(index, 1); + } + } + Object.defineProperty(globalThis, "PerformanceObserver", { + configurable: true, + value: StubObserver, + }); + const emit = (duration: number) => { + for (const callback of [...longTaskCallbacks]) { + callback({ getEntries: () => [{ duration }] }); + } + }; + + // Emitted DURING the run's own pre-trigger quiet wait (frame 1 is consumed + // by waitForQuietSurface), not merely before the call — an observer + // attached at function entry instead of at the trigger is listening by + // then, and this is the emission that catches it. + pending.onFrame = (frame: number) => { + if (frame === 1) emit(120); + }; + + try { + const result = await measureBenchInteractionRun( + root, + "pretable", + "filter-metadata", + { + focusedRowId: null, + resultRowCount: 3, + selectedRowId: null, + }, + () => ({ + focusedRowId: null, + resultRowCount: 3, + selectedRowId: null, + }), + () => { + // The trigger IS the interaction: a synchronous engine blocks right + // here. Two tasks so count and total are distinguishable. + emit(80); + emit(35); + pending.frames = 2; + }, + ); + + expect(result.status).toBe("completed"); + expect(result.metrics.post_interaction_long_tasks_count).toBe(2); + expect(result.metrics.post_interaction_long_tasks_ms).toBe(115); + // The run must also stop listening when it finishes. + expect(longTaskCallbacks).toHaveLength(0); + } finally { + Object.defineProperty(globalThis, "PerformanceObserver", { + configurable: true, + value: previousObserver, + }); + restore(); + } + }); + test("refuses to complete an interaction whose row count never reached the plan", async () => { const { layoutRow, root, viewport } = createDataUpdateHarness(); const rows = [ diff --git a/apps/bench/src/bench-runtime.ts b/apps/bench/src/bench-runtime.ts index 85f788b3..92ef3de5 100644 --- a/apps/bench/src/bench-runtime.ts +++ b/apps/bench/src/bench-runtime.ts @@ -758,6 +758,17 @@ async function measureRowSetChange( const scrollTopBefore = viewport.scrollTop; const startTimestamp = await waitForAnimationFrame(); + // Attached AT the trigger, not at function entry: a long task during the + // pre-trigger quiet wait is mount tail, and charging it to the interaction + // would poison the one metric that can show a synchronous engine blocking + // (#458). The trigger itself IS the interaction — a blocking sort runs + // inside it — so the observer must be live before it is called. Push-based, + // so unlike the height-error walk this adds no per-frame DOM work (#455). + const interactionLongTaskDurations: number[] = []; + const interactionLongTaskObserver = createLongTaskObserver( + interactionLongTaskDurations, + ); + performance.mark("pretable.interaction.start"); trigger(); @@ -879,6 +890,8 @@ async function measureRowSetChange( settledFrame = stalledFrame; } + interactionLongTaskObserver?.disconnect(); + if (firstChangedAt === null || settledAt === null) { return { status: "partial", @@ -927,6 +940,19 @@ async function measureRowSetChange( interaction_latency_ms: firstChangedAt - startTimestamp, settle_duration_ms: settledAt - firstChangedAt, post_interaction_blank_gap_frames: blankGapFrames, + // Zero when nothing observable blocked, absent when the host cannot + // observe long tasks at all — the same absent-vs-zero honesty rule as + // the row-height error above. + ...(interactionLongTaskObserver !== null + ? { + post_interaction_long_tasks_count: + interactionLongTaskDurations.length, + post_interaction_long_tasks_ms: interactionLongTaskDurations.reduce( + (total, duration) => total + duration, + 0, + ), + } + : {}), post_interaction_anchor_shift_px: percentile(anchorShifts, 0.95), ...summarizeRowHeightError(rowHeightError, { p95: "post_interaction_row_height_error_p95_px", diff --git a/packages/bench-runner/src/index.ts b/packages/bench-runner/src/index.ts index 4bce8ae5..828e8c44 100644 --- a/packages/bench-runner/src/index.ts +++ b/packages/bench-runner/src/index.ts @@ -33,6 +33,8 @@ export type BenchMetricId = | "interaction_latency_ms" | "settle_duration_ms" | "post_interaction_blank_gap_frames" + | "post_interaction_long_tasks_count" + | "post_interaction_long_tasks_ms" | "post_interaction_anchor_shift_px" | "post_interaction_row_height_error_p95_px" /** @see row_height_error_measurable_rows */