Repository navigation
perf(core): a preview's composition scripts give the page a turn every 50 ms, not one long task - #5713
Conversation
…y 50 ms, not one long task
Edit accuracy: accurate 271 (base branch 271), smooth 154 of thoseThe gate passes. Quarantined, measured but not gated (0) |
terencecho
left a comment
There was a problem hiding this comment.
Source review at 92833de1f1734b842968ac4d1478c0f862ff9a70, against effective base a43cdd93bf3f5cd5f04f797b601bac907a99432e (distinct from the current API base). No introduced, reachable blocker found.
packages/core/src/runtime/afterFonts.ts:130–146: the new task boundary is between scripts; font gating, classic/late ordering, in-place execution, existing external/module waits, and bootstrap-before-held-event replay remain intact.packages/producer/src/services/fileServer.ts:368–384,767–835: traced the actual truthy virtual-time marker and pre-head installation through the in-process render, reusable probe server, and distributed render callers. Those routes take no new yielding branch. Also checked the live-swap runner call atpackages/core/src/runtime/init.ts:3757–3765and synchronous per-script scope/animation bookkeeping.
Non-blocking wording nit: packages/core/src/runtime/afterFonts.ts:87 says the virtual clock holds every timer. The actual shim retains native setTimeout/setInterval; it virtualizes Date/performance/rAF. Narrow that explanation rather than asserting all timers stall. The render exclusion itself is preserved.
Scope: both changed files read fully at effective BASE before HEAD, plus source-traced integration points and two independent bounded audits. The new test meaningfully distinguishes preview/render sequencing, but uses a mocked scheduler/clock. No runtime, browser, tests, CI, or live services executed; native scheduling/MessageChannel fallback and reported performance gains remain unverified. No prior public blocking review to reconcile.
Verdict: APPROVE
Reasoning: The added preview-only task boundary preserves the audited execution and startup contracts; no concrete introduced wrong outcome survived review.
— tai
What
In a preview, the runtime now hands the page back to the browser after every 50 ms of composition scripts instead of running every composition's script in one task. Scripts still run once each, in the same order, before the runtime boots. Renders are unchanged: under the producer's virtual clock (
window.__HF_VIRTUAL_TIME__) the runner never yields.Why
runScriptsAfterFontsruns every composition script back to back. On a long film that is one very long task: in a 9-minute film with 91 sub-compositions it measured 6.5 to 7.7 s in Chrome, during which the page answers nothing (no input, no paint, no ping). Apps that host the preview and watch for a frozen page read that as a hang.Related work
Refs #5674 and #5675, which make playback and the Studio timeline scale with what is on screen; this is the load-time half. A follow-up will defer the scripts of compositions far from the playhead, which is what brings the time to a ready preview down.
How
runScriptsAfterFontstracks how long the current slice of scripts has run and, pastPREVIEW_SCRIPT_SLICE_MS(50 ms), awaits the next task before the next script.scheduler.yield()where it exists, else aMessageChannelmessage: nestedsetTimeout(0)calls clamp to 4 ms each.__HF_VIRTUAL_TIME__the runner does not yield, because a render's virtual clock holds timers until it advances and the yield would never return.Test plan
entry.afterFonts.test.tschecks that a preview takes a turn once a slice has run (scripts loga in-place turn b ready) and that a render takes none (a in-place b ready). Both pass 3 runs in a row with the rest of the file (13 of 13). fails without the fix: with main'safterFonts.tsthe preview case logsa in-place b ready, with no turn.bun run typecheck, oxlint, oxfmt and the comment ratchet pass.hyperframes preview(headless Chrome, two runs): the longest task at open went from 6.5-7.7 s to 0.88-0.96 s (the slowest single composition script). Time to a ready preview is unchanged, as expected.