fix(engine): parallelize forced screenshot workers (#1848)

This commit is contained in:
Miguel Ángel
2026-07-01 19:34:22 -07:00
committed by GitHub
parent 6be46813a2
commit 145c71e837
2 changed files with 70 additions and 12 deletions
@@ -4,6 +4,7 @@ import {
distributeFrames,
formatWorkerFailure,
selectWorkerDiagnostics,
shouldDisableBrowserPoolForParallelWorker,
shouldVerifyWorkerGpu,
} from "./parallelCoordinator.js";
import type { EngineConfig } from "../config.js";
@@ -76,6 +77,44 @@ describe("calculateOptimalWorkers", () => {
});
});
describe("shouldDisableBrowserPoolForParallelWorker", () => {
const linuxHeadlessWorker = {
parallel: true,
platform: "linux" as NodeJS.Platform,
deviceScaleFactor: 1,
headlessShellPath: "/tmp/chrome-headless-shell",
};
it.each([
["BeginFrame", false],
["forced screenshot", true],
])(
"disables the browser pool for parallel Linux/headless %s workers",
(_mode, forceScreenshot) => {
expect(
shouldDisableBrowserPoolForParallelWorker({
...linuxHeadlessWorker,
forceScreenshot,
}),
).toBe(true);
},
);
it.each([
["non-parallel", { parallel: false, forceScreenshot: true }],
["non-linux", { platform: "darwin" as NodeJS.Platform, forceScreenshot: true }],
["no headless shell", { headlessShellPath: undefined, forceScreenshot: true }],
["supersampled", { deviceScaleFactor: 2, forceScreenshot: false }],
])("keeps the shared pool for %s workers", (_case, overrides) => {
expect(
shouldDisableBrowserPoolForParallelWorker({
...linuxHeadlessWorker,
...overrides,
}),
).toBe(false);
});
});
describe("worker failure diagnostics", () => {
it("keeps only actionable worker diagnostics and caps the tail", () => {
const diagnostics = selectWorkerDiagnostics(
@@ -76,6 +76,15 @@ export interface WorkerSizingConfig extends Partial<
captureCostMultiplier?: number;
}
type WorkerBrowserPoolDecision = {
parallel?: boolean;
platform: NodeJS.Platform;
// Deliberately accepted but not used: forceScreenshot is not an exclusion.
forceScreenshot?: boolean;
deviceScaleFactor?: number;
headlessShellPath?: string;
};
const MEMORY_PER_WORKER_MB = 256;
const MIN_WORKERS = 1;
const MAX_WORKER_DIAGNOSTIC_LINES = 8;
@@ -98,6 +107,21 @@ function defaultSafeMaxWorkers(): number {
}
const MIN_FRAMES_PER_WORKER = 30;
// Linux/headless parallel workers need isolated browser processes: BeginFrame
// crashes when shared, while forceScreenshot is safe but serializes
// Page.captureScreenshot per browser. Supersampling keeps the existing path
// until browser-pool compatibility is keyed by DPR.
export function shouldDisableBrowserPoolForParallelWorker({
parallel,
platform,
deviceScaleFactor,
headlessShellPath,
}: WorkerBrowserPoolDecision): boolean {
return Boolean(
parallel && platform === "linux" && headlessShellPath && (deviceScaleFactor ?? 1) <= 1,
);
}
export function selectWorkerDiagnostics(
lines: readonly string[],
maxLines: number = MAX_WORKER_DIAGNOSTIC_LINES,
@@ -266,18 +290,13 @@ async function executeWorkerTask(
let session: CaptureSession | null = null;
let perf: CapturePerfSummary | undefined;
// BeginFrame's compositor is process-global — multiple pages driving
// beginFrame in the same browser race it and crash with "Target closed".
// Only disable the pool when BeginFrame mode would actually be active.
// Must match the predicate in createCaptureSession (frameCapture.ts):
// Linux + headless-shell + !forceScreenshot + !supersampling.
const supersampling = (captureOptions.deviceScaleFactor ?? 1) > 1;
const needsSeparateBrowsers =
parallel &&
process.platform === "linux" &&
!config?.forceScreenshot &&
!supersampling &&
resolveHeadlessShellPath(config) !== undefined;
const needsSeparateBrowsers = shouldDisableBrowserPoolForParallelWorker({
parallel,
platform: process.platform,
forceScreenshot: config?.forceScreenshot,
deviceScaleFactor: captureOptions.deviceScaleFactor,
headlessShellPath: resolveHeadlessShellPath(config),
});
const workerConfig: Partial<EngineConfig> | undefined = needsSeparateBrowsers
? { ...config, enableBrowserPool: false }
: config;