Repository navigation
perf(producer): drawElement films keep their parallel workers instead of dropping to one - #6633
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head f324c663
-
blocker — required
CI › Producer: unit testsfails at this head with 6 tests that pin the inversion the new default turns off:renderOrchestrator.deGateProbe.test.ts: "a comp drawElement accepts still inverts…" (expected 4 to be 1,:174) and "a comp the engine refuses at init keeps the resolver's workers…" (expected [] to have a length of 1).renderOrchestrator.segmentedDeRetry.test.ts: all 4 "retries the segmented stage once on a screenshot session after …" cases (expected null to be truthy).
These suites still test a path you can turn back on with
HF_DE_SINGLE_MIN_FRAMES. SetHF_DE_SINGLE_MIN_FRAMES=900in them (and restore it afterwards), or rewrite them for the new default. The test plan only ranrenderOrchestrator.test.ts. -
nit — the
shouldPreferSingleWorkerDrawElementJSDoc (renderOrchestrator.ts:~2226) still says DE-single "beats screenshot-parallel at every worker count" above 900 frames. That now contradicts the new default; point it at the measurement in this PR.
Edit accuracy: accurate 271 (base branch 271), smooth 197 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What
Long drawElement-eligible films no longer drop from parallel capture to a single drawElement worker on machines the 3-worker drawElement router doesn't cover (under 24 GB, or when the router is off). They keep the parallel screenshot workers the resolver sized, about 25% faster.
Why
The single-worker "priority inversion" (
shouldPreferSingleWorkerDrawElement, films of 900+ frames) bet that one drawElement worker beats parallel screenshot capture. Measured today, it loses on the machines where it fires:Apple M4 Pro (14 cores, 24 GB), published
hyperframes@0.8.210, the router made ineligible withHF_DE_PARALLEL_MIN_MEM_MB=999999(what a 16 GB Mac gets), compared with and withoutHF_DE_SINGLE_MIN_FRAMES=0. That knob is the same switch this PR flips by default. Interleaved, medians.Peak memory rises to what parallel screenshot capture uses for any comp drawElement refuses on the same machine: 4,131 MB for the same 42 s film with one mix-blend layer, on main. The resolver's existing memory and heap caps still size the workers, and the low-memory profile still pins 8 GB machines to one worker.
Why under 100 lines
One lever: the inversion's frame floor already had an off switch; this flips its default. The rest is the tests that exercised the inversion opting back in.
Related work
Refs #5945.
How
resolveDeSingleMinFrames(env)owns the inversion's frame floor and defaults it to 0, the inversion's documented off switch.HF_DE_SINGLE_MIN_FRAMES=<n>turns it back on. The short-comp band derives from the same floor, so it is off too. The 3-worker drawElement router (24 GB+ machines) is unchanged.Test plan
HF_DE_SINGLE_MIN_FRAMESsets one, and the inversion predicate does not fire at the default.renderOrchestrator.test.ts > shouldPreferSingleWorkerDrawElement (DE priority inversion) > is off unless HF_DE_SINGLE_MIN_FRAMES sets a floor(a default of 900 fails exactly this test, 1 failed / 289 passed).renderOrchestrator.deGateProbe.test.tsandrenderOrchestrator.segmentedDeRetry.test.tsexercise the inversion, so they now setHF_DE_SINGLE_MIN_FRAMES=900(the opt-in): 3 and 7 passed.tsc --noEmit(producer) exit 0;renderOrchestrator.test.ts290 passed; oxfmt and oxlint clean.Output: frames come from screenshot capture instead of drawElement, as they already do for every film under 900 frames. PSNR between the two paths: 52.5 dB average (34.9 dB worst frame) on the 42 s film, 54.5 dB on the 5 minute film. Two screenshot runs of the same film differ by about 52 dB on their own.