Repository navigation
perf(core): scene cuts stop re-reading every clip's position - #6311
miguel-heygen wants to merge 1 commit into
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 1 nit · head 53129a4b
- nit:
packages/core/src/runtime/init.ts:3637still says "leaf/in-flow caches may be stale" aboveinvalidateTimedClipCaches(), which now drops only the leaf cache.
Scope
Read the full diff and the visibility pass that calls isTimedClipInFlow (init.ts:3160), which is its only caller; invalidateTimedClipCaches (init.ts:3639) is the only invalidation site. A WeakMap keyed on the element already misses for a replaced element, so dropping the in-flow map on signature change bought nothing. Runtime writes to clip classes (__hf-flash, __hf-pick-highlight) or inline position (applyClipLayout) cost one re-read each, not one per frame. Open #6189 doesn't touch this code. Ran locally: the 3 new tests fail on main's init.ts and pass at this head; src/runtime has 98 files and 2,434 tests passing. CI was still running at review time, with no failures so far.
Edit accuracy did not run: 0 of 5 shards reported results; see the shard jobs.Nothing was compared with the base branch. |
|
Closing: no measurable frame win on the film this targeted. Longest frame of main-thread work from just before each scene cut to 0.5 s after it, ms, three runs per build. Whole 50 s 3840x2160 film at 1x, minified runtime, four builds on the same
This PR alone matches |
What
A scene cut in the preview no longer makes the runtime re-read every hidden clip's position. The cache of which timed clips sit in normal flow now survives changes to the set of timed elements and is refreshed only when the clip's own inline position or classes change.
Why
The visibility pass hides an in-flow timed clip with
display: noneand a positioned one withvisibility: hidden, and asksisTimedClipInFlowwhich one it is. That readsgetComputedStyle(el).position, which forces a style recalc, right after the pass wrote the clip'svisibility. The answer was cached per element, butpostTimelinedropped the whole cache whenevercomputeClipTreeSignaturechanged.Scene parking (#6189) changes that signature at every scene cut, so at each cut the pass re-read the position of every clip it hid, one forced recalc after another. On a 50 s 3840x2160 film with 7 scenes and 112 short video clips, that was the largest part of the task at the first cut.
The signature cannot tell the cache anything about position: an element's computed
positiondoes not change because other timed elements came or went, and a replaced element is a new object, so aWeakMapkeyed on it misses anyway. It changes when the element's own inlinepositionor classes change, which the old cache did not notice until the next signature change. The leaf cache does depend on the timed set (a clip gains or loses a nested[data-start]), so that one is still dropped.Scope next to #6189
This PR removes one forced read:
getComputedStyle(el).positioninisTimedClipInFlow(packages/core/src/runtime/init.ts), which the visibility pass repeated for every hidden timed clip after each change of the timed-element set, so at every scene cut once #6189 parks scenes. It does not touch #6189's own reads at a cut,getComputedStyle(host).positionandoffsetWidth/offsetHeightinfarScenes.ts(measureLeaving), and it does not touchfarScenes.tsat all. The per-cut numbers below are measured with this PR alone, #6189 alone and both, so neither claims the other's frames.How
style.positionand itsclassattribute, both read without a recalc. An entry is reused while those match and re-read when either changes. That covers the runtime's ownapplyClipLayoutwritingposition: absoluteand Studio edits that set position inline or through a class.invalidateTimedClipCachesnow drops only the leaf cache.Measured
That film, previewed in
<hyperframes-player>at 1x, headless Chrome 152 on an Apple silicon Mac with GPU compositing and rasterization. Base ismainwith #6189 at its current head on top; "after" adds this change. Each run started with the host's 1-minute load under 10, with builds in rotating order.Profiled (unminified runtime, CPU profiler on), playing 0.5 s to 10 s, three runs each:
isTimedClipInFlowin that task(program))What
isTimedClipInFlowstill costs at the cut is clips hidden for the first time, whose position has never been read.Without the profiler (minified runtime), playing 0 s to 9.8 s, six runs each in both orders: no film frames were missed either way, and the longest frame (18 to 30 ms in both builds) did not separate. The first scene's own frames are that long, so the cut is no longer the longest frame on this film either way.
Per cut, next to #6189
Longest frame of main-thread work from just before each scene cut to 0.5 s after it, whole film at 1x, minified runtime, no profiler. Four builds on the same
maincommit (main, this PR alone, #6189 alone, both), three rotations in changing order, each run started with the host's 1-minute load under 10. Values are the three runs, in ms.mainThis PR alone does not move any cut: without #6189 the timed set does not change at cuts. With #6189, adding it does not separate from #6189 alone at any cut, including 8.5 s; the profiled saving at that cut (above) is smaller than the run-to-run spread of the cut frame. The large frames at 23.17 s and 30.5 s in both #6189 columns are #6189's
measureLeavingreads and are not addressed here.Test plan
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
init.test.ts: after another timed clip is added (the clip tree signature changes), hiding three positioned clips again reads none of their computed styles (fails onmain: 3 reads). A clip whose inlinepositionchanges fromabsolutetostaticis taken out of the flow on its next hide (fails onmain: it keeps the stale positioned reading). A clip whose class change drops it from a positioning rule is taken out of the flow (fails with the class left out of the cache inputs).The new tests pass 3 runs in a row. Full
packages/core/src/runtimesuite: 98 files, 2,434 tests, exit 0. Pre-commit lint, format, typecheck and fallow pass;check-comment-citationsandcomment-ratchetpass.