Skip to content

perf(studio): parse saved HTML once when syncing timeline length - #5092

Merged
miguel-heygen merged 5 commits into
mainfrom
dbulk/parse-once
Oct 6, 2026
Merged

miguel-heygen merged 5 commits into
mainfrom
dbulk/parse-once

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What changes

Timeline moves and trims parsed the saved HTML twice while synchronizing the composition length. Read the furthest clip end and the root duration from one parsed document, and reuse that result on delete. Saved markup, duration rounding, empty-timeline behavior, and delete rollback remain unchanged.

The optional precomputed duration preserves null as a missing root. Zero or invalid content ends return without parsing.

Validation

At a0dd85d0 (main merged at 43d07781):

  • Each focused test file ran alone three consecutive times: timeline edit helpers 23 passed, root duration 13, asset drop 34, delete operations 11. Editing integration tests: 61 passed, including rollback after a failed delete save.
  • Mutations, each failing with an assertion: passing no precomputed root duration (two parses instead of one), reading the root duration before the zero-content check (four no-op cases parse), treating a missing root like an absent argument (??, two parses for a rootless composition), and returning a zero content end on delete (the length readout stays at 6 instead of 4).
  • Studio typecheck, changed-file lint and formatting, comment checks, comment ratchet, Fallow audit and the main-deletions guard pass.

Performance

Built Studio on Linux, the same launch project and one-clip timeline drag as the original measurement, 15 strictly alternating base/head rounds against 43d07781, each round started only below load 2.5. Each round drags the clip twice: the first edit after load, then the same clip again. Main-thread work is from CPU profiles of the release-to-settle window; a long task is a run of non-idle samples over 8 ms.

Measurement (median / p90 of 15) Base, first drag This PR, first drag Base, second drag This PR, second drag
Total main-thread work after release 296 / 306 ms 280 / 295 ms 253 / 340 ms 223 / 301 ms
Longest task 65 / 74 ms 51 / 66 ms 59 / 68 ms 37 / 57 ms
Time in tasks over 8 ms 207 / 216 ms 193 / 222 ms 175 / 244 ms 154 / 221 ms
Tasks over 8 ms 7 / 10 8 / 10 9 / 10 8 / 12
HTML parses for duration sync, deterministic witness 2 1 2 1

User-visible work is not worse: the medians and p90s for this PR are at or below base for total work and longest task on both drags. The earlier five-round run showed a single head round with more long-task time; across 15 rounds per side that does not reproduce. The one head round above base's p90 (first drag, round 9: 360 ms total, longest task 101 ms) spent that task in React rendering and live GSAP retiming, which this PR does not touch, and base has outliers of the same size (second drag, round 2: 428 ms total, longest 93 ms). The table does not claim a total-work saving from this change: the parse saving is about 9 ms, and round-to-round spread is larger than that.

Before

Parent build after moving card 13 under the playhead.

Before: card 13 after the timeline drop

After

Same move on the change (captured at 9350d4ac; the product code is unchanged since). Card placement and timeline match, and the saved HTML is byte-identical to the parent's.

After: matching card placement with one duration-sync parse

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 5, 2026 22:51
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 2055 (base branch 2055), smooth 1613 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@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.

Approving at a0dd85d0. Each length sync now parses the saved HTML once, and the result matches the old code.

Checked

  • One parse, used everywhere the length is worked out. syncCompositionDurationWithEnd parses once and reads both the furthest clip end and the root duration from that one document. syncCompositionDurationToContent wraps it, so move, resize, batch persist and link-edit all go through the single parse. Delete uses the same function and reuses contentEnd for the store readout, which before took a separate furthestClipEndFromSource parse.
  • Same result as before. I wrote a scratch test that runs the old path (furthestClipEndFromSource plus the 2-argument setCompositionDurationToContent) and the new path on 12 sources. The synced source and contentEnd matched on every one:
    • empty, invalid markup, and no root;
    • a root with no data-duration, and a root with data-duration="abc";
    • an empty timeline, and a clip with no duration;
    • a duration that needs rounding, and one that is already equal (4.00 vs 4);
    • single-quoted attributes;
    • a nested composition whose inner clip runs past the inner end.
  • No stale cache. Nothing is cached across calls. The document is parsed from the same string that is then patched, inside one call, so the parse can't be out of date.
  • Reuse. rootCompositionDurationOf(doc) is split out of readRootCompositionDuration rather than written again, and furthestClipEndFromDocument was already the shared helper. Removing furthestClipEndFromSource also drops its Fallow entry.

Non-blocking

  • setCompositionDurationToContent's optional rootDuration has three states: undefined reads the root itself, null means no root, and a number is used as given. The null-vs-undefined difference is easy to break later, though your mutation test catches ??. A one-line doc comment on the parameter would help.
  • Unrelated to this PR: extendCompositionDurationIfNeeded (timelineAssetDrop.ts) and extendRootDurationInSource (rootDuration.ts) are near copies of the same grow-only ratchet, differing only in rounding. They could be one helper someday.

Checks at a0dd85d0

  • The 4 changed test files pass 81 of 81. The full Studio suite passes 7634 tests, and tsc is clean.
  • CI: all 93 checks pass, and one is skipped.

— Rames

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit e8cfd6b Oct 6, 2026
169 checks passed
@miguel-heygen
miguel-heygen deleted the dbulk/parse-once branch October 6, 2026 23:29
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