Skip to content

Commit 0e07914

Browse files
committed
perf(studio): stop timeline scroll work when row virtualization is off
The row virtualization stack made the timeline publish a viewport snapshot on every scroll frame and swap `renderClipContent` across every mounted clip at gesture start and settle. Both are windowing concessions, and neither was gated on the flag, so the build users actually run paid for them while mounting all 1,000 clips anyway. Measured on a 3,000-clip project: median scroll step 16.6ms to 76.9ms, p95 17.9ms to 189.4ms, 40 long tasks to 247. Gate both on the row virtualization flag. The scroll path now stops at the door when the flag is off, so `isScrolling` stays false and resize-driven and programmatic syncs still publish through the immediate path. The flag moves into its own module: the scroll-viewport hook needs to read it, and the virtualization hook already imports the viewport snapshot type back, which would have closed an import cycle. Also release the perf fixture lease from the fixture rather than from the test-hook effect. Loading a fixture writes player state, which changed that effect's dependency identities and tore it down on the next frame, so the lease was revoked moments after it was taken and live iframe discovery overwrote the fixture before the gate could measure it. The e2e gate gains a flag-off arm (`test:timeline-default`, 1,000 elements) next to the existing flag-on one. It refuses the 50,000-element combination, verifies from the mounted DOM that the server under test matches the requested flag, and skips the DOM-size budgets for the unvirtualized build rather than relaxing them, so a skipped budget never reads as a passed one. Verified against a live Studio dev server on the fixture project: flag off, before: interactionP95 303.1ms, longest task 194ms, 0/5 runs pass flag off, after: interactionP95 33.6ms, longest task 0ms, 5/5 runs pass flag on, after: interactionP95 33.2ms, 4/5 runs pass, exit 0 The flag-on arm's fourth run reproducibly reports a 55-58ms long task against a 50ms budget. That is the residual tail of the window swap itself, tracked separately and not addressed here.
1 parent df935c5 commit 0e07914

11 files changed

Lines changed: 358 additions & 58 deletions

packages/studio/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@
4949
"build": "vite build && tsup",
5050
"typecheck": "tsc --noEmit",
5151
"test": "vitest run",
52-
"test:timeline-virtualization": "node tests/e2e/timeline-virtualization.mjs",
52+
"test:timeline-virtualization": "TIMELINE_ROW_VIRTUALIZATION=on TIMELINE_ELEMENT_COUNT=50000 node tests/e2e/timeline-virtualization.mjs",
5353
"test:watch": "vitest",
54-
"report:sdk-cutover": "bun src/utils/sdkCutoverPolicy.report.ts"
54+
"report:sdk-cutover": "bun src/utils/sdkCutoverPolicy.report.ts",
55+
"test:timeline-default": "TIMELINE_ROW_VIRTUALIZATION=off TIMELINE_ELEMENT_COUNT=1000 node tests/e2e/timeline-virtualization.mjs"
5556
},
5657
"dependencies": {
5758
"@codemirror/autocomplete": "^6.20.1",

packages/studio/src/hooks/useStudioTestHooks.test.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,11 @@ describe("timeline performance fixture", () => {
129129
unsubscribe();
130130
act(() => root.unmount());
131131
expect(window.__studioTest).toBeUndefined();
132-
expect(hasTimelinePerformanceFixtureLease()).toBe(false);
132+
// The lease outlives the effect on purpose. Loading a fixture changes the
133+
// player state this effect depends on, so the effect tears down right after
134+
// the loader runs; releasing the lease there let live iframe discovery
135+
// overwrite the fixture before anything could measure it.
136+
expect(hasTimelinePerformanceFixtureLease()).toBe(true);
133137
});
134138

135139
it("does not mutate state when the fixture request is invalid", () => {

packages/studio/src/hooks/useStudioTestHooks.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,12 @@ export function useStudioTestHooks({
9999
};
100100
window.__studioTest = api;
101101
return () => {
102-
setTimelinePerformanceFixtureLease(false);
102+
// The lease is deliberately NOT released here. Loading a fixture writes
103+
// player state, which changes this effect's dependency identities and
104+
// tears the effect down on the very next frame. Releasing on teardown
105+
// therefore revoked the lease moments after it was taken, and live iframe
106+
// discovery overwrote the fixture the loader had just installed. The
107+
// lease belongs to the fixture, and a page reload clears it.
103108
// delete, not `= undefined`: an own key holding undefined keeps
104109
// `"__studioTest" in window` true, which defeats feature detection.
105110
delete window.__studioTest;

packages/studio/src/player/components/Timeline.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,10 @@ export const Timeline = memo(function Timeline({
529529
blockedClipRef={blockedClipRef}
530530
suppressClickRef={suppressClickRef}
531531
scrollRef={scrollRef}
532-
renderClipContent={viewport.isScrolling ? undefined : renderClipContent}
532+
// Windowing drops content to mount a row cheaply; unvirtualized it is pure cost.
533+
renderClipContent={
534+
rowVirtualizationActive && viewport.isScrolling ? undefined : renderClipContent
535+
}
533536
renderClipOverlay={renderClipOverlay}
534537
playheadRef={playheadRef}
535538
onDrillDown={onDrillDown}

packages/studio/src/player/components/Timeline.virtualization.test.tsx

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,118 @@ describe("Timeline row virtualization", { timeout: 30_000 }, () => {
295295
usePlayerStore.getState().reset();
296296
});
297297
});
298+
299+
/**
300+
* The flag-off build is the one users get today. It mounts every clip, so the
301+
* scroll-time concessions windowing makes are pure cost there: this block pins
302+
* the timeline to doing no per-frame work at all while a gesture runs.
303+
*/
304+
describe("Timeline without row virtualization", { timeout: 30_000 }, () => {
305+
async function renderUnvirtualizedTimeline() {
306+
vi.stubEnv("VITE_STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED", "0");
307+
vi.resetModules();
308+
const [{ Timeline }, { usePlayerStore }] = await Promise.all([
309+
import("./Timeline"),
310+
import("../store/playerStore"),
311+
]);
312+
usePlayerStore.setState({
313+
duration: 60,
314+
timelineReady: true,
315+
selectedElementId: "clip-0",
316+
elements: Array.from({ length: 40 }, (_, track) => ({
317+
id: `clip-${track}`,
318+
label: `Clip ${track}`,
319+
tag: "div",
320+
start: 0,
321+
duration: 10,
322+
track,
323+
})),
324+
});
325+
326+
const host = document.createElement("div");
327+
document.body.append(host);
328+
const root = createRoot(host);
329+
await act(async () =>
330+
root.render(
331+
React.createElement(Timeline, {
332+
renderClipContent: () => React.createElement("span", { "data-rich-content": true }),
333+
}),
334+
),
335+
);
336+
await act(async () => {});
337+
return {
338+
host,
339+
dispose: () => {
340+
act(() => root.unmount());
341+
usePlayerStore.getState().reset();
342+
vi.stubEnv("VITE_STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED", "1");
343+
vi.resetModules();
344+
},
345+
};
346+
}
347+
348+
it("mounts every clip rather than a window", async () => {
349+
const { host, dispose } = await renderUnvirtualizedTimeline();
350+
try {
351+
expect(host.querySelectorAll("[data-clip]").length).toBe(40);
352+
} finally {
353+
dispose();
354+
}
355+
});
356+
357+
it("keeps clip content mounted across a scroll gesture", async () => {
358+
const { host, dispose } = await renderUnvirtualizedTimeline();
359+
try {
360+
const scroller = host.querySelector<HTMLElement>("[data-timeline-scroll-viewport]");
361+
expect(scroller).not.toBeNull();
362+
const richBefore = host.querySelectorAll("[data-rich-content]").length;
363+
expect(richBefore).toBe(40);
364+
365+
await act(async () => {
366+
scroller?.dispatchEvent(new Event("scroll"));
367+
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
368+
});
369+
370+
expect(host.querySelectorAll("[data-rich-content]").length).toBe(richBefore);
371+
} finally {
372+
dispose();
373+
}
374+
});
375+
376+
it("does not swap clip content back in after the gesture settles", async () => {
377+
const { host, dispose } = await renderUnvirtualizedTimeline();
378+
try {
379+
const scroller = host.querySelector<HTMLElement>("[data-timeline-scroll-viewport]");
380+
const clip = host.querySelector<HTMLElement>('[data-el-id="clip-0"]');
381+
await act(async () => {
382+
scroller?.dispatchEvent(new Event("scroll"));
383+
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
384+
});
385+
await act(async () => {
386+
await new Promise((resolve) => setTimeout(resolve, 150));
387+
});
388+
389+
expect(host.querySelector('[data-el-id="clip-0"]')).toBe(clip);
390+
expect(host.querySelectorAll("[data-rich-content]").length).toBe(40);
391+
} finally {
392+
dispose();
393+
}
394+
});
395+
396+
it("leaves the scroll position alone, so no snapshot round trip happens", async () => {
397+
const { host, dispose } = await renderUnvirtualizedTimeline();
398+
try {
399+
const scroller = host.querySelector<HTMLElement>("[data-timeline-scroll-viewport]");
400+
if (!scroller) throw new Error("Expected a timeline scroll viewport");
401+
scroller.scrollTop = 400;
402+
await act(async () => {
403+
scroller.dispatchEvent(new Event("scroll"));
404+
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
405+
});
406+
407+
expect(scroller.scrollTop).toBe(400);
408+
} finally {
409+
dispose();
410+
}
411+
});
412+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/**
2+
* Row virtualization opt-in. Disabled until horizontal windowing and stable
3+
* gesture lifetime land.
4+
*
5+
* It lives in its own module so the scroll-viewport hook can read it without
6+
* importing the virtualization hook that already imports the viewport snapshot
7+
* type back, which would close an import cycle.
8+
*/
9+
export const STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED =
10+
import.meta.env.DEV === true &&
11+
import.meta.env.VITE_STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED === "1";

packages/studio/src/player/components/useTimelineRowVirtualization.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ import { resolveTimelineFocusIdentity } from "./timelineFocusIdentity";
44
import { getTimelineScrollTopForGeometryChange } from "./timelineViewportGeometry";
55
import type { TimelineRowGeometry } from "./timelineLayout";
66
import type { TimelineScrollViewportSnapshot } from "./useTimelineScrollViewport";
7-
import {
8-
STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED,
9-
useTimelineVirtualRows,
10-
} from "./useTimelineVirtualRows";
7+
import { STUDIO_TIMELINE_ROW_VIRTUALIZATION_ENABLED } from "./timelineRowVirtualizationFlag";
8+
import { useTimelineVirtualRows } from "./useTimelineVirtualRows";
119

1210
interface UseTimelineRowVirtualizationInput {
1311
scrollRef: RefObject<HTMLDivElement | null>;

0 commit comments

Comments
 (0)