perf(studio): stop timeline scroll work when row virtualization is off - #2883
perf(studio): stop timeline scroll work when row virtualization is off#2883miguel-heygen wants to merge 1 commit into
Conversation
1761957 to
79f7704
Compare
9a4b176 to
0f2b4c6
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
79f7704 to
76db7a2
Compare
0f2b4c6 to
4f63c92
Compare
76db7a2 to
46c0d25
Compare
546751c to
d82a1d1
Compare
46c0d25 to
383514d
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
🟢 Exact delta clean at d82a1d126397da44863fb3546b9fb2f70903f617.
Reviewed only 383514d4..d82a1d12.
The mechanism is internally consistent:
- With row virtualization off, scroll-frame publication stops before state mutation, so ordinary scrolling no longer re-renders every mounted clip.
- Resize-driven and programmatic synchronization still take the immediate publication path.
- The flag is centralized and remains dev-gated; production behavior cannot be enabled by the env value alone.
Timeline.tsxonly strips rich clip content while virtualization is both enabled and actively scrolling.- The fixture lease intentionally survives the test-hook effect teardown, which is necessary because loading the fixture changes the dependencies that recreate that hook.
- Tests cover both module-load flag states and distinguish scroll updates from immediate/settled synchronization.
One operational note, not a blocker: after a performance fixture is loaded, discovery stays leased until a page reload. That matches the comment and the dedicated test route, but future reusable test tooling should expose an explicit fixture teardown rather than relying on navigation lifecycle.
git diff --check passes. The #2703 findings remain downstack blockers for the stack; I found no new blocker in #2883 itself.
— Magi
383514d to
9b5b8b2
Compare
b97d8c7 to
6753340
Compare
b8c4dd9 to
2922d03
Compare
6753340 to
0ab321b
Compare
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.
2922d03 to
027a269
Compare
0ab321b to
18e7d3f
Compare

What
Gates the two per-scroll-frame costs the row virtualization stack introduced on the row virtualization flag, so the build that ships today does none of them:
useTimelineScrollViewportpublished a viewport snapshot on every scroll frame. With the flag off it now returns at the door, soisScrollingstays false.TimelineswappedrenderClipContenttoundefinedacross every mounted clip at gesture start and back at settle. That swap is now conditioned on row virtualization actually being active.Two supporting changes:
The e2e gate gains a flag-off arm,
bun run test:timeline-default(1,000 elements), next to the existing flag-ontest:timeline-virtualization(50,000). It refuses the unvirtualized 50,000-element combination rather than hanging on it, 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.Why
Both behaviours are windowing concessions: dropping clip content keeps a row cheap while it is being mounted and unmounted, and the per-frame snapshot is what tells the virtualizer where the window is. Neither was gated on the flag, and the flag is off by default, so the default build paid for them while still mounting every clip.
Measured on a 3,000-clip project, merge base versus stack tip with the flag off: median scroll step 16.6ms to 76.9ms, p95 17.9ms to 189.4ms, long tasks 40 to 247, total blocking 6,073ms to 28,163ms. DOM size was unchanged, because with the flag off nothing is being windowed.
The existing gate never caught this: it takes
STUDIO_URLfrom the environment and never sets the flag itself, so it only ever measured the enabled build.How
The scroll-viewport gate is one condition at the top of
syncScrollViewport. Resize-driven and programmatic syncs arrive through the immediate, non-scrolling path and still publish, soResizeObserversizing and the follow-playhead scrolls are unaffected.Suppressing the publication at the source was chosen over memoizing the downstream consumers: the snapshot has one consumer that cares about per-frame precision, and not producing it is a smaller change than making everything below it resilient to it.
Timeline.tsxconditions therenderClipContentswap onrowVirtualizationActiverather than on the module constant, so a virtualization-enabled build with a zero-sized viewport also keeps its content.Test plan
Both arms of the e2e gate were run against a live Studio dev server on the documented fixture project, on the same machine, back to back:
The flag-off arm was run red against the unfixed tree first. A gate that has never gone red has not been shown to gate anything.
Timeline.virtualization.test.tsxgains an unvirtualized block; with both gates reverted its content-retention case fails 40 mounted contents to 0, so it is a real regression test rather than a passing assertion. The other three cases in that block are guards and pass either way.useTimelineScrollViewport.test.tsxwas restructured to mount the hook under both flag states and now covers scroll suppression, the absent frame and timer, and the resize and programmatic paths still publishing.Studio suite: 3,179 passed. One pre-existing failure in
vite.browser.test.ts(generateThumbnailbrowser-launch retry) reproduces identically on the unmodified stack tip and is unrelated to this change.Not covered here: the flag-on arm reproducibly reports a 55-58ms long task on its fourth measured run against a 50ms budget. That is the residual tail of the window swap itself, it predates this PR, and it is deliberately left for separate work. Wiring either gate arm into CI is also out of scope.