From 3a0590925cd2724c49d6c9dba0440361f78f2d0c Mon Sep 17 00:00:00 2001 From: James Russo Date: Sun, 26 Jul 2026 22:42:12 -0700 Subject: [PATCH] perf(ci): run the two heaviest fixtures in distributed mode (#2825) * perf(ci): run the two heaviest fixtures in distributed mode * test(ci): pin distributed-mode fixtures to harness support --- .github/workflows/regression.yml | 6 +- .../scripts/plan-regression-shards.mjs | 44 +++++++++- .../src/regression-shard-plan.test.ts | 86 ++++++++++++++++++- packages/producer/tests/shard-schedule.json | 14 ++- 4 files changed, 141 insertions(+), 9 deletions(-) diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index c51111104..9a6290a8c 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -113,15 +113,17 @@ jobs: # concurrent exports cannot exhaust the Actions cache service. cache-to: ${{ github.event_name == 'push' && 'type=gha,mode=max,scope=regression-test-image' || '' }} - - name: "Run regression shard: ${{ matrix.shard }}" + - name: "Run regression shard: ${{ matrix.shard }} (${{ matrix.mode }})" run: | echo "Shard: ${{ matrix.shard }}" - echo "Args: ${{ matrix.args }}" + echo "Mode: ${{ matrix.mode }}" + echo "Args: ${{ matrix.args }}" docker run --rm \ --security-opt seccomp=unconfined \ --shm-size=4g \ -v ${{ github.workspace }}/packages/producer/tests:/app/packages/producer/tests \ hyperframes-producer:test \ + --mode=${{ matrix.mode }} \ ${{ matrix.args }} - name: Upload failure artifacts diff --git a/packages/producer/scripts/plan-regression-shards.mjs b/packages/producer/scripts/plan-regression-shards.mjs index 88c2a5ccb..a6962a770 100644 --- a/packages/producer/scripts/plan-regression-shards.mjs +++ b/packages/producer/scripts/plan-regression-shards.mjs @@ -31,6 +31,13 @@ const SCHEDULE_FILE = join(TESTS_DIR, "shard-schedule.json"); export const DEFAULT_SHARD_COUNT = 8; +/** + * Shards for fixtures run via `--mode=distributed-simulated`. Chunked renders + * are fast enough that one shard absorbs all of them; raise it in the schedule + * if that stops being true. + */ +export const DEFAULT_DISTRIBUTED_SHARD_COUNT = 1; + /** * A fixture with no recorded timing still has to land somewhere. Assume it is * on the expensive side so an unmeasured newcomer cannot quietly overload the @@ -100,7 +107,10 @@ export function planShards({ const schedule = JSON.parse(readFileSync(scheduleFile, "utf-8")); const timings = schedule.timings ?? {}; const excluded = schedule.excluded ?? {}; + const distributed = schedule.distributed ?? {}; const resolvedShardCount = shardCount ?? schedule.shardCount ?? DEFAULT_SHARD_COUNT; + const resolvedDistributedShardCount = + schedule.distributedShardCount ?? DEFAULT_DISTRIBUTED_SHARD_COUNT; const onDisk = discoverFixtures(testsDir); const onDiskSet = new Set(onDisk); @@ -139,17 +149,47 @@ export function planShards({ ); } + // A distributed fixture must also be scheduled — the mode says *how* to run + // it, not *whether*. Listing one that is excluded or absent is a typo, and a + // silent one, since the mode map is not consulted when building the shard set. + const misdeclared = Object.keys(distributed).filter((name) => !(name in timings)); + if (misdeclared.length > 0) { + throw new Error( + `Fixtures marked "distributed" are not scheduled: ${misdeclared.join(", ")}.\n` + + `Add each to "timings" in ${scheduleFile}, or drop it from "distributed".`, + ); + } + const scheduled = onDisk.filter((name) => !(name in excluded)); - const bins = packShards(scheduled, timings, resolvedShardCount); + + // Harness mode is a per-invocation flag, so a shard cannot mix modes. Pack + // each mode into its own shards rather than trying to interleave them. + const inProcess = scheduled.filter((name) => !(name in distributed)); + const chunked = scheduled.filter((name) => name in distributed); + + const bins = [ + ...packShards(inProcess, timings, resolvedShardCount).map((bin) => ({ + ...bin, + mode: "in-process", + })), + ...(chunked.length > 0 + ? packShards(chunked, timings, resolvedDistributedShardCount).map((bin) => ({ + ...bin, + mode: "distributed-simulated", + })) + : []), + ]; return { include: bins.map((bin, index) => ({ shard: `shard-${index + 1}`, args: bin.fixtures.join(" "), + mode: bin.mode, })), // Diagnostics for the workflow log — not consumed by the matrix. plan: bins.map((bin, index) => ({ shard: `shard-${index + 1}`, + mode: bin.mode, fixtures: bin.fixtures.length, estimatedMinutes: Math.round((bin.seconds / 60) * 10) / 10, })), @@ -171,7 +211,7 @@ function main() { const worst = Math.max(...plan.map((row) => row.estimatedMinutes)); const best = Math.min(...plan.map((row) => row.estimatedMinutes)); for (const row of plan) { - console.log(`${row.shard}\t${row.fixtures} fixtures\t~${row.estimatedMinutes}m`); + console.log(`${row.shard}\t${row.mode}\t${row.fixtures} fixtures\t~${row.estimatedMinutes}m`); } console.log( `\nworst shard ~${worst}m, lightest ~${best}m, spread ~${ diff --git a/packages/producer/src/regression-shard-plan.test.ts b/packages/producer/src/regression-shard-plan.test.ts index 61978ab99..158154e0d 100644 --- a/packages/producer/src/regression-shard-plan.test.ts +++ b/packages/producer/src/regression-shard-plan.test.ts @@ -11,6 +11,7 @@ import { describe, expect, it } from "bun:test"; import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { checkDistributedSupport } from "./regression-harness-distributed.js"; import { discoverTestSuites } from "./regression-harness.js"; import { discoverFixtures, @@ -21,7 +22,11 @@ import { const TESTS_DIR = join(import.meta.dir, "..", "tests"); -function readSchedule(): { timings?: Record; excluded?: Record } { +function readSchedule(): { + timings?: Record; + excluded?: Record; + distributed?: Record; +} { return JSON.parse(readFileSync(join(TESTS_DIR, "shard-schedule.json"), "utf-8")); } @@ -72,6 +77,85 @@ describe("shard planner fixture discovery", () => { expect(() => planShards({ scheduleFile: tainted })).toThrow(/both scheduled and excluded/); }); + it("never mixes harness modes within a shard", () => { + // `--mode` is a per-invocation flag, so a shard carrying both kinds would + // silently run half of them in the wrong mode. + const { include } = planShards(); + const distributed = new Set(Object.keys(readSchedule().distributed ?? {})); + for (const row of include) { + const fixtures = row.args.split(" "); + const chunked = fixtures.filter((f) => distributed.has(f)); + expect(chunked.length === 0 || chunked.length === fixtures.length).toBe(true); + expect(row.mode).toBe(chunked.length > 0 ? "distributed-simulated" : "in-process"); + } + }); + + it("gives every shard a mode the harness accepts", () => { + // Guards against a typo reaching the workflow, where `--mode=` throws + // at parse time inside the container after the image has already built. + for (const row of planShards().include) { + expect(["in-process", "distributed-simulated", "lambda-local"]).toContain(row.mode); + } + }); + + it("rejects a distributed fixture that is not scheduled", () => { + const schedule = readSchedule(); + const tainted = join(mkdtempSync(join(tmpdir(), "hf-shard-schedule-")), "shard-schedule.json"); + writeFileSync( + tainted, + JSON.stringify({ + ...schedule, + distributed: { ...schedule.distributed, "not-a-real-fixture": "typo" }, + }), + ); + expect(() => planShards({ scheduleFile: tainted })).toThrow(/are not scheduled/); + }); + + it("only assigns distributed mode to fixtures the harness can actually run that way", () => { + // The blocking gap: `checkDistributedSupport` refuses HDR, non-integer fps, + // and fps outside {24,30,60}, and the harness records a refusal as + // `passed: true` with `skipped`. Skipping was safe while in-process also + // ran the fixture. It is not safe now — these fixtures run in distributed + // mode and nowhere else, so a later `hdr: true` or fps edit would turn + // their only coverage into a green no-op with every other planner + // invariant still passing. Membership and reason-text checks cannot see + // that; runtime support has to be part of the committed contract. + const suites = new Map(discoverTestSuites(TESTS_DIR, []).map((s) => [s.id, s])); + for (const fixture of Object.keys(readSchedule().distributed ?? {})) { + const suite = suites.get(fixture); + expect( + suite, + `${fixture} is marked distributed but the harness cannot load it`, + ).toBeDefined(); + const support = checkDistributedSupport( + (suite as { meta: { renderConfig: Parameters[0] } }).meta + .renderConfig, + ); + expect( + support.supported, + `${fixture} is scheduled distributed-only but distributed mode refuses it: ` + + `${support.supported ? "" : support.reason}`, + ).toBe(true); + } + }); + + it("would reject a distributed fixture the harness refuses to run chunked", () => { + // Proves the guard above has teeth rather than passing vacuously. + const hdr = checkDistributedSupport({ fps: { num: 30, den: 1 }, hdr: true }); + expect(hdr.supported).toBe(false); + const ntsc = checkDistributedSupport({ fps: { num: 30000, den: 1001 } }); + expect(ntsc.supported).toBe(false); + const odd = checkDistributedSupport({ fps: { num: 25, den: 1 } }); + expect(odd.supported).toBe(false); + }); + + it("gives every distributed fixture a written reason", () => { + for (const [fixture, reason] of Object.entries(readSchedule().distributed ?? {})) { + expect(typeof reason, `${fixture} needs a reason`).toBe("string"); + expect((reason as string).length, `${fixture} needs a real reason`).toBeGreaterThan(20); + } + }); + it("gives every excluded fixture a written reason", () => { // Exclusions are how a fixture legitimately stays out of CI, so the bar // is that someone had to type why. This is what stops the excluded list diff --git a/packages/producer/tests/shard-schedule.json b/packages/producer/tests/shard-schedule.json index c25e993f8..828031ea4 100644 --- a/packages/producer/tests/shard-schedule.json +++ b/packages/producer/tests/shard-schedule.json @@ -3,12 +3,14 @@ "Drives the regression workflow's shard matrix via scripts/plan-regression-shards.mjs.", "Every fixture on disk must appear in exactly one of 'timings' or 'excluded', or CI fails.", "timings: per-fixture wall-clock seconds, used only for bin-packing \u2014 approximate is fine.", - " Measured on CI run 30223148821 (green, post single-pass PSNR). A newly added fixture can", - " carry a conservative estimate until the next refresh; over-estimating only costs balance.", + " Measured on CI run 30223148821; entries in 'distributed' carry their chunked timing.", " Refresh from a green main run when shards drift apart; no test asserts these values.", + "distributed: fixtures run with --mode=distributed-simulated, mapped to why.", + " Mode is a per-invocation flag, so these get their own shards.", "excluded: fixtures deliberately not run, each mapped to the reason why." ], "shardCount": 8, + "distributedShardCount": 1, "timings": { "animejs-adapter": 17, "audio-mux-parity": 32, @@ -37,7 +39,7 @@ "style-11-prod": 324, "style-12-prod": 247, "style-13-prod": 898, - "style-15-prod": 1413, + "style-15-prod": 114, "style-16-prod": 401, "style-17-prod": 224, "style-18-prod": 394, @@ -51,7 +53,7 @@ "style-9-prod": 216, "sub-comp-id-selector": 10, "sub-comp-t0": 18, - "sub-composition-video": 1346, + "sub-composition-video": 99, "typegpu-adapter": 15, "variables-prod": 8, "vfr-screen-recording": 7, @@ -89,5 +91,9 @@ "video-hfid-no-id": "Was absent from the hand-written shard matrix, so it has never run in CI and its cost and pass state are unknown. Triage and either schedule it or record a real reason here.", "webgl-video-texture-render-compat": "Was absent from the hand-written shard matrix, so it has never run in CI and its cost and pass state are unknown. Triage and either schedule it or record a real reason here.", "wysiwyg-subcomp-css": "Rejected by the harness at load time (meta.json: maxAudioLagWindows must be >= 1), so it has never run despite looking scheduled-able. Fix the metadata, then schedule it." + }, + "distributed": { + "style-15-prod": "Renders 12.4x faster chunked (1413s -> 114s at 4 CPUs) because in-process capture is pinned to one worker by the auto-worker derate. Passes the same golden at the same 30dB bar, 0/100 failed frames, psnr min 39.3 vs 40.1 in-process, so the sensitivity cost is 0.8dB.", + "sub-composition-video": "Renders 13.6x faster chunked (1346s -> 99s at 4 CPUs). Slow in-process for a different reason to style-15-prod (video decode rather than feTurbulence), which is why chunking helps both. Passes the same golden, 0/100 failed." } }