Skip to content

fix(core): a paused preview lets go of the scenes it has unmounted - #7195

Merged
miguel-heygen merged 2 commits into
mainfrom
cloudperf/scene-window-memory
Nov 5, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
cloudperf/scene-window-memory

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What

A preview that plays or scrubs past several scenes kept the unmounted ones in memory: after playing, until playback ran again; after a paused scrub, for good. The scene window swaps each scene that leaves it for an empty placeholder, but three runtime caches still pointed at the old scene elements, so every element of those scenes stayed alive.

This is also the merge-queue failure in coreRuntimeBrowser.test.ts › lets go of each scene the preview mounts and unmounts… (expected 4695 to be less than 800): about 800 nodes is one scene, so that run held five.

Why

Memory in a long preview grows with every scene passed and is only given back when playback runs again, or never after a paused scrub. The test that guards this fails about half the time under load, which ejects unrelated PRs from the merge queue.

Related work

None.

How

The first two are rebuilt only on their next read, and a stopped transport reads nothing:

  • queryForTransport's cache (selector → elements) is now emptied in invalidateCompositionTimingCaches, which every timing change already goes through, unmounting a scene included. The revision stamp per entry is gone, since a cleared cache only ever holds current results.
  • The never-styled start cache is keyed by a WeakMap, so it holds no element.
  • The set of clips not yet styled visible was only emptied of removed clips on a playing tick, so a paused scrub kept every scene it passed. Unmounting a scene now drops its clips from the set, and a clip no longer in the page is never added (a visibility pass can still be handed a list from before the unmount).

The test gains a paused-scrub case (same five scenes, seeked without playing), and its node count now collects garbage until the count stops falling (at most six rounds) before it reads. One collection sometimes leaves a freed scene unswept, about one run in fifteen with the fix; a scene that is still held survives every round.

Test plan

  • Unit tests added
  • Manual testing performed: the three cases of the test, run with every core of a 32-core box busy, on a heap snapshot taken at the failure, and with the fix reverted
    • Before: held scenes were reachable from the runtime's closure through the query cache's nodes array or the start cache's map. The count stayed flat across eight collections over 2 s, so nothing was waiting to be freed.
    • With the fix: 18 of 18 playing cases pass (6 runs × 3 cases); the paused case passes every run (6) holding nothing.
    • Paused scrub before this PR: all five scenes held (4586 nodes), every run. Without the set's prune on unmount: 4586. Without the page check when adding: one scene held (802).
    • Fix reverted, new test kept: 15 of 18 fail with 1207 to 4560 nodes held.
    • Whole coreRuntimeBrowser.test.ts: 82 of 82 pass. Core src/runtime tests: 2757 pass. oxfmt and oxlint are clean on both files.
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments"

Fails without the fix: lets go of each scene the preview mounts and unmounts, its captured audio included fails with expected 2811 to be less than 800 (also 1207, 2009, 4560) when init.ts is reverted and the test is kept, 15 of 18 loaded cases; lets go of each scene … (its timeline built as the script runs, scrubbed while paused) fails with expected 4586 to be less than 800 every run.

Size

A lone runtime fix: one memory leak through three element caches, and the queue flake it causes, small enough to revert by itself, with nothing open on the same code to carry it.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 109 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 (8)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approve at 81efde98. No blockers.

  • Fails without the fix: The line is present. It names both cases of lets go of each scene the preview mounts and unmounts… with their failure counts: 15 of 18 playing runs fail and the paused scrub fails every run when init.ts is reverted.
  • queryCache: Dropping the per-entry revision and clearing the cache in invalidateCompositionTimingCaches gives the same results as before. Every path that bumps compositionTimingRevision goes through that function (the observer callback, takeRecords in queryForTransport, the media-event handlers, withoutOwn*Records and disconnect). A cleared map only ever holds results computed since the last invalidation. When the observer is off or a render capture has started, the query still bypasses the cache as before.
  • neverStyledStarts → WeakMap: It's only ever read with get/set and replaced whole when the revision changes. Nothing iterates it, so the switch is safe.
  • clipsNeverStyledVisible: The prune in retireScene covers the paused case, which the playing-tick prune (clock.isPlaying() guard) never reached. The isConnected check on add closes the stale-list path. The prune is O(set size) per unmount, which is fine.
  • Test: Collecting garbage until the count stops falling doesn't weaken the check, because a scene that is still held survives every round. The added paused case is the one that reproduced on every run.

Nit (doesn't block): kept() returns next when next >= count, so a count that rises between rounds is reported at the higher value. That's conservative, just noting it.

CI at this head when I reviewed: PR captures and Comments had passed. CI, regression, Windows render verification and the rest were still pending or running, after earlier runs on this SHA were cancelled and requeued.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit e2a6535 Nov 5, 2026
173 of 175 checks passed
@miguel-heygen
miguel-heygen deleted the cloudperf/scene-window-memory branch November 5, 2026 02:10
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.

1 participant