From f635deb86a75782567deb0524eb76350b9961e6c Mon Sep 17 00:00:00 2001 From: James Date: Wed, 6 May 2026 17:20:14 +0000 Subject: [PATCH] feat(engine): cache probe Promise + log resolved mode + sync docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from Vai's staff-eng review: 1. Concurrent-probe race (real bug): the parallel coordinator runs N workers via Promise.all, so `--workers 4` on a no-GPU host fired 4 simultaneous probe Chromes — each paying the same 240 ms launch cost. Cache the *Promise* (not the resolved value): first caller assigns the in-flight Promise, every other concurrent caller awaits the same one. Verified with a new test asserting all concurrent callers get the identical Promise reference. 2. Stale rendering.md (lines 23, 29): user-visible contract said "browser GPU enabled by default", which was wrong post-auto. Now describes the auto / hardware / software trichotomy explicitly. 3. Silent fallback: auto-mode produced no output, so a regression to "always falls back to software even with GPU present" would have been invisible in production logs. Added a single stderr line per process when the probe resolves: `[hyperframes] browserGpuMode auto → ()`. Cache hits don't re-log. Verification: - Engine 536/536 (incl. new concurrent-dedup test asserting Promise reference equality across simultaneous callers) - CLI 256/256 - Format / lint / typecheck clean --- packages/cli/src/docs/rendering.md | 4 +- .../src/services/browserManager.test.ts | 23 ++++ .../engine/src/services/browserManager.ts | 128 ++++++++++-------- 3 files changed, 100 insertions(+), 55 deletions(-) diff --git a/packages/cli/src/docs/rendering.md b/packages/cli/src/docs/rendering.md index 15a4c44f5..c2b49c466 100644 --- a/packages/cli/src/docs/rendering.md +++ b/packages/cli/src/docs/rendering.md @@ -20,13 +20,13 @@ Requires: Docker installed and running. - `--crf` — Override encoder CRF (mutually exclusive with `--video-bitrate`) - `--video-bitrate` — Target video bitrate such as `10M` (mutually exclusive with `--crf`) - `--gpu` — Use GPU encoding (NVENC, VideoToolbox, VAAPI, QSV) -- `--browser-gpu` / `--no-browser-gpu` — Use or opt out of host GPU acceleration for local Chrome/WebGL capture (enabled by default for local renders, disabled in Docker) +- `--browser-gpu` / `--no-browser-gpu` — Force host GPU or software (SwiftShader) for Chrome/WebGL capture. Default for local renders is `auto` — probe WebGL availability on first launch and fall back to software if no GPU is reachable. Docker mode always uses software. - `-o, --output` — Custom output path ## Tips - Use `draft` quality for fast previews during development -- Local renders use browser GPU capture automatically; use `--no-browser-gpu` to compare against the software-browser path +- Local renders auto-detect GPU on first launch; use `--browser-gpu` to force hardware (errors if no GPU) or `--no-browser-gpu` to force SwiftShader - Use `--gpu` when a local render also benefits from hardware FFmpeg encoding - Use `npx hyperframes benchmark` to find optimal settings - 4 workers is usually the sweet spot for most compositions diff --git a/packages/engine/src/services/browserManager.test.ts b/packages/engine/src/services/browserManager.test.ts index 4e6827ead..08af34802 100644 --- a/packages/engine/src/services/browserManager.test.ts +++ b/packages/engine/src/services/browserManager.test.ts @@ -103,6 +103,29 @@ describe("resolveBrowserGpuMode", () => { const third = await resolveBrowserGpuMode("hardware"); expect(third).toBe("hardware"); }); + + it("deduplicates concurrent auto-mode probes by caching the in-flight Promise", async () => { + // Parallel coordinator fires N workers via Promise.all — without Promise- + // level caching, a `--workers 4` render against a no-GPU host would launch + // 4 simultaneous probe Chromes. Verify all concurrent callers get the + // exact same Promise reference (proving the probe runs once, not N times). + const p1 = resolveBrowserGpuMode("auto", { + chromePath: "/definitely/not/a/real/chrome/binary", + browserTimeout: 2000, + }); + const p2 = resolveBrowserGpuMode("auto", { + chromePath: "/definitely/not/a/real/chrome/binary", + browserTimeout: 2000, + }); + const p3 = resolveBrowserGpuMode("auto", { + chromePath: "/definitely/not/a/real/chrome/binary", + browserTimeout: 2000, + }); + expect(p1).toBe(p2); + expect(p2).toBe(p3); + const results = await Promise.all([p1, p2, p3]); + expect(results).toEqual(["software", "software", "software"]); + }); }); describe("forceReleaseBrowser", () => { diff --git a/packages/engine/src/services/browserManager.ts b/packages/engine/src/services/browserManager.ts index e5fb48615..714b9a938 100644 --- a/packages/engine/src/services/browserManager.ts +++ b/packages/engine/src/services/browserManager.ts @@ -137,15 +137,18 @@ async function probeBeginFrameSupport(browser: Browser): Promise { } /** - * Cached result of `resolveBrowserGpuMode("auto", ...)` for the lifetime of - * this process. The probe launches a transient Chrome with hardware args and - * checks whether `canvas.getContext("webgl")` returns a context. Result is - * memoised because the answer is a property of the host (GPU/driver - * availability) and cannot meaningfully change inside one process. + * Cached *in-flight or resolved* probe Promise for `resolveBrowserGpuMode("auto", ...)`. + * + * Caching the Promise (rather than the resolved value) deduplicates concurrent + * callers — the parallel coordinator runs N workers via `Promise.all`, so a + * `--workers 4` render against a no-GPU host would otherwise fire 4 + * simultaneous probe Chromes. The first call assigns the Promise and every + * other concurrent caller awaits the same one, paying the ~240 ms probe cost + * exactly once per process lifetime. * * Exported for tests; production callers go through `resolveBrowserGpuMode`. */ -export let _autoBrowserGpuModeCache: "software" | "hardware" | undefined; +export let _autoBrowserGpuModeCache: Promise<"software" | "hardware"> | undefined; /** Test-only: reset the cached probe result. */ export function _resetAutoBrowserGpuModeCacheForTests(): void { @@ -158,8 +161,8 @@ export function _resetAutoBrowserGpuModeCacheForTests(): void { * For `"software"` / `"hardware"` this is a pure pass-through. For `"auto"` * it launches a tiny Chrome with the platform's hardware GPU args, runs a * one-shot WebGL availability probe, and falls back to `"software"` if - * hardware-mode WebGL is unavailable. The probe result is cached for the - * process lifetime — a multi-render run pays the ~1-2 s cost once. + * hardware-mode WebGL is unavailable. The Promise is cached for the process + * lifetime, so concurrent callers (parallel workers) share the same probe. * * Any failure (Chrome launch error, navigation timeout, missing canvas API, * etc.) is treated as a `"software"` fallback. The render path with @@ -167,7 +170,7 @@ export function _resetAutoBrowserGpuModeCacheForTests(): void { * safe failure mode; misclassifying toward hardware would error on the real * render. */ -export async function resolveBrowserGpuMode( +export function resolveBrowserGpuMode( mode: EngineConfig["browserGpuMode"], options: { chromePath?: string; @@ -175,58 +178,77 @@ export async function resolveBrowserGpuMode( platform?: NodeJS.Platform; } = {}, ): Promise<"software" | "hardware"> { - if (mode !== "auto") return mode; - if (_autoBrowserGpuModeCache !== undefined) return _autoBrowserGpuModeCache; + if (mode !== "auto") return Promise.resolve(mode); + if (_autoBrowserGpuModeCache) return _autoBrowserGpuModeCache; - const platform = options.platform ?? process.platform; - const browserTimeout = options.browserTimeout ?? DEFAULT_CONFIG.browserTimeout; - const executablePath = options.chromePath ?? resolveHeadlessShellPath({}); + _autoBrowserGpuModeCache = (async () => { + const platform = options.platform ?? process.platform; + const browserTimeout = options.browserTimeout ?? DEFAULT_CONFIG.browserTimeout; + const executablePath = options.chromePath ?? resolveHeadlessShellPath({}); - const probeArgs = [ - "--no-sandbox", - "--disable-setuid-sandbox", - "--disable-dev-shm-usage", - "--enable-webgl", - "--ignore-gpu-blocklist", - ...getBrowserGpuArgs("hardware", platform), - ]; + const probeArgs = [ + "--no-sandbox", + "--disable-setuid-sandbox", + "--disable-dev-shm-usage", + "--enable-webgl", + "--ignore-gpu-blocklist", + ...getBrowserGpuArgs("hardware", platform), + ]; - const ppt = await getPuppeteer().catch(() => null); - if (!ppt) { - _autoBrowserGpuModeCache = "software"; - return _autoBrowserGpuModeCache; - } + const ppt = await getPuppeteer().catch(() => null); + if (!ppt) { + logResolvedBrowserGpuMode("software", "puppeteer unavailable"); + return "software" as const; + } - let probeBrowser: Browser | undefined; - try { - probeBrowser = await ppt.launch({ - headless: true, - args: probeArgs, - defaultViewport: { width: 64, height: 64 }, - executablePath, - timeout: browserTimeout, - }); - const page = await probeBrowser.newPage(); - const hasWebGL = await page.evaluate(() => { - try { - const c = document.createElement("canvas"); - const gl = - c.getContext("webgl") || (c.getContext("experimental-webgl") as RenderingContext | null); - return gl !== null; - } catch { - return false; - } - }); - _autoBrowserGpuModeCache = hasWebGL ? "hardware" : "software"; - } catch { - _autoBrowserGpuModeCache = "software"; - } finally { - await probeBrowser?.close().catch(() => {}); - } + let probeBrowser: Browser | undefined; + try { + probeBrowser = await ppt.launch({ + headless: true, + args: probeArgs, + defaultViewport: { width: 64, height: 64 }, + executablePath, + timeout: browserTimeout, + }); + const page = await probeBrowser.newPage(); + const hasWebGL = await page.evaluate(() => { + try { + const c = document.createElement("canvas"); + const gl = + c.getContext("webgl") || + (c.getContext("experimental-webgl") as RenderingContext | null); + return gl !== null; + } catch { + return false; + } + }); + const resolved = hasWebGL ? ("hardware" as const) : ("software" as const); + logResolvedBrowserGpuMode(resolved, hasWebGL ? "WebGL probe succeeded" : "WebGL unavailable"); + return resolved; + } catch (err) { + logResolvedBrowserGpuMode( + "software", + `probe failed (${err instanceof Error ? err.message : String(err)})`, + ); + return "software" as const; + } finally { + await probeBrowser?.close().catch(() => {}); + } + })(); return _autoBrowserGpuModeCache; } +/** + * Single observability surface for the auto-detect outcome. Logged exactly + * once per process (the probe runs once); without this line, a regression + * to "always software even with a GPU present" would be invisible in + * production. Goes to stderr to stay out of stdout pipelines. + */ +function logResolvedBrowserGpuMode(resolved: "hardware" | "software", reason: string): void { + console.error(`[hyperframes] browserGpuMode auto → ${resolved} (${reason})`); +} + export async function acquireBrowser( chromeArgs: string[], config?: Partial<