Repository navigation
perf(studio): a long timeline no longer lays itself out on every render while it settles - #6855
Conversation
…er while it settles
Edit accuracy: accurate 271 (base branch 271), smooth 89 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (3)
|
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 3 blocker · 0 important · 0 nits · head e71656bb
I found no problem with the code itself. Locally, both new tests fail on main (9 and 6 reads, matching the body), pass here, and src/player passes (2433 tests). Skipping the read when rows are unchanged gives the same result as before: getTimelineScrollTopForGeometryChange returns scrollTop unchanged when keys, offsets and top padding all match. The blockers are all failing CI checks:
- CI → Format:
oxfmt --checkflagspackages/studio/src/player/components/useTimelineRowVirtualization.test.tsx. Runoxfmton it. - Comments ratchet: comment share went up in
timelineViewportGeometry.ts(0/55 → 2/67) anduseSettledScrollLeft.ts(3/39 → 5/44). Cut the added lines or move what they say into names. - PR captures: the body needs
## Before/## Aftercaptures. The source diff is over the 20-line limit, so## No visible changewon't pass either unless the gate's rules allow it.
Scope
Read end to end: all 5 files, plus the consumers of useSettledScrollLeft (TimelineRuler, TimelinePlayheadLayer) and how setScrollRef gets attached. Both consumers render inside the viewport div, so the ref is set before the subscribe effect runs. No prior reviews.
What
A long timeline no longer lays itself out on every React render while its project settles. Two reads forced a synchronous layout of the whole timeline on every commit:
useSettledScrollLeft) readscrollLeftinside itsuseSyncExternalStoresnapshot, which React calls on every render and after every commit. The settled value is now kept when a scroll settles (and when the hook subscribes) and the snapshot returns it.useTimelineRowVirtualization) readscrollTopwheneverrowGeometrychanged identity, even when the new geometry put every row at the same top. It now skips the read when the rows' keys, offsets and top padding are unchanged (the existingsameData); rows are tracks, so the comparison is a handful of numbers.Why
In a renderer CPU profile of a 600-clip, 23-minute project playing right after it opens, these two reads were the top self-time functions (0.4 to 0.9 s each in 6 s of play), each a forced layout of every clip. While the project settles (each clip's waveform arriving is a commit), playback dropped to a few frames a second.
Related work
None.
How
useSettledScrollLeft.ts: asettledRefset on subscribe (with anonChange()so the first render's value is replaced) and when a scroll settles; the snapshot returns it.useTimelineRowVirtualization.ts: the geometry branch runs only whensameDatasays the rows' keys, offsets or top padding changed (a geometry carries per-instance closures, so the whole objects never compare equal).No visible change
The timeline looks and scrolls the same; it stops forcing layouts that changed nothing on screen.
Test plan
fails without the fix: with the two source files restored to main, "reads scrollLeft when a scroll settles, never on a render" (useSettledScrollLeft.test.tsx) reads 9 times on renders, and "reads no scroll position for a new geometry that places every row where it was" (useTimelineRowVirtualization.test.tsx) reads 6 times; both read 0 with the fix.
scrollTopand keeps the view anchored.