From 0749cd9ff83ad41c72c7dc6be1b224e9bc27731b Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 29 Jul 2026 02:36:40 -0700 Subject: [PATCH] feat(producer): open the DE single-worker inversion to short comps under an element ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 31% of fleet renders (24h, v0.7.78+) are DE-eligible comps clamped to parallel screenshot purely because they sit under the 900-frame inversion floor — the median fleet render is ~250-600 frames, below every DE entry threshold. This opens a 250-899 frame band, gated on composition size. Measured, not assumed. A controlled sweep (fixed synthetic content, {250,400,600,900}f, single-DE vs parallel-screenshot-W4, 3 reps, capture mode verified per row, AC power, load-gated) showed single-DE winning 1.16-1.24x at every size — but only for content in constant motion. A follow-up 2x2 found motion and DOM size pull in OPPOSITE directions, so neither alone predicts the winner (ratio = ss4/de1, >1 means DE wins): 24 movers / 0 nodes -> 1.05 320 movers / 0 nodes -> 1.24 320 movers / 7000 nodes -> 1.09 24 movers / 7000 nodes -> 0.96 24 movers / 20000 nodes -> 0.71 24 movers / 40000 nodes -> 0.55 DE's wall-clock scales ~0.50ms/element against parallel screenshot's ~0.22ms — drawElement repaints the whole tree per frame while fan-out amortizes it — so the downside is NOT bounded and a bare floor drop would have handed a 1.8x regression to large comps. Since motion only ever helps DE, an element ceiling calibrated at the lowest-motion case is safe at every motion level; crossover there is ~3.9k, and the default sits at 2500. The predicate is untouched; the call site picks the floor. Above the ceiling, or at 900+ frames, behaviour is bit-identical to today — the change can only add inversions in the new band, never remove one. Instrumentation, since this ships at full exposure rather than cohorted: `composition_element_count` on EVERY render (the fleet distribution of the gate variable is unknown — without it we cannot tell whether 2500 opens the band for most short comps or almost none, nor re-derive the threshold from real content), and `de_short_band` = applied | skipped_elements, unset when the frame count made the band irrelevant, so a fleet perf shift is attributable to this change rather than to content mix. Safety is unchanged and already proven on this path: per-frame PSNR self-verify with screenshot fallback, exactly as the 900+ band has shipped default-on. Knobs: HF_DE_SHORT_MIN_FRAMES, HF_DE_SHORT_MAX_ELEMENTS (0 disables the band). Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/commands/render.ts | 2 + packages/cli/src/telemetry/events.ts | 8 ++ .../cli/src/telemetry/renderObservability.ts | 2 + .../src/services/render/observability.ts | 20 ++++ .../src/services/render/perfSummary.ts | 6 + .../src/services/renderOrchestrator.test.ts | 104 ++++++++++++++++++ .../src/services/renderOrchestrator.ts | 84 +++++++++++++- 7 files changed, 225 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index 5c8fb7531..6dcb77be8 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -1433,6 +1433,8 @@ function trackRenderMetrics( deClampReason: perf?.drawElement?.clampReason, deWorkerInversion: perf?.drawElement?.workerInversion, dePreInversionWorkers: perf?.drawElement?.preInversionWorkers, + compositionElementCount: perf?.drawElement?.compositionElementCount, + deShortBand: perf?.drawElement?.shortBand, deParallelRouter: perf?.drawElement?.parallelRouter, dePreRouterWorkers: perf?.drawElement?.preRouterWorkers, deGateReason: perf?.drawElement?.gateReason, diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index cd6874756..b0d37aabb 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -72,6 +72,8 @@ export interface RenderObservabilityTelemetryPayload { // the more authoritative perfSummary value wins when both are present. captureDeWorkerInversion?: string; captureDePreInversionWorkers?: number; + captureCompositionElementCount?: number; + captureDeShortBand?: string; captureDeParallelRouter?: string; captureDeGpuRenderer?: string; captureDePreRouterWorkers?: number; @@ -130,6 +132,8 @@ function renderObservabilityEventProperties(props: RenderObservabilityTelemetryP capture_memory_exhaustion_detected: props.captureMemoryExhaustionDetected, de_worker_inversion: props.captureDeWorkerInversion, de_pre_inversion_workers: props.captureDePreInversionWorkers, + composition_element_count: props.captureCompositionElementCount, + de_short_band: props.captureDeShortBand, de_parallel_router: props.captureDeParallelRouter, gpu_renderer: props.captureDeGpuRenderer, de_pre_router_workers: props.captureDePreRouterWorkers, @@ -204,6 +208,8 @@ export function trackRenderComplete( deClampReason?: string; deWorkerInversion?: string; dePreInversionWorkers?: number; + compositionElementCount?: number; + deShortBand?: string; deParallelRouter?: string; dePreRouterWorkers?: number; deGateReason?: string; @@ -303,6 +309,8 @@ export function trackRenderComplete( de_clamp_reason: props.deClampReason, de_worker_inversion: props.deWorkerInversion, de_pre_inversion_workers: props.dePreInversionWorkers, + composition_element_count: props.compositionElementCount, + de_short_band: props.deShortBand, de_parallel_router: props.deParallelRouter, de_pre_router_workers: props.dePreRouterWorkers, de_gate_reason: props.deGateReason, diff --git a/packages/cli/src/telemetry/renderObservability.ts b/packages/cli/src/telemetry/renderObservability.ts index 87554d693..10a1ad22f 100644 --- a/packages/cli/src/telemetry/renderObservability.ts +++ b/packages/cli/src/telemetry/renderObservability.ts @@ -42,6 +42,8 @@ export function renderObservabilityTelemetryPayload( captureMemoryExhaustionDetected: capture.memoryExhaustionDetected, captureDeWorkerInversion: capture.deWorkerInversion, captureDePreInversionWorkers: capture.dePreInversionWorkers, + captureCompositionElementCount: capture.compositionElementCount, + captureDeShortBand: capture.deShortBand, captureDeParallelRouter: capture.deParallelRouter, captureDeGpuRenderer: capture.deGpuRenderer, captureDePreRouterWorkers: capture.dePreRouterWorkers, diff --git a/packages/producer/src/services/render/observability.ts b/packages/producer/src/services/render/observability.ts index 39111960c..dfe0bc2a4 100644 --- a/packages/producer/src/services/render/observability.ts +++ b/packages/producer/src/services/render/observability.ts @@ -70,6 +70,26 @@ export interface RenderCaptureObservability { deWorkerInversion?: "inverted" | "reverted"; /** Worker count the resolver would have used absent the inversion; undefined if it never fired. */ dePreInversionWorkers?: number; + /** + * Rough element count of the compiled composition (`countElementTags`). + * + * Emitted on every render, not just inverted ones — this is the variable the + * short-comp inversion band is gated on, and the fleet distribution of it is + * unknown. Without it there is no way to tell whether the 2500 ceiling opens + * the band for most short comps or almost none, and no way to re-derive the + * threshold from real content instead of synthetic sweeps. + */ + compositionElementCount?: number; + /** + * Why the short-comp band did or did not apply to this render: + * "applied" (frames landed in 250-899 and the element count cleared the + * ceiling), "skipped_elements" (band was open by frame count but the comp + * was too large), or undefined when the frame count made the band + * irrelevant either way. Distinguishes "the new routing chose this" from + * "the pre-existing 900 floor chose this" — otherwise a fleet perf shift is + * unattributable. + */ + deShortBand?: "applied" | "skipped_elements"; /** DE parallel-router outcome: "routed" (fired, held) | "reverted" (fired, self-verify retry rolled back). */ deParallelRouter?: "routed" | "reverted"; /** diff --git a/packages/producer/src/services/render/perfSummary.ts b/packages/producer/src/services/render/perfSummary.ts index fe72e1e1c..0770d58ee 100644 --- a/packages/producer/src/services/render/perfSummary.ts +++ b/packages/producer/src/services/render/perfSummary.ts @@ -80,6 +80,10 @@ export interface DrawElementPerfInput { workerInversion?: "inverted" | "reverted"; /** Auto-resolved worker count before the inversion pinned it to 1 (set only when the inversion fired). */ preInversionWorkers?: number; + /** Rough compiled-composition element count — gate variable for the short-comp inversion band. */ + compositionElementCount?: number; + /** Short-comp band attribution: "applied" | "skipped_elements"; unset when the band was irrelevant. */ + shortBand?: string; parallelRouter?: "routed" | "reverted"; /** Auto-resolved worker count before the router pinned it to 3 (set only when the router fired). */ preRouterWorkers?: number; @@ -121,6 +125,8 @@ function aggregateDrawElement( clampReason: de.clampReason, workerInversion: de.workerInversion ?? "none", preInversionWorkers: de.preInversionWorkers, + compositionElementCount: de.compositionElementCount, + shortBand: de.shortBand, parallelRouter: de.parallelRouter ?? "none", preRouterWorkers: de.preRouterWorkers, gateReason: gateReasons.length > 0 ? gateReasons.join("|") : undefined, diff --git a/packages/producer/src/services/renderOrchestrator.test.ts b/packages/producer/src/services/renderOrchestrator.test.ts index 791a17389..16536f213 100644 --- a/packages/producer/src/services/renderOrchestrator.test.ts +++ b/packages/producer/src/services/renderOrchestrator.test.ts @@ -34,6 +34,8 @@ import { resolveParallelRouterRetryPlan, resetCaptureAttemptProgress, shouldRetryViaPinnedFallback, + countElementTags, + envInt, shouldPreferParallelDrawElement, shouldPreferSingleWorkerDrawElement, shouldStreamParallelCapture, @@ -1693,6 +1695,108 @@ describe("shouldPreferSingleWorkerDrawElement (DE priority inversion)", () => { expect(shouldPreferSingleWorkerDrawElement(eligible)).toBe(true); }); + // ── Short-comp band ──────────────────────────────────────────────────── + // The band lowers the effective floor from 900 to 250 for SMALL comps only. + // The predicate itself is unchanged — the call site picks the floor — so + // these pin the arithmetic the call site performs. + // + // Measured basis (400f, single-DE vs parallel-screenshot-W4, 2 reps, ratio + // = ss4/de1 so >1 means DE wins): + // 24 movers / 0 nodes -> 1.05 DE wins + // 320 movers / 0 nodes -> 1.24 DE wins + // 320 movers / 7000 nodes -> 1.09 DE wins + // 24 movers / 7000 nodes -> 0.96 DE LOSES + // 24 movers / 20000 nodes -> 0.71 DE loses badly + // 24 movers / 40000 nodes -> 0.55 DE loses very badly + // Motion helps DE, DOM size punishes it; the ceiling is calibrated at the + // lowest-motion case so every higher-motion comp is covered too. + describe("short-comp band floor arithmetic", () => { + const shortBandFloor = (elementCount: number, maxElements = 2500): number => + elementCount <= maxElements ? Math.min(900, 250) : 900; + + it("opens the 250-frame floor for a small comp", () => { + expect(shortBandFloor(800)).toBe(250); + expect( + shouldPreferSingleWorkerDrawElement({ + ...eligible, + totalFrames: 400, + minFrames: shortBandFloor(800), + }), + ).toBe(true); + }); + + it("keeps the 900-frame floor for a large comp — the measured 1.8x regression case", () => { + expect(shortBandFloor(40000)).toBe(900); + expect( + shouldPreferSingleWorkerDrawElement({ + ...eligible, + totalFrames: 400, + minFrames: shortBandFloor(40000), + }), + ).toBe(false); + }); + + it("never RAISES the floor: a large comp at 900+ frames still inverts as it did before", () => { + expect( + shouldPreferSingleWorkerDrawElement({ + ...eligible, + totalFrames: 2380, + minFrames: shortBandFloor(40000), + }), + ).toBe(true); + }); + + it("leaves comps below the band floor alone", () => { + expect( + shouldPreferSingleWorkerDrawElement({ + ...eligible, + totalFrames: 200, + minFrames: shortBandFloor(800), + }), + ).toBe(false); + }); + }); + + describe("countElementTags", () => { + it("counts closing tags", () => { + expect(countElementTags("
a
")).toBe(2); + }); + + it("undercounts void elements — biases the count DOWN, so the ceiling must stay conservative", () => { + expect(countElementTags("
")).toBe(0); + }); + + it("is stable on empty and malformed input rather than throwing", () => { + expect(countElementTags("")).toBe(0); + expect(countElementTags("<<<>>>")).toBe(0); + }); + + it("scales to a large document without a full parse", () => { + expect(countElementTags("

x

".repeat(40000))).toBe(40000); + }); + }); + + describe("envInt", () => { + afterEach(() => { + delete process.env.HF_TEST_ENV_INT; + }); + + it("falls back when unset, empty, or non-numeric — a typo must not disable a guard", () => { + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + process.env.HF_TEST_ENV_INT = ""; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + process.env.HF_TEST_ENV_INT = "lots"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(2500); + }); + + it("reads an explicit value, including 0 as a real disable", () => { + process.env.HF_TEST_ENV_INT = "700"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(700); + process.env.HF_TEST_ENV_INT = "0"; + expect(envInt("HF_TEST_ENV_INT", 2500)).toBe(0); + }); + }); + it("honors explicitly requested workers", () => { expect(shouldPreferSingleWorkerDrawElement({ ...eligible, requestedWorkers: 3 })).toBe(false); }); diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index 232bc29ac..2369bf2c0 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -492,6 +492,10 @@ export interface RenderPerfSummary { workerInversion?: string; /** Worker count the auto-resolution chose BEFORE the inversion pinned it to 1 — the parallel counterfactual for speedup math. Only set when the inversion fired. */ preInversionWorkers?: number; + /** Rough compiled-composition element count — the variable the short-comp inversion band is gated on. Always set. */ + compositionElementCount?: number; + /** Short-comp band attribution: "applied" | "skipped_elements"; unset when the frame count made the band irrelevant. */ + shortBand?: string; /** DE parallel-router outcome: "routed" (fired, held), "reverted" (fired, self-verify retry rolled back), "none". Mutually exclusive with workerInversion. */ parallelRouter?: string; /** Worker count the auto-resolution chose BEFORE the router pinned it to 3 — the single-worker-inversion counterfactual. Only set when the router fired. */ @@ -1222,6 +1226,37 @@ export function shouldUseStreamingEncode( return workerCount === 1; } +/** + * Integer tuning knob from the environment. Matches the convention the + * surrounding DE thresholds already use: unset OR set-but-empty falls back to + * the default (a blank var is not a kill switch), and so does anything + * non-numeric — a typo must never silently disable a routing guard. + */ +export function envInt(name: string, fallback: number): number { + const raw = process.env[name]; + if (raw === undefined || raw.trim() === "") return fallback; + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : fallback; +} + +/** + * Rough element count for compiled composition HTML. + * + * Deliberately a string scan and not a `parseHTML` + `querySelectorAll` (the + * `countAuthoredTimedClips` approach): this runs on EVERY render before the + * routing decision, and a full linkedom parse of the exact documents that + * matter here — the 20k-40k node ones — is the most expensive case. Precision + * is not needed. It feeds a threshold whose measured crossover is ~3.9k and + * whose default sits at 2500, so counting closing tags is comfortably inside + * the margin. Undercounts void elements (``, `
`) and self-closing + * SVG nodes, which biases the count DOWN — the direction that opens the band + * — so the ceiling is the thing to keep conservative. + */ +export function countElementTags(html: string): number { + const matches = html.match(/<\/[a-zA-Z]/g); + return matches === null ? 0 : matches.length; +} + /** * DE priority inversion predicate: should an AUTO-resolved multi-worker render * drop to single-worker verified drawElement streaming? @@ -2405,6 +2440,38 @@ async function executeRenderPipeline(input: { ? 900 : Number(deSingleMinFramesRaw); const deSingleMinFrames = Number.isFinite(deSingleMinFramesNum) ? deSingleMinFramesNum : 900; + // Short-comp band: 31% of fleet renders (24h, 0.7.78+) are DE-eligible + // comps clamped to parallel screenshot purely because they sit under this + // floor. A controlled sweep (fixed synthetic content, {250,400,600,900}f, + // single-DE vs parallel-screenshot-W4, 3 reps, capture modes verified per + // run) showed single-DE winning 1.16-1.24x at EVERY size — but only for + // content in constant motion. A follow-up 2x2 (movers x static DOM nodes) + // found the two variables pull in opposite directions: motion favours DE, + // DOM size punishes it, and DE's wall-clock scales ~0.50ms/node against + // parallel screenshot's ~0.22ms (drawElement repaints the whole tree per + // frame; fan-out amortizes it). At 24 movers / 400f the measured curve is + // +5% for DE at 0 nodes, -4% at 7k, -41% at 20k, -80% at 40k — crossover + // near ~3.9k. Since motion only ever helps DE, a node ceiling calibrated + // at the LOWEST-motion case is safe for every motion level, so the short + // band opens only below `deShortBandMaxElements`. Above it the original + // 900 floor stands, unchanged. + const deShortBandMinFrames = envInt("HF_DE_SHORT_MIN_FRAMES", 250); + const deShortBandMaxElements = envInt("HF_DE_SHORT_MAX_ELEMENTS", 2500); + const compositionElementCount = countElementTags(compiled.html); + const deShortBandOpen = + deShortBandMinFrames > 0 && + deShortBandMaxElements > 0 && + compositionElementCount <= deShortBandMaxElements; + const deEffectiveMinFrames = deShortBandOpen + ? Math.min(deSingleMinFrames, deShortBandMinFrames) + : deSingleMinFrames; + // Does the band even matter for this render? Only when the frame count + // falls in the newly-opened window — at or above the original floor the + // inversion fires regardless, and below `deShortBandMinFrames` nothing + // fires either way. Keeps the telemetry reason from claiming credit (or + // blame) on renders the change could not have affected. + const deShortBandApplies = + totalFrames >= deShortBandMinFrames && totalFrames < deSingleMinFrames; // "Would ANY multi-worker resolution be inverted?" — if workers resolve // to 1 naturally the outcome is identical either way. const WOULD_RESOLVE_MULTI_WORKER = 2; @@ -2416,7 +2483,7 @@ async function executeRenderPipeline(input: { forceScreenshot: captureForceScreenshot, outputFormat, totalFrames, - minFrames: deSingleMinFrames, + minFrames: deEffectiveMinFrames, singleWorkerStreamingOk: shouldUseStreamingEncode(cfg, outputFormat, 1, job.duration), layeredOrEffectRoute: hasHdrContent || compiled.hasShaderTransitions, supersampling: deviceScaleFactor > 1, @@ -2678,6 +2745,15 @@ async function executeRenderPipeline(input: { // any resource-pressure failure unique to this cohort. dePreInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, dePreRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, + // Short-comp band attribution — see the field docs. Emitted on every + // render so the fleet element-count distribution is readable, and so a + // perf shift can be split into "the new band did it" vs "unchanged". + compositionElementCount, + deShortBand: deShortBandApplies + ? deShortBandOpen + ? "applied" + : "skipped_elements" + : undefined, // Same rationale as the counters above: carried on live capture // observability, not only the success-path perfSummary, so a crash / // OOM / timeout still reports which GPU backend it happened on. That @@ -3524,6 +3600,12 @@ async function executeRenderPipeline(input: { clampReason: deClampReason, workerInversion: deWorkerInversion, preInversionWorkers: deWorkerInversion ? preRoutingWorkerCount : undefined, + compositionElementCount, + shortBand: deShortBandApplies + ? deShortBandOpen + ? "applied" + : "skipped_elements" + : undefined, parallelRouter: deParallelRouter, preRouterWorkers: deParallelRouter ? preRoutingWorkerCount : undefined, selfVerifyFallback: deSelfVerifyFallback,