perf(studio): scrubbing a large composition no longer stalls the editor - #3842
Open
miguel-heygen wants to merge 4 commits into
Open
perf(studio): scrubbing a large composition no longer stalls the editor#3842miguel-heygen wants to merge 4 commits into
miguel-heygen wants to merge 4 commits into
Conversation
… per element The dashed off-canvas markers are rebuilt whenever anything in the preview changes, which during playback or a scrub is several times a second. Most of what a rebuild asked about an element was really a question about its ANCESTORS: does each node up to the root render, what does each contribute to the composed transform, which node is the source-file boundary. Two siblings share their whole chain, so a preview of a thousand elements asked the platform the same questions about the same ancestors a thousand times over. A rebuild now threads one measure pass through the walk, and each node answers once. On a 1689-element carousel that takes computed-style reads from 11,159 to 2,526 per rebuild and the rebuild itself from 59 to 12-18 ms per seek, with the rendered marker set byte-identical. The pass is deliberately not a cache. A cache would have to say what could have changed since last time, and for a measurement the answer is anything: an image finishing decode, a font swapping in, a transition frame, a container query, an inserted stylesheet rule all move an element's box with nothing written to the DOM and no record to invalidate on. A pass lives inside one synchronous measurement that only reads, so nothing can move under it, and it is dropped when the measurement ends. Every rebuild still measures every element. Layout reads are untouched on purpose: each element still takes its own client rect every rebuild, because that is the read that cannot be shared and must not be remembered.
Every other render test in that file passes RENDER_TIMEOUT_MS; this it.each was left on vitest's 5s default and times out when the whole suite runs on a loaded machine. Unrelated to any behaviour: it passes on either side of the change when the machine is quiet, and fails in a full-suite run on either side when it is not.
…ld fixtures Three follow-ups from review of the measure pass, none of them behaviour: The source boundary was memoized against the element that asked for it, so two siblings still walked their whole chain separately and only the answer was reused. It now memoizes every node on the way up, like the other two walks: an element's boundary IS its parent's unless the element is one itself. `isElementVisibleThroughAncestors` returns what it always returned, but it now resolves top-down, so the set of nodes it takes a style read for is different (smaller for a subtree hidden near the root). Three callers use it with no memo, so that is written down next to it rather than left to be discovered. The three rebuild fixtures in the indicator tests each carried their own copy of the iframe/overlay/layout-stub scaffolding, which is four clone groups and 126 duplicated lines. One `mountPreview` helper now serves all of them and the pre-existing selector-index fixture too.
miguel-heygen
force-pushed
the
perf-offcanvas-geometry-cache
branch
from
September 10, 2026 12:54
ed82377 to
ed12ec7
Compare
…-derived input The existing no-mutation-record test varies an element's own box, and the pass never memoizes a box — so hoisting the pass to module scope, which is exactly the mistake that would reintroduce cross-rebuild staleness, left every test green. This one fades a wrapper between two rebuilds and asserts the marker under it goes away. Visibility through ancestors IS memoized, so the pass surviving the rebuild serves the stale answer: with `createOverlayMeasurePass()` hoisted the suite exits 1 on this test alone, and exits 0 at head.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes for someone using Studio
Scrubbing the playhead through a large composition made the editor sluggish, and the bigger the composition the worse it got. The dashed off-canvas markers were re-measured from scratch several times a second, and almost all of that work was the same question asked about the same ancestors once for every element underneath them.
The markers are unchanged, and every rebuild still measures every element. It just stops asking the platform the same thing a thousand times.
Mechanism
Studio draws a dashed outline for every element sitting outside the composition frame. Those outlines are rebuilt on an animation-frame loop, throttled to 100 ms, whenever a
MutationObserveron the preview subtree reports a change — and an animation writes inline style every frame, so during a scrub the rebuild runs at its full 10 Hz for as long as anything moves.For each element the rebuild asked three questions that are really questions about its ancestors:
getComputedStylefor every node up to the root;getComputedStyleand anew DOMMatrix()for every node up to the root, composed into a chain;Two siblings share their entire chain above themselves. On a 1689-element carousel that meant 18,700
getComputedStylecalls per single-frame step to answer a few hundred distinct questions.A rebuild now creates one
OverlayMeasurePassand threads it through the walk. Each node's visibility answer, composed transform and source boundary is computed once and reused by everything beneath it, so the tree costs one style read per node instead of one per node per descendant.Why a pass and not a cache
The first version of this change cached each element's finished rect across rebuilds, keyed on mutation records. An independent pre-review pass killed it, and it was right to.
A cache has to answer "what could have changed since last time". For a measurement the honest answer is anything: an
<img>finishing decode, a web font swapping in, a CSS transition frame, a container query re-evaluating, aCSSStyleSheet.insertRule— every one moves an element's box with nothing written to the DOM, so there is no mutation record to invalidate on. No affordable observer covers them all either:ResizeObserversees own-box size but not position, andtransitionend/animationendnever fire for an infinite CSS animation.A pass sidesteps the question instead of answering it wrong. It lives inside one synchronous measurement that only reads, so nothing can move under it, and it is dropped when the pass ends. There is no cross-rebuild state and therefore nothing to invalidate.
That costs some of the win — see below — and buys back a whole class of correctness. The falsifier is a regression test:
layout changes with no mutation record > re-measures an element whose own box changed with no DOM writeIt drives
recomputeOffCanvasIndicatorsend to end, changes only what layout reports between two rebuilds, and writes nothing. It passes onmain, failed on the cached version (-500,40,100,40where a full recompute gives-500,40,320,180), and passes here.Non-vacuity was proven by putting the defect back: letting the measured rects survive the rebuild in a
WeakMapfails both that test and the group one, with exactly the frozen values. Keeping the pass alive across rebuilds does NOT fail them, which is the point — the pass never holds a rect.What guards it
re-measures an element whose own box changed with no DOM writepays for a deeper tree per added ancestor, not per ancestor per cardre-measures a group when a member moves and nothing writes to the wrapperdata-hf-groupwrapper, whose box is the union of its members' rects — the one item measured from elements BELOW it.re-answers visibility when an ancestor fades between rebuildscreateOverlayMeasurePass()to module scope exits 1 on this test alone, against 0 at head.The cost assertion is invariance, not a threshold, matching the two tests already in that file: burying the same cards eight wrappers deeper may cost per added wrapper, and what it must not do is cost per wrapper per card.
Measured
Both arms measured back to back on the same machine at 1-minute load ~9, same fixture (1689 elements, 396 cards, 25 sub-compositions), same scrub (150 single-frame steps at 40 ms), hot mode, n=2 each. Raw counts, not derived:
getComputedStylegetBoundingClientRectThe layout-read line needs its explanation, because per seek is the wrong denominator for it. Rebuilds are throttled to 10 Hz, so a scrub that finishes 10% sooner contains 10% fewer of them. Divide out the rebuild count and the two lines separate cleanly:
getComputedStylegetBoundingClientRectSo: style reads down ~83% however you count them; layout reads flat per rebuild, and lower per seek only because the scrub gets through its 150 steps faster. That is the intended shape — each element still takes its own client rect on every rebuild, because that is the read that cannot be shared between siblings and must not be remembered between rebuilds.
Correction to an earlier revision of this table, which read
575 → 722 (+26%). Those were derived per-rebuild figures computed aswall x 10, and the two arms had been captured hours apart at 1-minute loads of roughly 100 and 200. That divisor assumes the throttle always achieves 10 rebuilds a second; under load the animation-frame loop is starved and it does not, so the divisor over-counts and the derived figure is meaningless. Both numbers were wrong and the apparent 26% regression was an artefact of comparing two different machine states. The table above replaces them with raw counts from one sitting.On layout reads specifically: a reviewer measured
441 → 301 (−32%)on a 300-element tree, from sharing the source-boundary rect between siblings. That saving is real and it cannot appear on this fixture, for a reason worth recording: the preview server inlines sub-compositions andmarkFlattenedInnerRootstripsdata-composition-fileanddata-composition-srcfrom the flattened roots, so the served carousel document contains zero source-boundary elements (measured:[data-composition-src]count is 0 in every probe). With no boundary to find,findSourceBoundaryreturns null for every element and there was never a secondgetBoundingClientRectto save. A composition that keeps its boundaries gets the reviewer's saving on top of the style-read one.The rendered marker set is byte-identical: the 152 markers' geometry hashes to the same SHA-256 on both arms and across every probe. Full-page screenshots are not a usable oracle here — they differ run to run on the same build, because lazily-loaded images land at different scrub points — so the marker geometry itself is the comparison.
Wall cost of the rebuild loop from CPU profiles, hot mode, 2 captures per arm: 58.3 / 59.7 ms per seek on main, 12.3 / 17.9 ms here. Those were taken earlier at load 100–200 and are directional only; the counts above are the measurement.
What this does NOT fix
The 5x bimodality is a separate defect and is not in scope. About one session in three the runtime's timed-element visibility pass never runs: 0 of 81
[data-start]elements ever get an inlinevisibility, so every scene stays painted on top of every other, 1313 elements compute visible instead of 90, and the editor draws 152 markers for elements the user cannot see. The animation keeps advancing, so nothing looks broken. It is a runtime lifecycle bug in another package, is written up separately with a reproduction, and is being picked up on its own branch.Also here
One unrelated one-line test fix: a
PropertyPanelcase was left on vitest's 5 s default while every sibling in that file passes the file's own 45 s constant. It times out in a full-suite run on a loaded machine, on either side of this change, and passes on either side when the machine is quiet.Review follow-ups applied
isElementVisibleThroughAncestorsreturns what it always returned, but resolves top-down now, so the set of nodes it takes a style read for differs (smaller for a subtree hidden near the root). Six call sites across five modules reach it with no memo and are unaffected in result —studioPreviewHelpers.ts(two),marqueeCommit.ts,useDomEditOverlayRects.ts,useMotionPathData.tsandsnapTargetCollection.ts, the last three through theisElementVisibleForOverlaywrapper — so the change is documented next to the function. (An earlier revision of this description said three; that was an undercount.)fallow audit. OnemountPreviewhelper now serves them and the pre-existing selector-index fixture.Checks
packages/studiofull vitest suitebunx oxlint,bunx oxfmt --checkon all changed filesbunx fallow audit --base origin/main --fail-on-issuestsc --noEmitforpackages/studiopackages/coretouched; no animation-frame loop scheduling line touched anywhereThe counts above were measured at
600de981a. The two commits since changehasAttributewalking and test scaffolding only — neither takes a computed style or a layout read — so the measured style/layout counts stand unchanged at the head.