perf: sublinear Studio rebuilds and seeks - #3837
Conversation
The iframe->overlay basis (composition root, root scale, iframe and overlay
rects) was resolved inside every geometry call, so measuring a preview cost one
querySelector("[data-composition-id]") plus three layout reads PER ELEMENT to
rediscover something that is a property of the composition and the canvas zoom.
It is now threaded through orientedGroupAwareOverlayRect, groupAwareOverlayRect,
orientedOverlayRect, orientedVisibleOverlayRect and toVisibleOverlayRect the way
toVisibleOverlayRects already batched it, and resolved once by the two callers
that measure many elements in one synchronous pass: the off-canvas indicator
rebuild and the overlay RAF loop.
Both passes only read the DOM, so nothing can move the canvas between two
measurements inside one of them.
A MutationObserver marked the off-canvas indicators dirty and the rebuild then re-derived every element in the preview from scratch. The observer's loudest source is inline style, which is what animation writes, so a composition just sitting there re-derived the whole document several times a second, and each element costs two getComputedStyle reads, two ancestor walks for its source file, and a textContent read over its whole subtree. The records now say WHAT to drop, not merely that something changed, and the per-element derivations are memoized between rebuilds. Two lifetimes, because they do not go stale together: whether an element renders (and how many layer children it has) dies on any nearby attribute write, while how it is ADDRESSED survives every style write and dies only on something that can renumber a selector. That split is what makes an ordinary animation frame cost no document queries at all. Not attributable to specific elements, so still a full rebuild: nodes added or removed, and any change to an identity attribute, which renumbers every element sharing a selector. Two supporting changes: - getDirectLayerChildren asked getDomLayerPatchTarget for a full patch target per child and used it as a boolean. The target carries the selector's occurrence index, which is a whole-document query the yes/no does not depend on. isDomLayerElement answers it without one, for every caller. Its unused options parameter goes with it. - The observer no longer filters attributes. An attribute it never hears about is one the cache would answer stale for, and the old filter omitted id and the data-composition-* attributes that decide a layer's identity. Widening only makes indicators refresh sooner; a rebuild is a pure read and is throttled either way. Every other caller of collectDomEditLayerItems passes no cache and is unchanged.
Every seek, and every frame of playback, rebuilt the window of every video and audio element in the document and handed all of them to the per-clip sync loop. On a composition of eighty videos that is eighty window derivations and eighty per-clip passes to conclude that one clip is on screen. A clip is active only while start <= t < end, so a clip whose window excludes the new time is inactive there whatever its element state is. Sorting the windows by each endpoint turns a seek into two binary searches: the clips whose start or end lies between the old time and the new one, plus the ones that were in window at the old time. Anything outside both was out of window before and is out of window now, and was already paused and already evicted by the pass that last saw it go out. A one-frame step visits the clip or two that flip; a jump over a hundred clips still visits the hundred boundaries it crosses. The index carries only the windows, and it rides the revision the duration floors already ride: the same timing attributes, the same media metadata events, the same timeline registry signature. Nothing else is cached. Every field handed to syncRuntimeMedia is re-read from the element on every pass, because el.duration can be reset under us by the load() retry, and a cached copy of it would be exactly the stale duration the two-resolver-scope rule exists to prevent. The render/export path does not consult the index at all and visits every element on every frame, and a render seek clears the sweep so a later live seek cannot inherit a position it did not establish. Also folds the duration floors' own invalidation into that shared revision. Draining the observer is what detects a change, and only the first reader in a task gets the records, so two caches each checking for themselves would have had the second told nothing changed.
…o their own modules Clears the 600-line file cap and the fallow audit on this branch. Behaviour is unchanged: this moves code, it does not alter any of it. File size. Both files were already near the cap on main and my three levers pushed them over (615 vs 598, and 605 vs 584): - domEditOverlayGeometry.ts -> 561. The iframe->overlay coordinate basis (OverlayRootScale, computeOverlayRootScale and the root/dimension lookups it owns) moves to domEditOverlayBasis.ts. It is the one piece of that file every other piece depends on, and nothing in it is about a single element's geometry. Its two callers now import it from there. - domEditingLayers.ts -> 587. The per-element read that the walk performs moves to readDomEditLayerWalkEntry in domEditLayerWalkCache.ts, the module that owns the memoization it reads through. The walk keeps the traversal and the depth bookkeeping, which are the parts that are actually about walking. No unrelated code was trimmed to make room. Duplication, all three clone groups fallow flagged: - The runtime seek fixture (createMockTimeline, the synchronous animation-frame clock, the CSS.escape shim, stubDuration) was copied between init.timingResolver.test.ts and init.mediaClipIndex.test.ts. It moves to runtimeSeekFixture.test-helpers.ts. Each suite keeps its own vi.mock calls, which are file-scoped and cannot be shared. The file is excluded from tsconfig.runtime.json alongside the test files it serves, for the same reason. - domEditLayerWalkCache.test.ts repeated its mount/observe/first-walk setup in three tests; that is now withWarmWalk. Complexity. Only one of fallow's three findings is attributable to this branch, and fallow agrees: it marks the other two inherited and excludes them from the gate. - offCanvasIndicatorRefresh.ts update was NEW (absent from main's report, 10 cyclomatic / 31.6 CRAP here). The optional-chain-and-default I added to drain the observer becomes drainPendingLayerMutations, with its own unit tests, and the function drops off the report entirely. - useDomEditOverlayRects.ts update: 37 cyclomatic / 69 cognitive on main AND here. My change added one statement and no branch; the function grew 139 -> 147 lines, which is what resurfaced it. Left alone. - domEditingDom.ts escapeCssIdentifier: 24 cyclomatic / 19 cognitive / 148.4 CRAP on main AND here. Untouched by this branch; only its line number moved (173 -> 182) because the composition-source-map revision counter sits above it. Left alone.
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
APPROVE at a0134ca841cf6678f30698c0a5ee13fb17318330 — three-lever perf refactor (overlay basis per composition + mutation-driven walk cache + time-window-indexed seek). Semantic verification stands from the pre-CI-fix review; delta from ded4972196c9de4995df4bdef8b108d53f336505 is pure moves + one clean module extraction with no semantic drift.
Head + author. 4 commits all by miguel-heygen. mergeable=MERGEABLE, mergeStateStatus=BLOCKED (missing approval; fresh CI run in progress).
Delta vs ded4972… (the fix-up commit). 12 files, +300/-182 net, all mechanical:
packages/studio/src/components/editor/domEditOverlayBasis.ts— NEW (72 lines): extractedOverlayRootScaleinterface +computeOverlayRootScale+findOverlayRootElement/resolveRootDimensions/readPositiveDimensionsupport helpers, with an improved docstring naming the composition-scoped-vs-element-scoped invariant.packages/core/src/runtime/runtimeSeekFixture.test-helpers.ts— NEW (64 lines): shared test fixture previously duplicated acrossinit.mediaClipIndex.test.tsandinit.timingResolver.test.ts.packages/core/tsconfig.runtime.json— +1/-1: adds"**/*.test-helpers.ts"toexclude, alongside"**/*.test.ts". Correct — the new shared-helper file otherwise fails the runtime build.packages/studio/src/components/editor/domEditLayerWalkCache.ts— +67/-1: absorbsreadDomEditLayerWalkEntry(per-element cache-read with graceful fallback for no-cache callers — same code path the walk previously did inline) +drainPendingLayerMutations(takeRecords wrapper). Both encapsulations preserve the four-independent-lifetimes reasoning; the new docstring explicitly names why the four reads go through cache separately.packages/studio/src/components/editor/domEditOverlayGeometry.ts— +5/-59: geometry file back to 561 lines by moving the basis/root helpers into the new module. Retains only per-element geometry.packages/studio/src/components/editor/domEditingLayers.ts— +7/-25: walk-read extracted (down to 587 lines).packages/studio/src/components/editor/offCanvasIndicatorRefresh.ts— +2/-5:walkCache.ingest(observer.takeRecords() ?? [])replaced withdrainPendingLayerMutations(observerRef.current, walkCache). Comment moved into the extracted function's docstring — no logic drift.packages/studio/src/components/editor/domEditLayerWalkCache.test.ts— +64/-20: new observer-drain unit tests.packages/core/src/runtime/init.mediaClipIndex.test.ts— +7/-30: uses shared helper.packages/core/src/runtime/init.timingResolver.test.ts— +8/-39: uses shared helper.packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts— +2/-1: import update.packages/studio/src/components/editor/useDomEditOverlayRects.ts— +1/-1: import update.
File-size + fallow. Both required lanes green at head per Miguel's confirmation. domEditOverlayGeometry.ts at 561 (was 615), domEditingLayers.ts at 587 (was 605). Two remaining CRAP findings (pre-existing mass on useDomEditOverlayRects.ts:update and domEditingDom.ts:escapeCssIdentifier) are inherited and fell out of diff scope naturally — no fallow-ignore suppressions added.
Semantic verdict (from prior head, unchanged by these moves).
LEVER 1 — overlay basis per composition. computeOverlayRootScale is a local-var-per-synchronous-pass (one call per RAF frame in useDomEditOverlayRects, one call per rebuild in recomputeOffCanvasIndicators); no cross-frame cache exists to invalidate. Parity trivial (fallback path IS the same function). Invariance test at offCanvasIndicatorGeometry.complexity.test.ts:135 pins querySelector("[data-composition-id]") count invariant at 12 vs 48 cards with ceiling of exactly 2 (walk root + basis) — the ceiling doubles as an anti-stale-cache guard.
LEVER 2 — mutation-driven layer walk cache. Two lifetimes (presence WeakMap, identity WeakMap) because a style write flips whether an element renders without touching how it's addressed. Observer UNFILTERED (attributes: true, characterData: true, childList: true, subtree: true, no attributeFilter) — the design comment at domEditLayerWalkCache.ts:79-83 calls this out ("an unlisted attribute is one the cache would never learn about"). childList or SELECTOR_IDENTITY_ATTRIBUTES change → invalidateAll. First rebuild = every element misses cache = full derivation. Iframe swap → walkCache.invalidateAll(). inlineVisibilityChanged remembers parsed CSSOM per element to avoid subtree-walk per transform tick under GSAP autoAlpha (guarded by domEditLayerWalkCache.test.ts:189). Parity witness at domEditLayerWalkCache.test.ts:161 (10 mutation kinds, .toEqual uncached walk on full item shape).
LEVER 3 — seek visits only crossable clips. MediaClipIndex with byStart/byEnd sorted arrays + binary-search lower-bound. Sweep predicate = mediaClipsInWindow ∪ {start ∈ [lo, hi]} ∪ {end ∈ [lo, hi]}, symmetric for forward/backward, inductive-correct by boundary reasoning (start_C == t_prev case straddles previous seek's hi_prev). Shared compositionTimingRevision counter folds duration-floor invalidation with clip-index invalidation into ONE signal — the two cache consumers cannot diverge. Every signal that busted durationFloorsCache on main (MutationObserver on DURATION_FLOOR_INPUT_ATTRIBUTES including id/src; capture-phase loadedmetadata/durationchange/emptied; timeline registry signature; childList+subtree) now also busts mediaClipIndex. Render/export bypass at init.ts:2483 (renderCaptureSeekStarted) sets lastSyncedMediaTimeSeconds = null; mediaClipsInWindow = [] so a later live seek cannot inherit a stale sweep. Cross-check witness at init.mediaClipIndex.test.ts:147 — 4 random seek sequences × 16 seeks × deterministic seed 0x5eed × full state comparison per element.
Lever independence. L1/L2/L3 hunks are in disjoint files except offCanvasIndicatorGeometry.ts (L1 adds computeOverlayRootScale call + threads scale; L2 adds walkCache? param) — orthogonal concerns, git revert on one would mechanically conflict but the semantic split is clean. compositionTimingRevision folding (L3) is an intentional consolidation; reverting L3 would need to re-split into two counters, but neither L1 nor L2 depend on it.
Non-blocking coverage gaps (from the extension pass, worth naming as follow-ups):
- No test simulates observer disconnect during high-mutation load. Real-world reconnect happens only on iframe swap where
walkCache.invalidateAll()fires unconditionally — safe by construction, not by test. - The seeded-random cross-check at
init.mediaClipIndex.test.ts:159uses stable clips. A duration-change-between-seeks case would strengthen it: e.g.seed → seek 5 → Object.defineProperty(video, "duration", {value: 3}); video.dispatchEvent(new Event("durationchange")) → seek 6, then assert indexed state equals full-visit state after the mutation. Currently the "clip moved" test at line 232 is the closest proxy but asserts only a boolean (visibility != "hidden"), not the full state comparison. Correctness inherited by construction from the shared revision counter, but explicit test would pin it against thedurationchangepath specifically.
CI at new head. No FAILURE / CANCELLED. Fresh run kicked off by the fix-up push has file-size + fallow SUCCESS (the two prior-head blockers), remaining lanes IN_PROGRESS (Test, Typecheck, Build, Player-perf ×5, all 9 regression shards, Windows render, CodeQL, CLI smoke). Branch protection separately gates merge on the fresh run settling.
Concurrent reviews. No at-head APPROVE / CHANGES_REQUESTED.
— tai
2 participants
Editing a big composition in Studio stays responsive as the composition grows. Before this, three pieces of work were sized by how big the project is rather than by what you just changed: moving the playhead on a project with 79 videos did work for all 79, and nudging one element made the editor re-measure all 1,689 elements on screen.
Three independent changes, one commit each so they can be reviewed and reverted separately.
1. The editor works out where the canvas is once, not once per element
The overlay that draws selection boxes and off-screen markers has to convert coordinates from the preview into screen space. That conversion depends on the composition and the zoom level, not on the element being measured — but every element was working it out from scratch.
Measured on a 1,689-element project: 968.9 → 17.9 canvas lookups per overlay rebuild.
2. An edit costs what the edit touched, not what the project contains
A change in the preview told the editor "something moved", and the editor then re-derived every element on screen. The change notifications now say which elements moved, and everything else is reused.
The saved work is split in two, because the two halves stop being valid at different moments. Whether an element is visible changes constantly (that is what animation writes); how an element is addressed changes only when someone renames it or adds a node. Keeping them apart is what lets an ordinary animation frame do no document lookups at all.
Structural changes — nodes added or removed, or anything that renumbers an element's identity — still rebuild everything, deliberately.
Measured: one style edit costs the same whether the project has 12 cards or 48 (61 → 241 style reads before, flat after), and 3 → 0 document lookups.
3. Moving the playhead only touches clips that can start or stop
Every playhead move rebuilt the timing of every video and audio clip in the project and then asked each one whether it should be playing. A clip can only be active while the playhead is inside it, so sorting clips by their start and end times turns that into two lookups: the clips whose edges the playhead just crossed, plus the ones already playing.
Measured on a 79-video, 12-audio project: 91 → 10 clips touched per playhead move. A long jump still visits every clip whose edge it crosses — that is the point, not a gap.
Rendering and export are deliberately excluded and still visit everything on every frame. A frame captured against stale timing cannot be recovered afterwards.
Behaviour
Unchanged: same manifest, same playback, same render output. Each of the three is held to that by a test that fails without it:
Verification
packages/coreruntime CI (including the parity and duration guards): 52 files, 1,102 tests, exit 0.packages/studio: 436 files, 4,814 tests, all passing.tsc --noEmitclean in bothpackages/coreandpackages/studio.A/B measurements were taken in a real browser against this branch and against the base, on two stress projects, counting operations rather than milliseconds because the machine is shared. Every measurement carries a liveness counter proving the measured code actually ran, asserted non-zero before any ratio was read.
Size
Larger than the usual budget at roughly 1,336 changed lines, about 530 of them tests. The three levers are independent and split cleanly by commit; they are together because they were measured together against the same two stress projects.