mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(engine,producer): fix quadratic dedup rescan, correct race justification
Address two max-effort code-review findings on PR #2056 not covered by the earlier review-gap commit: - captureFrameToBufferPipelined's static-dedup reuse branch never advanced session.lastEncodeResultFrame, unlike its sibling real-capture branches. The gap-check window is computed from that watermark, so every consecutive reuse in a static run rescanned an ever-widening window instead of just the newest frame — O(n^2) total work over a long static stretch instead of O(n). - The "single-threaded, no race" justification on the shared parallelGuard closure was wrong: the guard has real internal await points (recapture, PSNR) between reading and writing its sizes/absFloor/acceptedSmall state, so concurrent workers' calls do interleave there (confirmed). Replaced with the actual reason it's safe: absFloor only ratchets down, sizes is append-only and order-independent for the median, and acceptedSmall's fast path re-validates by exact byte-equality regardless of which worker wrote the reference buffer.
This commit is contained in:
@@ -2677,6 +2677,11 @@ export async function captureFrameToBufferPipelined(
|
||||
if (gapStatic) {
|
||||
session.staticDedupCount = (session.staticDedupCount ?? 0) + 1;
|
||||
session.capturePerf.frames += 1;
|
||||
// Advance the watermark on reuse too, not just on real captures — the
|
||||
// gap-check above starts from lastEncodeResultFrame, so leaving it
|
||||
// pinned to the last REAL capture makes every consecutive reuse rescan
|
||||
// an ever-widening window instead of just the one new frame (review).
|
||||
session.lastEncodeResultFrame = frameIndex;
|
||||
return { encodeResult: session.lastEncodeResult, captureTimeMs: Date.now() - startTime };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -526,11 +526,18 @@ export async function runCaptureStreamingStage(
|
||||
// orchestrator's DrawElementVerificationError handler re-renders via
|
||||
// screenshot (post-#2026 it also reverts any worker inversion).
|
||||
// Intentionally ONE guard shared by all workers rather than one per
|
||||
// worker: Node is single-threaded and the guard's checks run
|
||||
// synchronously between await points, so there's no cross-worker race
|
||||
// on `parallelStats`. Sharing also means the rolling median it tracks
|
||||
// is computed across every worker's interleaved frames together,
|
||||
// which is a better signal than per-worker medians would be.
|
||||
// worker, so the rolling median it tracks is computed across every
|
||||
// worker's interleaved frames together — a better signal than
|
||||
// per-worker medians would be. This IS touched from concurrent workers
|
||||
// across real await points (recapture, PSNR), so the shared
|
||||
// `sizes`/`absFloor`/`acceptedSmall` state can interleave — safe by
|
||||
// construction rather than by single-threadedness: `absFloor` only
|
||||
// ratchets down (order-independent min), `sizes` is append-only (order
|
||||
// doesn't affect the median once ≥12 samples exist), and
|
||||
// `acceptedSmall`'s fast path only ever fires on exact byte-equality
|
||||
// against a buffer some worker already re-verified deterministic — so
|
||||
// whichever worker's buffer lands there, the equality check itself is
|
||||
// what re-validates it, not which worker wrote it (review).
|
||||
const parallelStats: DeDrainStats = {
|
||||
verifyChecked: 0,
|
||||
blankSuspects: 0,
|
||||
|
||||
Reference in New Issue
Block a user