Repository navigation
perf(producer): the probe skips the still-frame check unless its session captures - #7525
Merged
miguel-heygen merged 3 commits intoNov 11, 2026
Conversation
…ck files at their base
Contributor
There was a problem hiding this comment.
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) setsdeferStaticDedup: truebut noonStaticDedupActivity. OnlybuildCaptureOptions(renderOrchestrator.tsL3824) andrenderChunk.tsset it. prepareCaptureSessionForReusedoesn't replacesession.options. So whenrunSequentialDiskCapture(captureStage.ts~L502) reuses the probe session,armDeferredStaticDedupruns with the probe's options andverifyStaticFramesSafenever calls the callback.- That call runs inside the
frameStallRulewindow (renderOrchestrator.tsL4922/L5499). The window isfirstFrameWindowMs(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 iscapture_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
watchStallrefactor behaves as before.activityMovedonly resets the silence clock and doesn't changebudgetMs, 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.closeCaptureSessionis idempotent, so a later close is safe. staticDedupDeferredis cleared before the await, so a secondprepareCaptureSessionForReusewon't run the check again.- The "fails without the fix" line names four break checks with specific tests.
…as stall-watcher activity
miguel-heygen
merged commit Nov 11, 2026
8dbc55a
into
perf/static-dedup-verify-once
22 checks passed
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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 withprobeSession ?? openSession()and then callprepareCaptureSessionForReuse(captureStage.ts,captureStreamingStage.ts,captureSegmentedStage.ts). Calibration opens its own session and only closes the probe's (render/captureCost.ts).renderOrchestrator.tsreads the probe page (element count, adapters) and closes it. No path captures frames with the probe session withoutprepareCaptureSessionForReuse. A path that did would only lose dedup (slower, same output).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 inrender/shared.ts.How
CaptureOptions.deferStaticDedupmakesarmStaticDedupstop after its motion-blur classification, which still runs at init as before, and mark the sessionstaticDedupDeferred. The check itself moved intoarmStaticDedupFrames, unchanged.armDeferredStaticDedupruns it for a deferred session;prepareCaptureSessionForReusecalls it and is now async (its synchronous resets still happen before the first await).CaptureOptions.onStaticDedupActivityis called after every still-frame check screenshot (including spot checks).deferStaticDedup: true.buildCaptureOptionsand the chunk's capture options count check screenshots per render withnoteStaticDedupActivity.frameStallRulereads that count asactivity: a change restarts the silence clock without changing the window or the reported progress, so the error still saysstuck 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.Test plan
deferStaticDedup, the probe stage test fails. Without the activity branch inwatchStall,keeps the first-frame window open while bounded check work goes on, and stalls once it stopsfails. Without the call inprepareCaptureSessionForReuse,runs when the session is reused to capturefails. Without the screenshot callback,reports every check screenshot to onStaticDedupActivityfails.gives each parallel worker its own browser and the probe session to one workercaught 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.frameCapture-staticDedupDefer.test.ts3/3,observability.test.ts23/23,probeStage.test.ts74/74. EngineframeCapture*andparallelCoordinator*: 471. Producer under vitest (renderOrchestrator*,observability): 409. Under bun: segmented and streaming stages 88, probe stage andrenderChunk111. Typecheck,oxfmt --check,oxlintand the comment ratchet pass.data-duration, rendered to PNG. With 6 workers it is byte-identical toHF_STATIC_DEDUP=false(1,800 frames). With 1 worker, which reuses the probe session and runs the deferred check there (deferred, 5366msin the log), it is byte-identical toHF_STATIC_DEDUP=falsewith 1 worker, and to the 6-worker render. The 1-worker render took 26 s with dedup and 124 s without.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.