perf(runtime): a paused preview stops burning CPU - #3845
Conversation
…paused A paused, untouched preview asked the browser for a fresh frame sixty times a second and re-read the page on each one. Nothing it looked at could change without some observable event firing, so the loop now stands down and wakes on that event instead, with a slow timer as the safety net. Parked, the loop keeps two jobs the 60 Hz version did implicitly: the control bridge's paused heartbeat, on the same interval as before, and a re-read of the timeline registry, which is a plain object no observer and no event can report. Everything else arrives by an event now: timing-attribute edits, mounted or removed timed elements and media metadata through the composition-timing observer that already existed; a manual-edit gesture starting or ending through a new attribute-filtered observer, which also replaces a whole-document query that ran on every paused frame; and playhead or play-state changes through the forced state post every transport mutation already ends in. Three periodic jobs (re-binding the root timeline, posting the clip manifest, binding media-metadata listeners) used a frame counter as a proxy for "the document may have changed". They now ask that question directly, because the counter stops advancing while the loop is parked. The render path is untouched: the loop never parks while an export render is driving frames. That test is the pair of renderCaptureSeekStarted and the producer's injected seek config, not the flag alone, because Studio's own preview falls back to renderSeek for overhanging timelines. Idle, paused, no input, on a 1689-element project: main-thread self time 128 -> 8.5 ms/s and animation-frame callbacks 282 -> 4.3 per second, both measured with the runtime and editor changes in place.
miguel-heygen
changed the title
The parked-loop change let a composition-timing change OR its way past shouldAttemptPeriodicTimelineBind, which removed the hold that keeps an async rebind off the first two seconds of playback, and let the clip manifest post on every frame of a composition that mutates the DOM every frame (measured: 30 posts in 30 frames against one). The change is now an input to that policy, which still applies the hold, and the change-driven path is confined to the paused path and rate-limited to the posts per second the frame counter already produced. A change it defers stays pending, and a pending change keeps the loop awake, so deferring can never drop it. Two more holes from the same review: The parked poll compared only the composition timing revision, so an adapter duration floor that grew was never noticed. Adapters infer duration from live animation objects that change with no DOM mutation and no media event, which makes it the second input nothing can push; both are now in one witness. Draining the gesture observer's records to answer within a task suppressed the observer's own callback for them, so a reader could consume the notification that un-parks the transport. Draining now notifies. The watch reports whether it is observing at all, and the loop refuses to park when it is not. That path is unreachable today because the colour-grading runtime constructs a MutationObserver unconditionally during the same init; the flag is the explicit statement of the invariant for the day that changes.
postTimeline walks author DOM and can throw. The tail scheduler runs in the tick's finally either way, so clearing the latch first let it see nothing owed and park with the change undelivered — and nothing would deliver it until some unrelated change happened to wake the loop again. Clearing after the post means a throw leaves the change owed, the loop stays awake, and the frame counter retries it within twenty frames. Same rule the shared editor loop already follows for its own re-arm. Also rewords the note on draining the gesture observer's records: that is hardening, not a fix for a live lost wake. isActive has exactly one caller today, inside transportTick, and a tick is its own task, so the observer's microtask has already run by then.
The parked transport holds a timer where the old loop held only an animation frame, and a frame is discarded when a page or a test environment is torn down while a timer is not. A test that initialises a runtime and abandons it therefore left an 80ms timer to fire into a dead global, which CI reported as an unhandled ReferenceError attributed to whichever file was running when it landed. Two changes, because the leak has two ends. The heartbeat stops instead of re-arming when window or document is gone: nothing is left to report a state change to, so stopping is the answer rather than throwing. And the one test that initialised a runtime without ever tearing it down now tears it down.
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
APPROVE at 637e2fe942e37bdcd8cad68aa0a978df6b598fcd — runtime transport stops requesting a fresh frame 60x/s while nothing on screen is changing, wakes only on observable events with a slow heartbeat as the safety net, and the two inputs no event can announce are polled off the same heartbeat with a fixed staleness bound. Render path stays 60Hz behind the same PAIR guard scheduleMetadataDurationHydration already uses.
Author + scope. miguel-heygen (trust-listed). Head 637e2fe94. Base main. +968/-68 across 6 files, all in packages/core/src/runtime/ — init.ts, init.test.ts, init.timingResolver.test.ts, manualEditGestureWatch.ts (new), manualEditGestureWatch.test.ts (new), timelineRebindPolicy.ts, transportPark.test.ts (new). No scope creep. mergeStateStatus BLOCKED (missing approval — this is the first at-head review). Reviewed each of the four commits' actual mechanism, not just its fix message.
Park mechanism
transportTick's self-re-arm (the old state.transportRafId = window.requestAnimationFrame(transportTick) line) is gone. Scheduling lives at the tail in scheduleNextTransportFrame at init.ts:3486, which either arms rAF (a wake was requested, or the loop can't park) or arms a setTimeout heartbeat via armParkTimer (:3460). Test stops asking for animation frames once paused and settled at transportPark.test.ts:144 pins raf.pending() === 0 post-quiesce.
The TWO no-push fallback inputs (Via's Q1)
readParkedPollWitness at init.ts:3457-3458 reads two things:
${readCompositionTimingRevision()}|${resolveAdapterDurationFloorSeconds() ?? ""}
- Input 1: timeline registry (
window.__timelines, plain object — no observer/event can announce a change). Signature compare inreadCompositionTimingRevisionat:1090-1094. Staleness surface:getDuration()on the player. Pinned bydelivers a timeline registered into window.__timelinesattransportPark.test.ts:191. - Input 2: adapter duration floors — CSS / WAAPI / Lottie animation-object inferred durations, no event when the underlying animation object mutates its internal duration.
resolveAdapterDurationFloorSecondsatinit.ts:918. SamegetDuration()surface. Pinned bydelivers an adapter duration that grows while parkedat:408(uses Lottie).
N = state.bridgeMaxPostIntervalMs = PARK_HEARTBEAT_MS = 80 (same interval as the pre-refactor paused heartbeat — Miguel's "on exactly the interval it used before"). Worst-case observable staleness for the two witness inputs moves from ~16ms (60Hz frame poll) to ~80ms — one heartbeat tick. Both surface only through getDuration(); no scrub-position / play-pause icon / badge depends on either witness input in the diff's read paths.
Park/resume race (Via's Q2)
No explicit interleaving test (fire wake DURING the park transition), but the mechanism is static-race-safe. wakeTransport sets transportWakeRequested = true when re-entered mid-tick (init.ts:3507-3508); scheduleNextTransportFrame reads the flag as its park-vs-wake decision at :3489-3492. Any wake fired between "decided to park" and "actually parked" is either absorbed by the flag or triggers wakeTransport which clears the timer + arms rAF.
Every wake source has an independent test pinning "external signal → transport observes it while parked": attribute mutation (transportPark.test.ts:178), registry change (:191), late DOM mount (:207), media metadata event (:225), gesture attribute (:240), gesture end reconciling seek (:251), control-surface seek (:277). An explicit interleaving test would harden the invariant explicitly rather than relying on flag re-entry reasoning; non-blocking follow-up, worth adding.
"Ask if the document changed" for the 3 periodic jobs
Composition-timing revision readCompositionTimingRevision() at init.ts:1076 is the key; bumped in invalidateCompositionTimingCaches at :1022, driven from:
- MutationObserver on
documentElementfiltered byDURATION_FLOOR_INPUT_ATTRIBUTES(:1048-1053) - capture-phase
loadedmetadata,durationchange,emptiedlisteners (:1058-1060) - observer drain on read (
:1084-1086) - registry signature compare on read (
:1090-1094)
Old counter path preserved when playing — changeDrivenService gated by !clock.isPlaying() at init.ts:3530. During playback only the counter cadence (transportTickCount % TIMELINE_POST_INTERVAL_FRAMES === 0) fires. Change-driven path rate-limited to CHANGE_DRIVEN_SERVICE_MIN_INTERVAL_MS = 1000*20/60 ≈ 333ms (timelineRebindPolicy.ts:12) so no consumer sees a faster cadence than the counter would have produced at 60Hz. Pinned by keeps the manifest on its frame cadence while playing, whatever the DOM does at transportPark.test.ts:335.
Rebind-policy hold still owned by shouldAttemptPeriodicTimelineBind (timelineRebindPolicy.ts:26) — compositionChanged is passed IN as an input (init.ts:3552-3557), so the 2-second play-hold isn't ORed around. Policy short-circuits on the hold before consulting the trigger.
Render-path preservation
Guard is the PAIR: isExportRenderDrivingFrames = renderCaptureSeekStarted && window.__HF_EXPORT_RENDER_SEEK_CONFIG != null at init.ts:959. Used at canParkTransport (:3405) AND scheduleMetadataDurationHydration (:2041) — same pair, one owner, exactly what Miguel names. Flag-alone would latch on the first Studio scrub of an overhanging-timeline project because playbackAdapter.createStaticSeekPlaybackAdapter also calls renderSeek; the pair is required.
Pinned by two tests: never parks while an export render is driving frames (:289, config set) and still parks after Studio's own renderSeek fallback, which is not a render (:304, config unset). Reverting to flag-alone flips the second red.
Drag detection — MutationObserver replacing document.querySelector
createManualEditGestureWatch(document, wakeTransport) at init.ts:3382. Observer on doc.documentElement with subtree: true, attributes: true, attributeFilter: [STUDIO_MANUAL_EDIT_GESTURE_ATTR] at manualEditGestureWatch.ts:69-73. Same observer un-parks on both drag START and END. Initial state seeded from doc.querySelectorAll(SELECTOR) at :51 — handles mid-drag re-init where the marker already exists. takeRecords() drained in isActive at :90 makes the answer correct within the synchronous task, not just across tasks.
Fallback for no-MO returns observing: false at :43, and canParkTransport gates on manualEditGestureWatch.observing at init.ts:3410 — a host with no MutationObserver never parks (safe direction). Pinned by 4 tests in manualEditGestureWatch.test.ts and 2 in transportPark.test.ts (:240 gesture attribute, :251 gesture-end reconciling seek).
Head-commit page-gone-away fix
parkedTransportHeartbeat guard at init.ts:3474: if (typeof window === "undefined" || typeof document === "undefined") return;. Bails without re-arming when the embedder discards the frame or a test env is torn down around an abandoned runtime. Teardown itself clears the timer at :4001-4003. init.timingResolver.test.ts:50 adds an afterEach(() => window.__hfRuntimeTeardown?.()) covering the abandoned-runtime case for that suite.
No direct test that stubs window/document to undefined and asserts early-return — optional pin, low-risk defensive check.
Test invariants
Every mechanism above has a pinning test in transportPark.test.ts. Additional discipline: does not park when the manifest post throws with a change still pending at :360 pins commit 3's "clear latch AFTER post" ordering — a throwing postTimeline must leave compositionChangePending=true so the loop stays awake. stops both the frame loop and the parked timer on teardown at :319 verifies vi.getTimerCount()===0 post-teardown (not just silence).
CI
60 SUCCESS / 1 SKIPPED at head. Skipped is Catalog: search index covers the registry, correctly skipped by path-filter on a runtime-only PR.
Non-blocking observations
- Double-arm race in
scheduleNextTransportFrame. Atinit.ts:3499parkedPollWitness = readParkedPollWitness()can synchronously callwakeTransport()via the observer-drain path (readCompositionTimingRevision→invalidateCompositionTimingCaches→wakeTransport,:1085). In that re-entrant call,state.transportRafId==nulland!inTransportTick, sowakeTransportarms a rAF. Control returns andarmParkTimer()at:3500runs unconditionally — arming asetTimeouttoo.armParkTimerat:3460overwritestransportParkTimerIdwithout clearing, so the previously-armed one leaks and eventually fires an extraparkedTransportHeartbeat→ extrapostState. Triggers when the tick's own work causes observer-observable mutations (e.g.sanitizeCompositionDurationAttributesat:528stampingAUTHORED_DURATION_ATTRon newly-mounted sub-composition nodes). The!node.hasAttributeguard means it fires at most once per node, so this is bounded by composition-mount events — not a steady-state leak. Each extra timer is a singlepostState. No correctness impact; on teardown the extras no-op via thestate.tornDownguard. A guard at:3500(if (transportWakeRequested || state.transportRafId != null) return;) or makingarmParkTimerclear an existingtransportParkTimerIdbefore setting would close it. - Explicit park/resume interleaving test absent — see Via's Q2 above; static reasoning is sound but an explicit test would harden.
- Page-gone-away guard has no direct pin — optional.
- CI reduced-matrix note does not apply here — full CI ran on this base=main PR.
Coordination. Via (peer bot) deferred stamp to tai as primary with three shape-of-question angles. Q1 (2 fallback-timer inputs) verified above with names, N=80ms, and surface (getDuration()). Q2 (park/resume race) verified via transportWakeRequested flag + per-source wake tests; explicit interleaving test noted as follow-up. Q3 (shared-loop questions) applies to #3846, not this PR. Concur with Via's read across the concern surface — the two fallback-timer inputs ARE the residual risk surface she named, and they are correctly bounded to ~80ms staleness on getDuration().
— Review by tai (pr-review)
2 participants
A paused preview kept asking the browser for a fresh frame sixty times a second and re-reading the page on each one, with nothing on screen changing. It now stands down and wakes when something actually happens.
Nothing a tick could discover was able to change without some observable event firing first, so the loop re-arms on those events instead of on a frame, with a slow timer as the safety net. That timer keeps two jobs the sixty-a-second version did implicitly: the control bridge's paused heartbeat, on exactly the interval it used before, and a re-read of the timeline registry, which is a plain object that no observer and no event can report a change to.
Three periodic jobs — re-binding the root timeline, posting the clip manifest, binding media-metadata listeners — used to fire on a frame counter, which was only ever standing in for "the document may have changed since last time". The counter stops advancing while the loop is parked, so they now ask that question directly, off the composition-timing revision the duration caches already track. The old counter stays as a backstop while the loop is awake, so playback behaviour is unchanged.
"Is the Studio mid-drag?" was a whole-document
querySelectoron every paused frame. Measured, that alone was 1.8 ms/s on a 1689-element project and 3.0 ms/s (6.4% of the entire paused main-thread budget) on a media-heavy one. It is now an attribute-filteredMutationObserver, and the same observer is what un-parks the transport when a drag starts or ends.The render path is untouched. The loop never parks while an export render is driving frames. That test is the pair
renderCaptureSeekStarted && __HF_EXPORT_RENDER_SEEK_CONFIG, not the flag alone — the same pairscheduleMetadataDurationHydrationalready uses, and for the same reason: Studio's own preview falls back torenderSeekfor overhanging timelines, so the flag alone would latch on the first scrub of such a project and leave that session unparked for good.Every state change the loop used to find by looking again, and how it arrives now
delivers a timeline registered into window.__timelines, which no observer can seedelivers an adapter duration that grows while parked, with no DOM mutation and no eventdata-durationor other timing-attribute editMutationObserverthat already existed → wakedelivers a live data-duration edit while parkeddelivers a timed element mounted after the loop parkedload()loadedmetadata/durationchange/emptiedlisteners that already existed → wakedelivers media metadata that arrives after the loop parkedMutationObserveron the gesture marker → wakedelivers the start of a Studio manual-edit gesture while parkedruns the reconciling seek the frame a manual-edit gesture endswakes on an explicit seek from the control surface,keeps asking for animation frames while playingposts the paused bridge heartbeat on its documented interval while parkednever parks while an export render is driving frames,still parks after Studio's own renderSeek fallback, which is not a renderkeeps asking for animation frames while playingstops both the frame loop and the parked timer on teardownEach row was proved non-vacuous by breaking its delivery path on purpose and confirming exactly the expected tests fail.
Second commit: what the independent pre-review pass found
Playback was not unchanged, and that was blocking. The change signal was ORed in front of
shouldAttemptPeriodicTimelineBind, which removed the hold that keeps an async rebind off the first two seconds of playback, and let the clip manifest post on every frame of a composition that mutates the DOM every frame — 30 posts in 30 frames against one. The policy is the single owner of "may rebind now" again: the change is an input to it, the hold still applies to every trigger, and the change-driven path is confined to the paused path and rate-limited to the posts per second the frame counter already produced. A deferred change stays pending and a pending change keeps the loop awake, so deferring can never drop it; the frame counter clears it within 20 frames either way, so it cannot hold the loop awake indefinitely. Test:keeps the manifest on its frame cadence while playing, whatever the DOM does.A second input nothing can push. The parked poll compared only the composition timing revision, so an adapter duration floor that grew was never noticed. Row 1b above; the "single input" claim in the first commit's message was wrong and this corrects it.
The teardown test was vacuous for the timer. Deleting the
clearTimeoutleft it green, because a heartbeat that is still armed returns early ontornDownand posts nothing either. It now asserts the timer count is zero.Draining now notifies, defensively. Reading the gesture watch inside a task takes the observer's records, which suppresses the observer's own callback for them, and that callback is what un-parks the transport. This is hardening, not a live bug:
isActivehas exactly one caller today, insidetransportTick, and a tick is its own task, so the observer's microtask has already fired before any drain. A second caller running earlier in the task would eat the wake, and nothing in the type would stop it.Where nothing can push at all. The watch reports whether it is observing, and the loop refuses to park when it is not. That path is unreachable today: the colour-grading runtime constructs a
MutationObserverunconditionally during the same init, so a host without one never reaches a running transport. The flag stays as the explicit statement of the invariant, andmanualEditGestureWatch.test.tspins it.Measured
Idle means paused, loaded, focused, no key presses, no pointer. Two projects: a 1689-element / 396-card carousel and a 79-video / 12-audio media project. Both arms were served from the same rig and captured interleaved within each instrument, so a machine-load spike lands on both. This Mac was running other work throughout (1-minute load 44 to 256 across the captures), which inflates absolute milliseconds on both arms equally; the counts do not move with load and are the harder numbers.
FireAnimationFrameper secondCommitper secondsample)Two interleaved passes per fixture per instrument on the final build; the renderer-percentage row is the median over five passes because process sampling is the noisiest instrument here. An earlier three-pass run on the same code minus three behaviour-neutral refactors gave 129.2 → 9.4 ms/s and 283 → 3.8 callbacks per second on the carousel, and 66.8 → 13.0 ms/s and 287 → 3.7 on media: the same result.
Compositor commits are the row worth reading twice. Frame callbacks are a mechanism metric and a change like this can move them without moving any work; commits cannot be gamed, because nothing commits a frame without having done the work. They fall with the callbacks, so the work really is gone.
Two numbers that are not measured
The idle table below was captured before two later changes and has not been re-taken; disk on the measuring machine is under 3 GB and each trace capture is 28-100 MB.
getAnimations()per animated element. It cannot be a regression against the base — the per-frame loop made the same call 60 times a second — but the after column is optimistic by whatever that costs at 12.5 Hz, and on a composition with many CSS-animated elements that is not nothing.A correction to the callback-count figure, and a sixth loop
Re-verifying the final build in a real browser with a per-realm probe (patching
requestAnimationFrameand recording the caller's stack) found 297 of 297 sampled registrations in the preview realm coming from the Studio bundle and none from the runtime, at about 60 a second. Attributed from the served bundle, it isstartStudioManualEditPlaybackReapplyinmanualEdits.ts: it re-arms for as long as__player,__timelineor any entry of the timeline registry reports itself as playing, and on this project a scene timeline added to the paused root is itself unpaused, so it never stops.Two things follow, and both are stated rather than smoothed over:
manualEdits.tsis untouched by either branch and is a candidate for the same treatment in follow-up work. I could not build a control to date the discrepancy: the before-arm build's dependencies have since been removed from the machine.Playback, scrubbing and jumps are unchanged
Same harness, play / scrub / jump phases, interleaved. Five replicate pairs on media, two on the carousel.
Playhead positions and settle times are identical. Dropped frames are not worse on any run; the two arms' worst cases are 503 and 32, and both arms had runs where playback stalled entirely under a load spike (two of five each), so the stall is the machine, not the change. The carousel's playback frame rate rising from ~27 to ~45 is the same main-thread time coming back.
End to end, in a real browser
On the same selected card, at the same playhead advance, the selection overlay moved 255.8 px on both arms — the overlay still tracks an animating element with the loops parked. A drag of a timeline-animated card moves the box on neither arm (the Studio blocks manual movement of an element whose transform the timeline owns), which is the same behaviour before and after.