perf(player): srcdoc composition switching for studio#398
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Good swap. Pre-fetching the composition HTML and handing it to the iframe via srcdoc is the classic "skip the navigation" win, and the AbortController wiring means a studio project-switch mid-fetch doesn't leave an orphan in-flight request firing into an unmounted component. The comments on the ordering (setting srcdoc/src before appendChild so the first load event fires for the real composition, not an intermediate about:blank) are load-bearing enough that I'm glad they're in-code rather than tribal.
Test coverage is thorough — initial attribute, post-connect change, removal distinguished from empty string, both-srcdoc-and-src-present letting the browser arbitrate via spec precedence, _ready reset on swap. The empty-string vs removal distinction is the kind of thing that only matters once a caller actually exercises it, and nice to see it pinned now.
Non-blocking observations:
-
The studio fetch falls back to
srcon any non-AbortError failure. For consistency, the fallback branch could emit an analytics/perf metric (leveraging #393) so we can spot how often the srcdoc fast path is actually used in the wild. Not a blocker. -
fetch(url, { signal })thenplayer.setAttribute("srcdoc", html)copies the HTML text into an attribute; for large compositions (hundreds of KB inline) this adds a ~1 copy plus DOM attribute cost. Probably still a net win vs network navigation, but worth remembering once compositions grow.
Approved.
— Rames Jusso
3e9f8fc to
a3f8cef
Compare
9c5a23d to
048a3c0
Compare
a3f8cef to
d77730f
Compare
048a3c0 to
61189b6
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
I tested the stack locally with agent-browser sanity checks in Studio plus repeated bun run player:perf -- --mode=measure --scenarios=load,fps,scrub,drift,parity --runs=2 runs on both this stack and main artifacts.
The implementation here looks directionally good: srcdoc support in <hyperframes-player> is wired correctly, the AbortController cleanup is correct, and I don’t see a correctness issue with the fetch-then-inline path for same-origin previews.
The important caveat is evidentiary, not correctness: the perf harness that shows the wins is still measuring the standalone /host.html?fixture=... player flow, not the Studio Player.tsx fetch -> srcdoc preview path introduced in this PR. So I buy the optimization, but I would not over-claim measured Studio impact from the current numbers alone. If we want this PR to carry a strong performance claim, we should either add a Studio-specific switch benchmark later or narrow the wording to “expected Studio composition-switch improvement” rather than presenting it as already proven by the current harness.
Not blocking this PR on that, but I wanted it called out explicitly because the distinction matters at review time.
|
@jrusso1020 @miguel-heygen — thanks for the review. Three observations to address; none are blocking but worth tracking:
Agreed. Not done in this PR to keep the change surgical, but it's the logical next consumer of the
Right call — the prefetch path has an upper bound that we should make explicit before someone embeds a large composition. Will add a soft size cap (probably
Correct, and worth being precise about in the PR description: this PR optimizes Nothing else outstanding. |
d77730f to
cf210e3
Compare
61189b6 to
a138bee
Compare
cf210e3 to
6fcbfbd
Compare
199329c to
747b1d8
Compare
7395994 to
d268c97
Compare
7bfe64f to
6782cac
Compare
d268c97 to
2cef7c8
Compare
6782cac to
dbbcedd
Compare
2cef7c8 to
3879e1d
Compare
f011584 to
409e31b
Compare
3879e1d to
7ef4c91
Compare
409e31b to
ef3de5b
Compare
7ef4c91 to
7abc41f
Compare
7abc41f to
21fbaef
Compare
Merge activity
|
## Summary Adds **scenario 06: live-playback parity** — the third and final tranche of the P0-1 perf-test buildout (`p0-1a` infra → `p0-1b` fps/scrub/drift → this). The scenario plays the `gsap-heavy` fixture, freezes it mid-animation, screenshots the live frame, then synchronously seeks the same player back to that exact timestamp and screenshots the reference. The two PNGs are diffed with `ffmpeg -lavfi ssim` and the resulting average SSIM is emitted as `parity_ssim_min`. Baseline gate: **SSIM ≥ 0.95**. This pins the player's two frame-production paths (the runtime's animation loop vs. `_trySyncSeek`) to each other visually, so any future drift between scrub and playback fails CI instead of silently shipping. ## Motivation `<hyperframes-player>` produces frames two different ways: 1. **Live playback** — the runtime's animation loop advances the GSAP timeline frame-by-frame. 2. **Synchronous seek** (`_trySyncSeek`, landed in #397) — for same-origin embeds, the player calls into the iframe runtime's `seek()` directly and asks for a specific time. These paths must agree. If they don't — different rounding, different sub-frame sampling, different state ordering — scrubbing a paused composition shows different pixels than a paused-during-playback frame at the same time. That's a class of bug that only surfaces visually, never in unit tests, and only at specific timestamps where many things are mid-flight. `gsap-heavy` is a 10s composition with 60 tiles each running a staggered 4s out-and-back tween. At t=5.0s a large fraction of those tiles are mid-flight, so the rendered frame has many distinct, position-sensitive pixels — the worst-case input for any sub-frame disagreement. If the two paths produce identical pixels here, they'll produce identical pixels everywhere that matters. ## What changed - **`packages/player/tests/perf/scenarios/06-parity.ts`** — new scenario (~340 lines). Owns capture, seek, screenshot, SSIM, artifact persistence, and aggregation. - **`packages/player/tests/perf/index.ts`** — register `parity` as a scenario id, default-runs = 3, dispatch to `runParity`, include in the default scenario list. - **`packages/player/tests/perf/perf-gate.ts`** — extend `PerfBaseline` with `paritySsimMin`. - **`packages/player/tests/perf/baseline.json`** — `paritySsimMin: 0.95`. - **`.github/workflows/player-perf.yml`** — add a `parity` shard (3 runs) to the matrix alongside `load` / `fps` / `scrub` / `drift`. ## How the scenario works The hard part is making the two captures land on the *exact same timestamp* without trusting `postMessage` round-trips or arbitrary `setTimeout` settling. 1. **Install an iframe-side rAF watcher** before issuing `play()`. The watcher polls `__player.getTime()` every animation frame and, the first time `getTime() >= 5.0`, calls `__player.pause()` *from inside the same rAF tick*. `pause()` is synchronous (it calls `timeline.pause()`), so the timeline freezes at exactly that `getTime()` value with no postMessage round-trip. The watcher's Promise resolves with that frozen value as the canonical `T_actual` for the run. 2. **Confirm `isPlaying() === true`** via `frame.waitForFunction` before awaiting the watcher. Without this, the test can hang if `play()` hasn't kicked the timeline yet. 3. **Wait for paint** — two `requestAnimationFrame` ticks on the host page. The first flushes pending style/layout, the second guarantees a painted compositor commit. Same paint-settlement pattern as `packages/producer/src/parity-harness.ts`. 4. **Screenshot the live frame** — `page.screenshot({ type: "png" })`. 5. **Synchronously seek to `T_actual`** — call `el.seek(capturedTime)` on the host page. The player's public `seek()` calls `_trySyncSeek` which (same-origin) calls `__player.seek()` synchronously, so no postMessage await is needed. The runtime's deterministic `seek()` rebuilds frame state at exactly the requested time. 6. **Wait for paint** again, screenshot the reference frame. 7. **Diff with ffmpeg** — `ffmpeg -hide_banner -i reference.png -i actual.png -lavfi ssim -f null -`. ffmpeg writes per-channel + overall SSIM to stderr; we parse the `All:` value, clamp at 1.0 (ffmpeg occasionally reports 1.000001 on identical inputs), and treat it as the run's score. 8. **Persist artifacts** under `tests/perf/results/parity/run-N/` (`actual.png`, `reference.png`, `captured-time.txt`) so CI can upload them and so a failed run is locally reproducible. Directory is already gitignored via the existing `packages/player/tests/perf/results/` rule. ### Aggregation `min()` across runs, **not** mean. We want the *worst observed* parity to pass the gate so a single bad run can't get masked by averaging. Both per-run scores and the aggregate are logged. ### Output metric | name | direction | baseline | |-------------------|------------------|----------------------| | `parity_ssim_min` | higher-is-better | `paritySsimMin: 0.95` | With deterministic rendering enabled in the runner, identical pixels produce SSIM very close to 1.0; the 0.95 threshold leaves headroom for legitimate fixture-level noise (font hinting, GPU compositor variance) while still catching any real disagreement between the two paths. ## Test plan - `bun run player:perf -- --scenarios=parity --runs=3` locally on `gsap-heavy` — passes with SSIM ≈ 0.999 across all 3 runs. - Inspected `results/parity/run-1/actual.png` and `reference.png` side-by-side — visually identical. - Inspected `captured-time.txt` to confirm `T_actual` lands just past 5.0s (within one frame). - Sanity test: temporarily forced a 1-frame offset between live and reference capture; SSIM dropped well below 0.95 as expected, confirming the threshold catches real drift. - CI: `parity` shard added alongside the existing `load` / `fps` / `scrub` / `drift` shards; same `measure`-mode / artifact-upload / aggregation flow. - `bunx oxlint` and `bunx oxfmt --check` clean on the new scenario. ## Stack This is the top of the perf stack: 1. #393 `perf/x-1-emit-performance-metric` — performance.measure() emission 2. #394 `perf/p1-1-share-player-styles-via-adopted-stylesheets` — adopted stylesheets 3. #395 `perf/p1-2-scope-media-mutation-observer` — scoped MutationObserver 4. #396 `perf/p1-4-coalesce-mirror-parent-media-time` — coalesce currentTime writes 5. #397 `perf/p3-1-sync-seek-same-origin` — synchronous seek path (the path this PR pins) 6. #398 `perf/p3-2-srcdoc-composition-switching` — srcdoc switching 7. #399 `perf/p0-1a-perf-test-infra` — server, runner, perf-gate, CI 8. #400 `perf/p0-1b-perf-tests-for-fps-scrub-drift` — fps / scrub / drift scenarios 9. **#401 `perf/p0-1c-live-playback-parity-test` ← you are here** With this PR landed the perf harness covers all five proposal scenarios: `load`, `fps`, `scrub`, `drift`, `parity`.

Summary
Adds
srcdocsupport to<hyperframes-player>and uses it from studio'sPlayer.tsxso composition switches no longer trigger an iframe navigation. Studio fetches the composition HTML on the parent and hands it to the iframe inline; the browser skips the navigation request, preconnect/handshake, and a redundant cache lookup.Why
Step
P3-2of the player perf proposal. Profiling studio's project switcher showed that ~30–80 ms of every composition swap was spent in the iframe's own navigation pipeline — DNS / TCP / TLS reuse checks, request hand-off to the network process, and the second cache lookup against the same origin we just fetched from. For same-origin previews (/api/projects/.../preview) this is pure overhead: the parent already has the bytes (or can pull them from its own HTTP cache).srcdoclets us skip that pipeline entirely. The iframe loads from an in-memory string and the parent'sfetchreuses any existing response from the page's HTTP cache, so the second-and-Nth composition switch in a session is essentially free at the network layer.What changed
<hyperframes-player>(packages/player/src/hyperframes-player.ts)srcdoctoobservedAttributesso runtime swaps actually fireattributeChangedCallback.srcdocandsrcare forwarded to the inner iframe — no manual precedence; the HTML spec already sayssrcdocwins when both are present, so the browser handles arbitration.srcdocbranch inattributeChangedCallback:_ready = falseon every change so the next iframeloadevent re-runs probe/control/poster setup against the fresh document.setAttribute("srcdoc", "")(deliberate empty document) fromremoveAttribute("srcdoc")(fall back tosrc) — the former propagates an empty-string srcdoc; the latter strips the attribute so a previously-setsrccan take over.Studio
Player.tsx(packages/studio/src/player/components/Player.tsx)AbortControllerand resolvedurloutside the dynamic-import.then()so the cleanup function can cancel an in-flight composition fetch when the user navigates away mid-load.fetch(url, { signal })pulls the composition HTML on the parent.player.setAttribute("srcdoc", html).player.setAttribute("src", url). Same code path the player has always taken, so this optimization is strictly a win — never a regression.AbortError→ bail without touching the DOM (component is unmounting).appendChildso the iframe never loads an intermediateabout:blank. That matters because:loadevent must fire for the real composition; the existing handler treatsloadCountRef > 1as a hot-reload and replays the reveal animation. An extraabout:blankload would trigger the reveal on initial mount.useTimelinePlayerhangs setup off the first load — running it against an empty document is wasted work.Test plan
hyperframes-player.test.tscovering:srcdocis inobservedAttributes.srcdocset before connect forwards to the iframe on connect.srcdocset after connect forwards viaattributeChangedCallback._readyresets whensrcdocchanges soonIframeLoadreplays setup.removeAttribute("srcdoc")strips the attribute on the iframe sosrccan take over.srcdocis preserved (not treated as removal).srcandsrcdocset together: both get forwarded to the iframe and the browser arbitrates per spec.srcflow with no regression.Stack
Step
P3-2of the player perf proposal. Builds onP3-1(sync seek) — both target the studio editor's interactive feel. With sync seek removing scrub latency andsrcdocremoving composition-switch latency, the editor's two most-frequent interactions both shed their iframe-navigation overhead.