From 5a029f0d64fb22071ba0a14bab32707cf1679f14 Mon Sep 17 00:00:00 2001 From: liufengkai Date: Mon, 7 Sep 2026 16:25:51 -0700 Subject: [PATCH 1/2] fix(vapor): compile the three GB sdcc translation units in parallel The units have no compile-time dependency and write distinct outputs, so dispatching them together cuts a full --target gb build from a median of 3108 ms to 1847 ms (-40.6%, n=15 each). All three .rel files, crt0.rel, app.ihx and todo.gb stay byte-identical to the serial build across 5 runs. Failures now have to be collected rather than thrown by the first await: report the failing unit in link order with sdcc's stderr attached, and delete the target .rel files up front so a stale one from an earlier build can never reach the link step. Co-Authored-By: Claude Code --- vapor/compiler/rom.ts | 44 +++++++- vapor/tests/gb-build.test.ts | 150 +++++++++++++++++++++++++ vapor/tests/harness/gb_build_runner.ts | 29 +++++ vapor/tests/harness/sdcc_shim.sh | 51 +++++++++ 4 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 vapor/tests/gb-build.test.ts create mode 100644 vapor/tests/harness/gb_build_runner.ts create mode 100644 vapor/tests/harness/sdcc_shim.sh diff --git a/vapor/compiler/rom.ts b/vapor/compiler/rom.ts index 906d84722..e81f2f5f4 100644 --- a/vapor/compiler/rom.ts +++ b/vapor/compiler/rom.ts @@ -8,6 +8,7 @@ // Toolchain recipes carry over from Pocket Static's target packagers. import { $ } from "bun"; +import { rm } from "node:fs/promises"; import { dirname, join } from "node:path"; import { nesFontBytes, VAPOR_TARGETS, type CompiledApp, type VaporTargetName } from "./compile.ts"; import { buildEsp32Firmware } from "./esp32.ts"; @@ -154,13 +155,50 @@ export async function buildGbRom(app: CompiledApp, outRom: string): Promise<{ ro const defines = targetDefines("gb"); const cflags = ["-msm83", "--opt-code-size", ...defines, `-I${RUNTIME}`, `-I${gbDir}`]; - for (const [src, rel] of [ + // The three translation units have no compile-time dependency on each other + // and write distinct outputs, so sdcc runs them concurrently — the external + // toolchain dominates a GB build's wall clock. Order below is the link order + // used further down and also the order failures are reported in, so a build + // that breaks two units at once still prints the same message every run. + const units = [ [join(RUNTIME, "vapor_core.c"), "vapor_core.rel"], [join(gbDir, "vapor_gb.c"), "vapor_gb.rel"], [genC, "gen_app.rel"], - ] as const) { - await $`sdcc ${cflags} -c ${src} -o ${join(genDir, rel)}`.quiet(); + ] as const; + + // A .rel left by an earlier build would otherwise still be on disk when its + // sdcc run fails, and the link step cannot tell it apart from a fresh one. + await Promise.all(units.map(([, rel]) => rm(join(genDir, rel), { force: true }))); + + // `.nothrow()` so one unit's failure does not discard the others' output, and + // so sdcc's stderr is available to report instead of a bare exit code. + const compiled = await Promise.allSettled( + units.map(([src, rel]) => + $`sdcc ${cflags} -c ${src} -o ${join(genDir, rel)}`.quiet().nothrow(), + ), + ); + const failures = compiled.flatMap((result, i) => { + const rel = units[i][1]; + if (result.status === "rejected") { + const reason: unknown = result.reason; + return [{ rel, detail: String(reason instanceof Error ? reason.message : reason) }]; + } + if (result.value.exitCode !== 0) { + const stderr = result.value.stderr.toString().trim(); + return [{ rel, detail: stderr || `sdcc exited ${result.value.exitCode}` }]; + } + return []; + }); + if (failures.length > 0) { + const [{ rel, detail }] = failures; + const alsoFailed = failures.slice(1).map(({ rel: other }) => other); + throw new Error( + `sdcc failed compiling ${rel} for target gb${ + alsoFailed.length > 0 ? ` (${alsoFailed.join(", ")} also failed)` : "" + }\n${detail.trimEnd()}`, + ); } + await $`sdasgb -plosgff -o ${join(genDir, "crt0.rel")} ${join(gbDir, "crt0.s")}`.quiet(); // _HOME holds sdcc's library routines (long div/mod): pin it into ROM diff --git a/vapor/tests/gb-build.test.ts b/vapor/tests/gb-build.test.ts new file mode 100644 index 000000000..1a20dc485 --- /dev/null +++ b/vapor/tests/gb-build.test.ts @@ -0,0 +1,150 @@ +// vapor/tests/gb-build.test.ts — GB toolchain scheduling and failure propagation. +// +// buildGbRom compiles three independent translation units with sdcc. They run +// concurrently, which is only safe if a failure in any one of them still fails +// the build and no stale .rel from a previous build can be linked in its place. +// Each test spawns harness/gb_build_runner.ts with a shim named `sdcc` first on +// PATH (harness/sdcc_shim.sh) — a fresh process, because Bun's `$` resolves +// PATH as it was at startup. The shim logs when each compile starts and ends +// and fails chosen units on demand. + +import { afterAll, describe, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { chmod, copyFile, mkdir, readFile, rm } from "node:fs/promises"; +import { join } from "node:path"; + +const HERE = import.meta.dir; +const ENTRY = join(HERE, "..", "examples", "todo", "todo.tsx"); +const SHIM = join(HERE, "harness", "sdcc_shim.sh"); +const RUNNER = join(HERE, "harness", "gb_build_runner.ts"); +const OUT = join(HERE, "..", "..", "dist", "vapor", "gb-build-test"); + +const UNITS = ["vapor_core.c", "vapor_gb.c", "gen_app.c"] as const; +const RELS = ["vapor_core.rel", "vapor_gb.rel", "gen_app.rel"] as const; + +const REAL_SDCC = (await Bun.$`which sdcc`.text()).trim(); + +interface ShimRun { + unit: string; + start: number; + end: number; +} + +interface BuildResult { + ok: boolean; + romBytes?: number; + message?: string; + runs: ShimRun[]; +} + +/** Build todo.tsx for GB in a child process with `sdcc` shimmed. */ +async function build(dir: string, shimEnv: Record = {}): Promise { + const bin = join(dir, "bin"); + await mkdir(bin, { recursive: true }); + await copyFile(SHIM, join(bin, "sdcc")); + await chmod(join(bin, "sdcc"), 0o755); + + const logPath = join(dir, "sdcc.log"); + await rm(logPath, { force: true }); + + const proc = Bun.spawn(["bun", RUNNER, ENTRY, join(dir, "todo.gb")], { + env: { + ...process.env, + ...shimEnv, + VP_SDCC_REAL: REAL_SDCC, + VP_SDCC_LOG: logPath, + PATH: `${bin}:${process.env.PATH ?? ""}`, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, code] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + const line = stdout.trim().split("\n").at(-1) ?? ""; + if (code !== 0 || !line.startsWith("{")) { + throw new Error(`gb_build_runner exited ${code}\nstdout:\n${stdout}\nstderr:\n${stderr}`); + } + + const at = new Map>(); + const log = existsSync(logPath) ? await readFile(logPath, "utf8") : ""; + for (const entry of log.split("\n").filter((l) => l.trim())) { + const [kind, unit, ms] = entry.split(" "); + const run = at.get(unit) ?? { unit }; + if (kind === "S") run.start = Number(ms); + else run.end = Number(ms); + at.set(unit, run); + } + const runs = [...at.values()].filter( + (r): r is ShimRun => r.start !== undefined && r.end !== undefined, + ); + return { ...(JSON.parse(line) as Omit), runs }; +} + +afterAll(async () => { + await rm(OUT, { recursive: true, force: true }); +}); + +describe("GB build: three sdcc translation units", () => { + test("the three units overlap in time instead of running one after another", async () => { + // Stub mode writes junk to each -o, so the link fails; what this build is + // for is the timing log the three compiles leave behind. + const result = await build(join(OUT, "concurrent"), { + VP_SDCC_STUB: "1", + VP_SDCC_STUB_DELAY: "0.4", + }); + + const compiles = result.runs.filter((r) => (UNITS as readonly string[]).includes(r.unit)); + expect(compiles.map((r) => r.unit).sort()).toEqual([...UNITS].sort()); + // Serial execution puts every start at or after the previous end. Each unit + // sleeps 400 ms, so real overlap is far wider than clock granularity. + const lastStart = Math.max(...compiles.map((r) => r.start)); + const firstEnd = Math.min(...compiles.map((r) => r.end)); + expect(lastStart).toBeLessThan(firstEnd); + }, 60_000); + + for (const [i, unit] of UNITS.entries()) { + test(`a failure in ${unit} fails the build and names the unit`, async () => { + const dir = join(OUT, `fail-${unit}`); + const result = await build(dir, { VP_SDCC_FAIL: unit }); + + expect(result.ok).toBe(false); + expect(result.message).toContain(RELS[i]); + expect(result.message).toContain("target gb"); + // sdcc's own stderr survives into the message, or the failure is + // undiagnosable from a build log alone. + expect(result.message).toContain(`injected failure for ${unit}`); + // The failing unit produced nothing, and the build stopped before + // makebin/rgbfix could write a ROM. + expect(existsSync(join(dir, "gen-gb", RELS[i]))).toBe(false); + expect(existsSync(join(dir, "todo.gb"))).toBe(false); + }, 60_000); + } + + test("a unit that fails on a rebuild does not let the previous .rel be linked", async () => { + const dir = join(OUT, "stale"); + const rel = join(dir, "gen-gb", RELS[2]); + + const first = await build(dir); + expect(first.ok).toBe(true); + expect(first.romBytes).toBe(32768); + expect((await readFile(rel)).length).toBeGreaterThan(0); + + const second = await build(dir, { VP_SDCC_FAIL: UNITS[2] }); + expect(second.ok).toBe(false); + // The good .rel from the first build is gone rather than silently reused. + expect(existsSync(rel)).toBe(false); + }, 120_000); + + test("all three units failing at once reports one unit and mentions the others", async () => { + const result = await build(join(OUT, "fail-all"), { VP_SDCC_FAIL: UNITS.join(",") }); + + expect(result.ok).toBe(false); + // Reported in link order, so one build breakage reads the same way every + // run regardless of which process happened to exit first. + expect(result.message).toContain(`sdcc failed compiling ${RELS[0]} for target gb`); + expect(result.message).toContain(`${RELS[1]}, ${RELS[2]} also failed`); + }, 60_000); +}); diff --git a/vapor/tests/harness/gb_build_runner.ts b/vapor/tests/harness/gb_build_runner.ts new file mode 100644 index 000000000..1a0d2b4c9 --- /dev/null +++ b/vapor/tests/harness/gb_build_runner.ts @@ -0,0 +1,29 @@ +#!/usr/bin/env bun +// vapor/tests/harness/gb_build_runner.ts — run one buildGbRom in a child. +// +// Bun's `$` resolves PATH as it was when the process started, so a test that +// wants a shimmed `sdcc` has to launch a fresh process with PATH already set. +// gb-build.test.ts spawns this and reads the JSON line it prints: +// +// {"ok":true,"romBytes":32768} | {"ok":false,"message":"..."} +// +// argv: + +import { compileVaporApp } from "../../compiler/compile.ts"; +import { buildGbRom } from "../../compiler/rom.ts"; + +const [entry, outRom] = process.argv.slice(2); +if (!entry || !outRom) { + console.error("usage: gb_build_runner.ts "); + process.exit(2); +} + +const app = compileVaporApp(entry, await Bun.file(entry).text(), "VAPOR TODO", "gb"); +try { + const { romBytes } = await buildGbRom(app, outRom); + console.log(JSON.stringify({ ok: true, romBytes })); +} catch (e) { + console.log( + JSON.stringify({ ok: false, message: String(e instanceof Error ? e.message : e) }), + ); +} diff --git a/vapor/tests/harness/sdcc_shim.sh b/vapor/tests/harness/sdcc_shim.sh new file mode 100644 index 000000000..c7868f1c5 --- /dev/null +++ b/vapor/tests/harness/sdcc_shim.sh @@ -0,0 +1,51 @@ +#!/bin/bash +# vapor/tests/harness/sdcc_shim.sh — stand in for sdcc during GB build tests. +# +# Placed on PATH under the name `sdcc` by gb-build.test.ts. It records when +# each invocation starts and ends (so a test can see whether the three +# translation units overlap in time) and can fail a chosen unit on demand +# (so a test can see whether that failure reaches the caller). +# +# VP_SDCC_REAL path to the real sdcc (required) +# VP_SDCC_LOG append "S|E " per run (optional) +# VP_SDCC_FAIL comma-separated source basenames to fail (optional) +# VP_SDCC_STUB 1 = don't really compile; just touch -o (optional) +# +# Only `-c` compiles are shimmed by name; the link invocation has no -c and +# no source basename, so it always falls through to the real sdcc. + +src="" +out="" +compiling="" +prev="" +for arg in "$@"; do + case "$prev" in + -o) out="$arg" ;; + esac + case "$arg" in + -c) compiling=1 ;; + *.c) src="$arg" ;; + esac + prev="$arg" +done +unit="${src##*/}" + +now_ms() { echo $(( $(date +%s%N) / 1000000 )); } +log() { [ -n "${VP_SDCC_LOG:-}" ] && echo "$1 ${unit:-link} $(now_ms)" >> "$VP_SDCC_LOG"; } + +log S +status=0 +if [ -n "$compiling" ] && [ -n "$unit" ] && [[ ",${VP_SDCC_FAIL:-}," == *",$unit,"* ]]; then + echo "sdcc_shim: injected failure for $unit" >&2 + status=1 +elif [ -n "$compiling" ] && [ -n "${VP_SDCC_STUB:-}" ]; then + # Stubbed success: a non-empty file at -o that is not a valid .rel, so a + # link that wrongly proceeds on it fails loudly rather than silently. + [ -n "$out" ] && printf 'sdcc_shim stub\n' > "$out" + sleep "${VP_SDCC_STUB_DELAY:-0}" +else + "$VP_SDCC_REAL" "$@" + status=$? +fi +log E +exit $status From 07ce66b8a7ef74a251bcfc18b31b1bde779a6c8b Mon Sep 17 00:00:00 2001 From: liufengkai Date: Mon, 7 Sep 2026 16:58:41 -0700 Subject: [PATCH 2/2] review(vapor): harden GB parallel build failures Co-authored-by: TRAE CLI --- vapor/compiler/rom.ts | 10 ++++--- vapor/tests/gb-build.test.ts | 45 +++++++++++++++++++++++++++----- vapor/tests/harness/sdcc_shim.sh | 19 ++++++++++++-- 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/vapor/compiler/rom.ts b/vapor/compiler/rom.ts index e81f2f5f4..e8464ee76 100644 --- a/vapor/compiler/rom.ts +++ b/vapor/compiler/rom.ts @@ -148,6 +148,8 @@ export async function buildGbRom(app: CompiledApp, outRom: string): Promise<{ ro const outDir = dirname(outRom); const genDir = join(outDir, "gen-gb"); await $`mkdir -p ${genDir}`.quiet(); + // A failed rebuild must not leave the previous ROM looking current. + await rm(outRom, { force: true }); const genC = join(genDir, "gen_app.c"); await Bun.write(genC, app.c); @@ -185,17 +187,19 @@ export async function buildGbRom(app: CompiledApp, outRom: string): Promise<{ ro } if (result.value.exitCode !== 0) { const stderr = result.value.stderr.toString().trim(); - return [{ rel, detail: stderr || `sdcc exited ${result.value.exitCode}` }]; + const stdout = result.value.stdout.toString().trim(); + return [{ rel, detail: stderr || stdout || `sdcc exited ${result.value.exitCode}` }]; } return []; }); if (failures.length > 0) { - const [{ rel, detail }] = failures; + await Promise.all(failures.map(({ rel }) => rm(join(genDir, rel), { force: true }))); + const [{ rel }] = failures; const alsoFailed = failures.slice(1).map(({ rel: other }) => other); throw new Error( `sdcc failed compiling ${rel} for target gb${ alsoFailed.length > 0 ? ` (${alsoFailed.join(", ")} also failed)` : "" - }\n${detail.trimEnd()}`, + }\n${failures.map((failure) => `${failure.rel}: ${failure.detail.trimEnd()}`).join("\n")}`, ); } diff --git a/vapor/tests/gb-build.test.ts b/vapor/tests/gb-build.test.ts index 1a20dc485..c86276031 100644 --- a/vapor/tests/gb-build.test.ts +++ b/vapor/tests/gb-build.test.ts @@ -108,13 +108,18 @@ describe("GB build: three sdcc translation units", () => { for (const [i, unit] of UNITS.entries()) { test(`a failure in ${unit} fails the build and names the unit`, async () => { const dir = join(OUT, `fail-${unit}`); - const result = await build(dir, { VP_SDCC_FAIL: unit }); + const result = await build(dir, { + VP_SDCC_FAIL: unit, + // Exercise both common diagnostic streams: gen_app stands in for an + // sdcc wrapper that reports its failure on stdout. + ...(unit === "gen_app.c" ? { VP_SDCC_FAIL_STDOUT: unit } : {}), + }); expect(result.ok).toBe(false); expect(result.message).toContain(RELS[i]); expect(result.message).toContain("target gb"); - // sdcc's own stderr survives into the message, or the failure is - // undiagnosable from a build log alone. + // sdcc's own diagnostic survives into the message, whether the tool or + // wrapper writes it to stderr or stdout. expect(result.message).toContain(`injected failure for ${unit}`); // The failing unit produced nothing, and the build stopped before // makebin/rgbfix could write a ROM. @@ -123,28 +128,54 @@ describe("GB build: three sdcc translation units", () => { }, 60_000); } - test("a unit that fails on a rebuild does not let the previous .rel be linked", async () => { + test("a unit that fails on a rebuild removes stale and partial outputs", async () => { const dir = join(OUT, "stale"); const rel = join(dir, "gen-gb", RELS[2]); + const rom = join(dir, "todo.gb"); const first = await build(dir); expect(first.ok).toBe(true); expect(first.romBytes).toBe(32768); expect((await readFile(rel)).length).toBeGreaterThan(0); - const second = await build(dir, { VP_SDCC_FAIL: UNITS[2] }); + const second = await build(dir, { + VP_SDCC_FAIL: UNITS[2], + // Some compiler versions or wrappers can truncate/write -o before + // returning nonzero; that partial output must not replace the stale one. + VP_SDCC_FAIL_OUTPUT: "1", + }); expect(second.ok).toBe(false); - // The good .rel from the first build is gone rather than silently reused. + // Neither the old ROM nor a stale/partial .rel can masquerade as output + // from the failed rebuild. expect(existsSync(rel)).toBe(false); + expect(existsSync(rom)).toBe(false); }, 120_000); test("all three units failing at once reports one unit and mentions the others", async () => { - const result = await build(join(OUT, "fail-all"), { VP_SDCC_FAIL: UNITS.join(",") }); + const result = await build(join(OUT, "fail-all"), { + VP_SDCC_FAIL: UNITS.join(","), + // Complete in reverse link order. The diagnostic must still use link + // order, rather than whichever subprocess happens to exit first. + VP_SDCC_DELAY_VAPOR_CORE: "0.4", + VP_SDCC_DELAY_VAPOR_GB: "0.2", + }); expect(result.ok).toBe(false); + expect( + result.runs + .filter((r) => (UNITS as readonly string[]).includes(r.unit)) + .sort((a, b) => a.end - b.end) + .map((r) => r.unit), + ).toEqual([...UNITS].reverse()); // Reported in link order, so one build breakage reads the same way every // run regardless of which process happened to exit first. expect(result.message).toContain(`sdcc failed compiling ${RELS[0]} for target gb`); expect(result.message).toContain(`${RELS[1]}, ${RELS[2]} also failed`); + const detailOffsets = RELS.map((rel) => result.message!.indexOf(`${rel}:`)); + expect(detailOffsets.every((offset) => offset >= 0)).toBe(true); + expect(detailOffsets).toEqual([...detailOffsets].sort((a, b) => a - b)); + for (const unit of UNITS) { + expect(result.message).toContain(`injected failure for ${unit}`); + } }, 60_000); }); diff --git a/vapor/tests/harness/sdcc_shim.sh b/vapor/tests/harness/sdcc_shim.sh index c7868f1c5..631e5e0cd 100644 --- a/vapor/tests/harness/sdcc_shim.sh +++ b/vapor/tests/harness/sdcc_shim.sh @@ -9,7 +9,10 @@ # VP_SDCC_REAL path to the real sdcc (required) # VP_SDCC_LOG append "S|E " per run (optional) # VP_SDCC_FAIL comma-separated source basenames to fail (optional) +# VP_SDCC_FAIL_OUTPUT 1 = leave partial -o before failing (optional) +# VP_SDCC_FAIL_STDOUT comma-separated failures using stdout (optional) # VP_SDCC_STUB 1 = don't really compile; just touch -o (optional) +# VP_SDCC_DELAY_ seconds to wait before that unit exits (optional) # # Only `-c` compiles are shimmed by name; the link invocation has no -c and # no source basename, so it always falls through to the real sdcc. @@ -30,13 +33,24 @@ for arg in "$@"; do done unit="${src##*/}" -now_ms() { echo $(( $(date +%s%N) / 1000000 )); } +now_ms() { bun -e 'process.stdout.write(String(Date.now()))'; } log() { [ -n "${VP_SDCC_LOG:-}" ] && echo "$1 ${unit:-link} $(now_ms)" >> "$VP_SDCC_LOG"; } +delay="" +case "$unit" in + vapor_core.c) delay="${VP_SDCC_DELAY_VAPOR_CORE:-}" ;; + vapor_gb.c) delay="${VP_SDCC_DELAY_VAPOR_GB:-}" ;; + gen_app.c) delay="${VP_SDCC_DELAY_GEN_APP:-}" ;; +esac log S status=0 if [ -n "$compiling" ] && [ -n "$unit" ] && [[ ",${VP_SDCC_FAIL:-}," == *",$unit,"* ]]; then - echo "sdcc_shim: injected failure for $unit" >&2 + [ -n "${VP_SDCC_FAIL_OUTPUT:-}" ] && [ -n "$out" ] && printf 'sdcc_shim partial output\n' > "$out" + if [[ ",${VP_SDCC_FAIL_STDOUT:-}," == *",$unit,"* ]]; then + echo "sdcc_shim: injected failure for $unit" + else + echo "sdcc_shim: injected failure for $unit" >&2 + fi status=1 elif [ -n "$compiling" ] && [ -n "${VP_SDCC_STUB:-}" ]; then # Stubbed success: a non-empty file at -o that is not a valid .rel, so a @@ -47,5 +61,6 @@ else "$VP_SDCC_REAL" "$@" status=$? fi +[ -n "$delay" ] && sleep "$delay" log E exit $status