Skip to content

perf(core): a jump starts its video first and rebuilds the clip list after the frame lands - #5761

Merged
miguel-heygen merged 1 commit into
perf/playback-scales-with-screenfrom
perf/jump-paints-first
Oct 18, 2026
Merged

miguel-heygen merged 1 commit into
perf/playback-scales-with-screenfrom
perf/jump-paints-first

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

What changes for the user

Jumping around a long film costs less work: in the 12-jump test, the page now does about 18% less main-thread work per jump, and the jump median improves about 5% (843-874 ms -> 801-859 ms on a 9-minute test film). This is the first of three seek steps; jumps are still bound by video decoding and the page's layout, which the next PRs address (preview proxies, then an instant still).

What this PR does

  1. The video is asked first. A paused jump used to render the whole page at the target (every registered timeline, scene discovery, overlays) and only then set the on-screen video's time. Now the media sync runs first, so decoding overlaps the page work. Measured in the page: the video's time is set 2-4 ms after the jump reaches the runtime.
  2. The clip list is rebuilt after the frame lands. A jump that mounts scenes near its target triggered a full clip-list rebuild (all clips, with computed-style reads up each clip's ancestors), twice per jump: once from the scene mount and once from the transport tick. It cost 90-430 ms per jump inside the seek. While a jump's seek completions are pending, postTimeline now holds the post and sends one after they settle and the frame paints. The hold applies only after a jump: at load, posts (and window.__clipManifest) land at once, and render capture is never held.

seek-dispatch gains two read-only helpers (hasPendingSeekCompletion, seekCompletionsSettled), so the hold never consumes a failure that waitForSeekCompletion reports.

Tests

init.seekOrder.test.ts. Each test fails without its change; I checked each by removing the change and rerunning:

  • On a paused jump, the on-screen video's time is set before the page's timeline is seeked.
  • A jump with a pending completion posts no clip list until the completion lands, then posts exactly one.
  • With a completion pending but no jump, the post lands at once. This one fails if the hold applies to any pending completion.

Core runtime suite: 2099 tests pass. Typecheck, lint and the comment ratchet pass. The new tests pass 3 runs in a row.

Measurement

Setup:

  • Headless Chrome on a shared 8-core Linux box, load 0.5-3.9.
  • 12 jumps per run, plus playback from 0:00, 3:00 and 8:00 to check nothing regressed.
  • Whole CLI builds swapped, and the served runtime checked by hash on every run.
  • Alternating with the base.
Run Jump median / p95 (12 of 12 land) CPU for 12 jumps Playback fps 0:00 / 3:00 / 8:00
base, run 1 874 / 1183 ms 5.5 s 55.3 / 51.8 / 54.2
base, run 3 843 / 1206 ms 5.5 s 56.0 / 56.5 / 47.0
this PR, run 1 812 / 1181 ms 4.5 s 57.3 / 54.7 / 56.2
this PR, run 2 859 / 1219 ms 4.8 s 47.5 / 54.2 / 55.5
this PR, run 3 801 / 1192 ms 4.4 s 52.0 / 51.5 / 55.8

One base run was lost to a bug in my test rig: it clicked Play before Studio enabled the button (Play stays disabled for 0.2-1.9 s after the preview reports its duration). Fixed in the rig; not a Studio issue.

What a jump still spends, per profile:

  • The video's own decode: 0.1-1.2 s. The test media has keyframes every ~8 s and is decoded in software.
  • About 300 ms of page layout after the jump.
  • Rendering the newly mounted scenes.

Stacking

Stacked on #5754, which is stacked on #5741. Measured on a build that also includes the held-scenes stack (#5719, #5722, #5726): the hold covers the post at the end of nestRanScenes without changing that code.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at d7f172d9, only this PR's own diff against perf/playback-scales-with-screen.

Media first. syncMediaForCurrentState() now runs before seekTimelineAndAdapters on a paused jump, and stays skipped on the keepPlaying path as before. mediaForceSyncNextTick is still set, so media in a scene that the page seek mounts gets synced on the next tick.

Clip-list hold.

  • postTimeline holds only when all of these are true: a paused jump is landing, render capture hasn't started, and a completion is pending. Load-time posts and render capture are never held, and the third test pins the "no jump" case.
  • Holds made during one jump collapse into one post. seekCompletionsSettled re-reads the pending set on each pass, so a second jump that lands mid-hold is covered by the same post.
  • registerSeekCompletion wraps every promise so that it can't reject. That means Promise.all in seekCompletionsSettled always settles, and _pendingFailure is still left for waitForSeekCompletion to report.
  • The post after the hold re-checks state.tornDown and goes back through postTimeline. If playback started in the meantime, the playback hold still applies.

Non-blocking. The hold lasts as long as the slowest pending completion. The media and proxy completions have caps (media.ts, mediaProxy.ts HOLD_CAP_MS), but frameSources.ts:114/143 registers source.ready and source.seek() as they are. A frame source that never settles would therefore hold the clip list until some later post finds nothing pending. Before this change that list posted at once. A cap at the hold, or on those registrations, would bound it.

The tests cover each change. No live run; I checked the source and CI only.

— Rames

@miguel-heygen
miguel-heygen force-pushed the perf/jump-paints-first branch from d7f172d to 9183736 Compare October 17, 2026 07:21

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 91837362. git range-diff shows the patch is identical to d7f172d9, which I approved. It's only rebased onto #5754's fix (73de4fd8).

No live run.

— Rames

@miguel-heygen
miguel-heygen force-pushed the perf/playback-scales-with-screen branch from 73de4fd to d7824e1 Compare October 17, 2026 12:32
@miguel-heygen
miguel-heygen force-pushed the perf/jump-paints-first branch from 9183736 to 816b8d5 Compare October 17, 2026 12:32

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding at 816b8d50. The code is fine; the problem is CI.

git range-diff shows the patch is identical to the one I approved at 91837362 (91837362 = 816b8d50), so the code review stands.

The hold: the base is perf/playback-scales-with-screen. At this head only Preflight and the Perf jobs run, and the required contexts (Typecheck, Build, Test, runtime contract, the Windows jobs, Studio captures, the edit accuracy gate) haven't run. I'll approve once this PR runs them green against main.

— Rames

@miguel-heygen
miguel-heygen force-pushed the perf/playback-scales-with-screen branch from d7824e1 to 24cfc0f Compare October 17, 2026 14:56
@miguel-heygen
miguel-heygen force-pushed the perf/jump-paints-first branch from 816b8d5 to f12f0f1 Compare October 17, 2026 14:56

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at f12f0f1c. Range-diff against 816b8d50: this PR's own commit is unchanged; the stack below it gained #5741's c7d2f864 and #5754's matching isDiscoveryStamped carry into the cached scene clock (reviewed on #5754).

Holding with a comment because the base is still perf/playback-scales-with-screen (#5754, open), so the required contexts haven't run at this head. I'll approve once it targets main with required CI green.

No live run.

— Rames

@miguel-heygen
miguel-heygen force-pushed the perf/playback-scales-with-screen branch from 24cfc0f to 9165514 Compare October 17, 2026 16:25
@miguel-heygen
miguel-heygen force-pushed the perf/jump-paints-first branch from f12f0f1 to 41e3850 Compare October 17, 2026 16:28

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 41e3850e. Range-diff against f12f0f1c: this PR's commit is unchanged, and the stack below it is #5754 rebased onto main now that #5741 has merged.

Still holding with a comment: the base is still perf/playback-scales-with-screen (#5754, open), so required CI hasn't run at this head. For the record, my review at f12f0f1c was a COMMENT, not an approval. I'll approve once #5754 merges and this PR is on main with required CI green.

No live run.

— Rames

@miguel-heygen
miguel-heygen force-pushed the perf/jump-paints-first branch from 41e3850 to 4ec9d26 Compare October 17, 2026 17:44
@miguel-heygen miguel-heygen reopened this Oct 18, 2026
@miguel-heygen
miguel-heygen merged commit fdc40d8 into perf/playback-scales-with-screen Oct 18, 2026
54 checks passed
@miguel-heygen
miguel-heygen deleted the perf/jump-paints-first branch October 18, 2026 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants