Skip to content

perf(producer): the probe skips the still-frame check unless its session captures - #7525

Merged
miguel-heygen merged 3 commits into
perf/static-dedup-verify-oncefrom
perf/probe-defers-static-dedup
Nov 11, 2026
Merged

miguel-heygen merged 3 commits into
perf/static-dedup-verify-oncefrom
perf/probe-defers-static-dedup

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

The probe no longer runs the still-frame dedup check at start-up. It only runs it if a stage later reuses the probe's session to capture. And time spent inside the bounded still-frame check now counts as activity for the capture stall watcher, so a capture browser running that check is not taken for a stuck one.

Stacked on #7521 (check once per page and share the result).

Why

On a long, mostly still composition the check costs up to its 300 s budget. The probe paid it even in a parallel render, where the probe's session never captures a frame and is closed before capture. That was the largest single wait before the first frame.

Moving the check out of the probe changes what the capture stall watcher sees. Its first-frame window is sized from the probe's measured start-up, which until now included the probe's check. The capture browsers still run a check (the first one on the final page, with #7521). With a short probe start-up, that check plus page load can outlast the window and be reported as capture_disk stalled ... (stuck at 0). The check is bounded by its own wall budget, so counting its work as activity keeps the window honest without a second copy of that budget. A check that hangs still stops at its budget, after which the normal window applies.

Related work

  • The probe's session reaches capture only through prepareCaptureSessionForReuse (packages/engine/src/services/frameCapture.ts), which now runs the deferred check. Searched for every use of the probe session in the producer. The disk, streaming and segmented stages take it with probeSession ?? openSession() and then call prepareCaptureSessionForReuse (captureStage.ts, captureStreamingStage.ts, captureSegmentedStage.ts). Calibration opens its own session and only closes the probe's (render/captureCost.ts). renderOrchestrator.ts reads the probe page (element count, adapters) and closes it. No path captures frames with the probe session without prepareCaptureSessionForReuse. A path that did would only lose dedup (slower, same output).
  • The watcher change sits in watchStall (packages/producer/src/services/render/observability.ts), beside the existing progress and tail handling. The per-render counter sits beside the worker-phase map in render/shared.ts.

How

  • Engine: CaptureOptions.deferStaticDedup makes armStaticDedup stop after its motion-blur classification, which still runs at init as before, and mark the session staticDedupDeferred. The check itself moved into armStaticDedupFrames, unchanged. armDeferredStaticDedup runs it for a deferred session; prepareCaptureSessionForReuse calls it and is now async (its synchronous resets still happen before the first await).
  • Engine: CaptureOptions.onStaticDedupActivity is called after every still-frame check screenshot (including spot checks).
  • Producer: the probe's options set deferStaticDedup: true. buildCaptureOptions and the chunk's capture options count check screenshots per render with noteStaticDedupActivity. frameStallRule reads that count as activity: a change restarts the silence clock without changing the window or the reported progress, so the error still says stuck at <frames>.
  • watchStall's tick moved its activity check and its trip handling (abort the open attempt, or fail the stage) into two named helpers, so the tick stays under the repo's complexity limit. Behaviour is the same; the existing stall tests pass unchanged.
  • Segmented stage: the probe session is now released before awaiting its reuse, since its workers open together and only one may borrow it. If that reuse fails, the borrowed session is closed.

Test plan

  • fails without the fix, four break checks (each restored afterwards). Without the probe's deferStaticDedup, the probe stage test fails. Without the activity branch in watchStall, keeps the first-frame window open while bounded check work goes on, and stalls once it stops fails. Without the call in prepareCaptureSessionForReuse, runs when the session is reused to capture fails. Without the screenshot callback, reports every check screenshot to onStaticDedupActivity fails.
  • The existing segmented-stage test gives each parallel worker its own browser and the probe session to one worker caught a race in the first version of this change, where two workers took the probe session. Fixed by releasing it before the await; it passes now.
  • New tests 3 runs in a row: frameCapture-staticDedupDefer.test.ts 3/3, observability.test.ts 23/23, probeStage.test.ts 74/74. Engine frameCapture* and parallelCoordinator*: 471. Producer under vitest (renderOrchestrator*, observability): 409. Under bun: segmented and streaming stages 88, probe stage and renderChunk 111. Typecheck, oxfmt --check, oxlint and the comment ratchet pass.
  • Start-up on a 28-minute synthetic composition (51,558 frames, mostly still, alpha, webm, 6 workers). fix(producer): long, mostly still films reach their first frame before the stall check stops them #7521 and this branch were run back to back on the same machine. Probe start-up fell from 150 s to 0.9 s and the first frame from 314 s to 175 s. Screenshots before the first frame fell from 4,329 to 2,181; main takes 15,036.
  • The check's budget inside capture: a 28-minute composition with 5,000 tweens, whose check runs into its 300 s budget. With the probe now starting in 1.4 s the first-frame window is its 300 s floor. The first worker's check ran 300 s inside the capture stage and frames flowed at 306 s, with no stall.
  • Output unchanged: a 60-second composition with no root data-duration, rendered to PNG. With 6 workers it is byte-identical to HF_STATIC_DEDUP=false (1,800 frames). With 1 worker, which reuses the probe session and runs the deferred check there (deferred, 5366ms in the log), it is byte-identical to HF_STATIC_DEDUP=false with 1 worker, and to the 6-worker render. The 1-worker render took 26 s with dedup and 124 s without.
  • Not run: the full engine and producer suites (CI runs them), a render with calibration enabled, an HDR or layered render, a distributed render.

No visible change

Output frames are unchanged (byte-equal, above). Start-up order, the dedup log lines and the time before the first frame change.

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

Request changes at c8f293c5. One blocker. The rest looks right.

Blocker: when the probe session is reused, the deferred check sends no activity, so a sequential render of a long, mostly still film can be falsely reported as stalled.

  • The probe's captureOpts (probeStage.ts, around L439) sets deferStaticDedup: true but no onStaticDedupActivity. Only buildCaptureOptions (renderOrchestrator.ts L3824) and renderChunk.ts set it.
  • prepareCaptureSessionForReuse doesn't replace session.options. So when runSequentialDiskCapture (captureStage.ts ~L502) reuses the probe session, armDeferredStaticDedup runs with the probe's options and verifyStaticFramesSafe never calls the callback.
  • That call runs inside the frameStallRule window (renderOrchestrator.ts L4922/L5499). The window is firstFrameWindowMs(browserProbeMs). With the probe now starting in about 1 s, that is the 300 s floor.
  • A deferred check that runs to its own 300 s budget, plus seek and first capture, can therefore pass the window with staticDedupActivity(job) still flat. The result is capture_disk stalled ... (stuck at 0). Before this PR, that check ran in the probe and widened the window (2× start-up), so the same render passed.
  • This is the failure the PR describes. It is fixed for parallel workers, which build their options with the callback, but not for the 1-worker reuse path. The PR's 300 s budget run used 6 workers. The 1-worker run was a 60 s film with a 5 s check, so it didn't cover this path.

Suggested fix: add onStaticDedupActivity: () => noteStaticDedupActivity(job) to the probe's captureOpts, or have prepareCaptureSessionForReuse take the callback. Then add a test that reuses a deferred probe session and checks that the callback fires (frameCapture-staticDedupDefer.test.ts builds the session already).

Checked, fine:

  • The stack is on perf/static-dedup-verify-once (#7521).
  • The watchStall refactor behaves as before. activityMoved only resets the silence clock and doesn't change budgetMs, the reported progress, or the tail handling.
  • Segmented stage: the probe session is now cleared before the await, which closes the double-borrow race. If reuse fails, the borrowed session is closed. closeCaptureSession is idempotent, so a later close is safe.
  • staticDedupDeferred is cleared before the await, so a second prepareCaptureSessionForReuse won't run the check again.
  • The "fails without the fix" line names four break checks with specific tests.

@miguel-heygen
miguel-heygen merged commit 8dbc55a into perf/static-dedup-verify-once Nov 11, 2026
22 checks passed
@miguel-heygen
miguel-heygen deleted the perf/probe-defers-static-dedup branch November 11, 2026 00:50
@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Folded into #7521: these commits, plus the reused-session activity fix from this review, are on that branch. Nothing from here reaches main until #7521 merges.

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