From 89a3d6e0eb3ff5bd4390221a3d597288ff95f0aa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 02:03:51 +0000 Subject: [PATCH] ci: trace the linux-aarch64 symbol order file on every main build [generate symbol order] Add linux-aarch64 to traceOrderTargets, so it gets the same trace-order step darwin-aarch64 and linux-x64 already have. Its build lane runs on the aarch64 host and so was left to publish its own file, but a canary lane only inherits, so it re-published the same file on every build and the file was only refreshed by a release or an opt-in trace. With the trace-order step publishing for every target, the build lane no longer uploads the standalone .order artifact (it would race the fresh trace under the same name), and a canary that inherits nothing links unordered and annotates instead of tracing and relinking itself, which removes mustGenerateOrderFile and reportOrderFileBootstrap. The cross-compile-specific annotation becomes reportOrderFileNotInherited and names the step that should have published the file. --- .buildkite/ci.mjs | 42 +++++----- scripts/build.ts | 27 ++++--- scripts/build/ci.ts | 107 ++++++++------------------ scripts/orderfile/generate.ts | 6 +- test/js/bun/perf/linker-order.test.ts | 23 ++---- 5 files changed, 75 insertions(+), 130 deletions(-) diff --git a/.buildkite/ci.mjs b/.buildkite/ci.mjs index 176ee6eeb24f..8549ffb85ade 100755 --- a/.buildkite/ci.mjs +++ b/.buildkite/ci.mjs @@ -718,13 +718,13 @@ function getVerifyBaselineStep(platform, options) { } /** - * Targets whose build lane cross-compiles (so `canTraceOrderFile()` is false) - * but whose test fleet is native. A `-trace-order` step runs there, downloads - * the cross-built `bun-profile`, traces it, and uploads the `.order` artifact - * that the next build's `inheritOrderFile()` picks up. One build of lag. - * - * linux-aarch64 is absent because its build lane runs on the aarch64 host and - * traces itself; `packageAndUpload()` is its sole publisher. + * Every target that links with a symbol order file (`usesOrderFile()` in + * scripts/build/flags.ts), and the native test platform its `-trace-order` + * step runs on. That step is the only publisher of the `.order` artifact the + * next build's `inheritOrderFile()` downloads, so every target needs one, + * including linux-aarch64, whose build lane is native: a canary lane inherits + * rather than paying a second LTO link, so it only traces for a release, and + * without this step its canaries kept re-inheriting one release's trace. * * The `on` platforms are entries of `testPlatforms`, so the step runs on an * image that exists. The windows tracer is built on the test VM for whichever @@ -733,24 +733,24 @@ function getVerifyBaselineStep(platform, options) { */ const traceOrderTargets = [ { os: "darwin", arch: "aarch64", on: { os: "darwin", arch: "aarch64", release: "26", tier: "latest" } }, + { os: "linux", arch: "aarch64", on: { os: "linux", arch: "aarch64", distro: "debian", release: "13" } }, { os: "linux", arch: "x64", on: { os: "linux", arch: "x64", distro: "debian", release: "13" } }, { os: "windows", arch: "x64", on: { os: "windows", arch: "x64", release: "2019", tier: "oldest" } }, { os: "windows", arch: "aarch64", on: { os: "windows", arch: "aarch64", release: "11", tier: "latest" } }, ]; /** - * Trace the symbol order file for a cross-compiled target on a native-arch - * host, so the next build's `inheritOrderFile()` has something to download. + * Trace the symbol order file for a target on its native test fleet, so the + * next build's `inheritOrderFile()` has something to download. One build of + * lag. * - * The build lane cross-compiles from the aarch64 `buildHostPlatform` and cannot - * run the binary it linked. This step runs on the target-arch test fleet, - * downloads that lane's unstripped `bun-profile`, runs it under `scripts/ + * Downloads the build lane's unstripped `bun-profile`, runs it under `scripts/ * orderfile/generate.ts` (the traced binary doubles as the interpreter), and * uploads the result. * - * Non-PR only — `orderFileEligible()` ignores PR builds, so a trace there has - * no consumer. Soft-fail: the order file is an optimization, and a broken - * tracer must not fail a build. + * Main only, unless opted into (see getPipeline): `orderFileEligible()` ignores + * PR builds, so a trace there has no consumer. Soft-fail: the order file is an + * optimization, and a broken tracer must not fail a build. * * Windows agents run commands under cmd.exe (see getVerifyBaselineStep for the * `|| exit /b 1` convention). The generator compiles the tracer there, which @@ -1541,12 +1541,12 @@ async function getPipeline(options = {}) { steps.push(getStepWithDependsOn(getVerifyBaselineStep(target, options), ...verifyDeps)); } - // Seed the symbol order file for a cross-compiled target on its native - // test fleet (see getTraceOrderStep). Always on main so the inheritance - // chain stays fed, and anywhere else on commit-message opt-in so a PR - // that changes the tracer can prove the step works before merge — the - // same `[generate symbol order]` tag ci.ts already honours. Release - // profile only — usesOrderFile() is false under a sanitizer anyway. + // Publish the target's symbol order file from its native test fleet + // (see getTraceOrderStep). Always on main so the inheritance chain + // stays fed, and anywhere else on commit-message opt-in so a PR that + // changes the tracer can prove the step works before merge — the same + // `[generate symbol order]` tag ci.ts already honours. Release profile + // only — usesOrderFile() is false under a sanitizer anyway. const traceOn = traceOrderTargets.find( t => t.os === target.os && t.arch === target.arch && !target.abi && (target.profile ?? "release") === "release", diff --git a/scripts/build.ts b/scripts/build.ts index 2af7a1ee5942..744361fcb3b7 100644 --- a/scripts/build.ts +++ b/scripts/build.ts @@ -24,19 +24,16 @@ import { spawnSync } from "node:child_process"; import { readFileSync } from "node:fs"; import { join } from "node:path"; import { - canTraceOrderFile, downloadArtifacts, inheritOrderFile, isCI, - mustGenerateOrderFile, orderFileContext, orderFileEligible, packageAndUpload, printEnvironment, regenerateOrderFile, - reportOrderFileBootstrap, - reportOrderFileCannotTrace, reportOrderFileFailure, + reportOrderFileNotInherited, shouldGenerateOrderFile, spawnWithAnnotations, startGroup, @@ -159,15 +156,14 @@ async function main(): Promise { await startGroup("Build", () => runNinja()); - // Trace and relink when we are a release, when a commit asked for it, or when - // there was nothing to inherit. A failed trace is not fatal: the order file is - // an optimization, and a flaky workload must not kill a release 40 minutes in. - if (mustGenerateOrderFile(result.cfg, orderCtx, inherited)) { - if (!inherited && !shouldGenerateOrderFile(result.cfg, orderCtx)) reportOrderFileBootstrap(result.cfg); + // Trace and relink when we are a release or when a commit asked for it. A + // failed trace is not fatal: the order file is an optimization, and a flaky + // workload must not kill a release 40 minutes in. + if (shouldGenerateOrderFile(result.cfg, orderCtx)) { let traced = true; await startGroup("Generate symbol order file", () => { try { - regenerateOrderFile(result.cfg, orderCtx); + regenerateOrderFile(result.cfg); } catch (error) { traced = false; reportOrderFileFailure(error as Error); @@ -178,10 +174,13 @@ async function main(): Promise { // We traced this exact binary: nearly every symbol must resolve. Hard-fail. if (result.output.exe) verifyOrderFileApplied(result.cfg, orderCtx, result.output.exe); } - } else if (orderFileEligible(result.cfg, orderCtx) && result.output.exe) { - // Inherited: a stale file is a slower binary, not a broken one. - if (!inherited && !canTraceOrderFile(result.cfg)) reportOrderFileCannotTrace(result.cfg); - verifyOrderFileApplied(result.cfg, orderCtx, result.output.exe, { strict: false }); + } else if (orderFileEligible(result.cfg, orderCtx)) { + if (!inherited) { + reportOrderFileNotInherited(result.cfg); + } else if (result.output.exe) { + // Inherited: a stale file is a slower binary, not a broken one. + verifyOrderFileApplied(result.cfg, orderCtx, result.output.exe, { strict: false }); + } } // cpp-only/rust-only: upload build outputs for downstream link-only. diff --git a/scripts/build/ci.ts b/scripts/build/ci.ts index 13cff829e4bb..cb08d9d4f146 100644 --- a/scripts/build/ci.ts +++ b/scripts/build/ci.ts @@ -442,24 +442,16 @@ export function packageAndUpload(cfg: Config, output: BunOutput): void { // having no symbol table, so without them that step has nothing to work from. files.push(...linkerMapOutputs(cfg).map(map => basename(map))); // The symbol ordering file this binary was linked with, next to the linker - // map. Skip the seeded placeholder — it has no functions in it. - const hasOrderFile = usesOrderFile(cfg) && orderFileFunctionCount(cfg) > 0; - if (hasOrderFile) { + // map. Skip the seeded placeholder — it has no functions in it. Only inside + // the zip: the standalone `.order` the next build inherits is the trace-order + // step's fresh trace of this binary (.buildkite/ci.mjs), and this copy is + // usually itself inherited, so publishing it under that name too would hand + // inheritOrderFile() a stale artifact racing the fresh one. + if (usesOrderFile(cfg) && orderFileFunctionCount(cfg) > 0) { files.push(basename(orderFilePath(cfg))); } zipPaths.push(makeZip(cfg, bunPath, files)); - // Also upload it standalone, so the next build inherits it with a small - // download instead of pulling the whole profile zip. Only when this lane - // traced the file itself — a cross-compiled lane's fresh trace comes from the - // sibling trace-order step (.buildkite/ci.mjs), and re-uploading the inherited - // copy would give inheritOrderFile() two same-named artifacts to race over. - if (hasOrderFile && canTraceOrderFile(cfg)) { - const artifact = orderFileArtifact(cfg); - cpSync(orderFilePath(cfg), resolve(buildDir, artifact)); - zipPaths.push(artifact); - } - // ─── Stripped zip ─── // Only for plain release (shouldStrip). Just the stripped `bun` binary. // cmake: bunStripPath = string(REPLACE bun ${bunTriplet} bunStripPath bun) = bunTriplet. @@ -682,11 +674,12 @@ async function waitForStepOutcome(stepKey: string): Promise { // Symbol ordering file // // A build either generates one (trace its own binary, relink against the result) -// or inherits an earlier build's and links once. Releases generate, canaries -// inherit, PRs do neither; one that inherits nothing generates, seeding the chain. +// or inherits one and links once. Releases generate; canaries inherit the file an +// earlier build's trace-order step published (getTraceOrderStep in +// .buildkite/ci.mjs, the only publisher); PRs do neither. // ═══════════════════════════════════════════════════════════════════════════ -/** Cap on builds we ask for an order file before giving up and generating one. */ +/** Cap on builds we ask for an order file before giving up and linking unordered. */ const PREVIOUS_BUILDS_TO_TRY = 50; /** Bound on the number-probe fallback: a branch is sparse among build numbers. */ @@ -729,23 +722,21 @@ export function orderFileEligible(cfg: Config, ctx: OrderFileContext): boolean { } /** Tracing runs the binary we just linked, so the host must be able to execute it. */ -export function canTraceOrderFile(cfg: Config): boolean { +function canTraceOrderFile(cfg: Config): boolean { return cfg.canRunOnHost; } /** - * An eligible lane that cannot trace (cross-compiled) and inherited nothing is - * shipping unordered. A sibling `-trace-order` step on a native-arch host seeds - * the chain (see getTraceOrderStep in .buildkite/ci.mjs), so this fires once on - * the first build and then the next build inherits that trace. If it persists, - * the trace step is failing or missing for this target. + * An eligible build that inherited nothing ships unordered: a canary does not + * trace (a second link), and a cross-compiled release cannot. Every main build's + * trace-order step publishes a file, so this fires once while that step seeds + * the chain; on consecutive builds it means the step is failing or missing. */ -export function reportOrderFileCannotTrace(cfg: Config): void { +export function reportOrderFileNotInherited(cfg: Config): void { const msg = - `${orderFileArtifact(cfg)}: nothing to inherit and this lane cross-compiles ` + - `(target ${cfg.crossTarget}), so the binary cannot be traced here. Shipping unordered. ` + - `Expected once while the native-arch trace-order step seeds the chain; if this ` + - `appears on every build, that step is failing or missing.`; + `${orderFileArtifact(cfg)}: no recent build on this branch published one, so this binary links ` + + `unordered. Every main build's ${traceOrderStepKey(cfg)} step publishes it, so expect this once, ` + + `while that step seeds the chain; on consecutive builds it means that step is failing or missing.`; console.log(`~ symbol order: ${msg}`); if (!isBuildkite) return; utils.reportAnnotationToBuildKite({ @@ -754,7 +745,7 @@ export function reportOrderFileCannotTrace(cfg: Config): void { label: "symbol order file", content: utils.formatAnnotationToHtml({ filename: "scripts/build/ci.ts", - title: "symbol order file: cross-compiled lane cannot trace, shipping unordered", + title: "symbol order file: nothing to inherit, shipping unordered", content: msg, source: "build", level: "warning", @@ -767,6 +758,11 @@ function orderFileArtifact(cfg: Config): string { return `${computeBunTriplet(cfg)}.order`; } +/** The Buildkite step that publishes it, as getTraceOrderStep keys it: `linux-x64-trace-order`. */ +function traceOrderStepKey(cfg: Config): string { + return `${computeBunTriplet(cfg).replace(/^bun-/, "")}-trace-order`; +} + /** "1m4s" / "12s" — durations show up in every order-file log line. */ function since(start: number): string { const seconds = Math.round((Date.now() - start) / 1000); @@ -784,7 +780,8 @@ function orderFileFunctionCount(cfg: Config): number { /** * Releases always trace their own binary — it is the artifact people install. - * A canary only does so on request, since it costs a second link. + * A canary only does so on request, since it costs a second link; otherwise it + * inherits, and the trace-order step traces it for the builds after it. */ export function shouldGenerateOrderFile(cfg: Config, ctx: OrderFileContext): boolean { if (!orderFileEligible(cfg, ctx) || !canTraceOrderFile(cfg)) return false; @@ -792,15 +789,6 @@ export function shouldGenerateOrderFile(cfg: Config, ctx: OrderFileContext): boo return /\[generate symbol order\]/i.test(ctx.commitMessage); } -/** - * A build that inherited nothing must generate: otherwise it publishes nothing, - * the next build inherits nothing either, and the chain never recovers. - */ -export function mustGenerateOrderFile(cfg: Config, ctx: OrderFileContext, inherited: boolean): boolean { - if (shouldGenerateOrderFile(cfg, ctx)) return true; - return orderFileEligible(cfg, ctx) && canTraceOrderFile(cfg) && !inherited; -} - /** * Builds on this branch that might have published an order file, newest first. * Lazy: the first candidate is nearly always the answer and the caller stops @@ -869,9 +857,8 @@ export async function inheritOrderFile(cfg: Config, ctx: OrderFileContext): Prom for await (const build of candidateBuilds(ctx)) { if (++tried > PREVIOUS_BUILDS_TO_TRY) break; - // No --step: exactly one step per build publishes the target-unique name — - // packageAndUpload() for a lane that traced its own binary, the sibling - // trace-order step (.buildkite/ci.mjs) for a cross-compiled one. + // No --step: the name is target-unique, and only that target's trace-order + // step (.buildkite/ci.mjs) publishes it. const result = spawnSync("buildkite-agent", ["artifact", "download", artifact, ".", "--build", build.id], { cwd: cfg.buildDir, stdio: "ignore", @@ -884,7 +871,7 @@ export async function inheritOrderFile(cfg: Config, ctx: OrderFileContext): Prom cpSync(downloaded, orderFilePath(cfg)); rmSync(downloaded, { force: true }); - // An empty artifact would make us publish nothing, breaking the next build. + // An empty file is a no-op for the linker; an older build's is better than none. const functions = orderFileFunctionCount(cfg); if (functions === 0) { console.log(` #${build.number ?? "?"}: ${artifact} is empty — looking further back`); @@ -908,14 +895,10 @@ export async function inheritOrderFile(cfg: Config, ctx: OrderFileContext): Prom * ninja, which relinks and nothing else: `linkDepends()` lists the order file, * so it is the only edge whose input changed. */ -export function regenerateOrderFile(cfg: Config, ctx: OrderFileContext): void { +export function regenerateOrderFile(cfg: Config): void { const start = Date.now(); const exeName = bunExeName(cfg); // bun-profile, or bun-assertions on an assertions build - const why = !cfg.canary - ? "release build" - : shouldGenerateOrderFile(cfg, ctx) - ? "[generate symbol order] in the commit message" - : "nothing to inherit"; + const why = cfg.canary ? "[generate symbol order] in the commit message" : "release build"; console.log(`Tracing ${exeName} to build a fresh order file (${why})`); console.log("Each workload runs under an injected function-entry tracer, so it is slower than a normal run.\n"); @@ -924,32 +907,6 @@ export function regenerateOrderFile(cfg: Config, ctx: OrderFileContext): void { console.log(`\n+ symbol order: traced ${count} functions in ${since(start)} — relinking against them`); } -/** - * A canary found nothing to inherit and is paying a second link to seed the - * chain. Expected once; on every build it means inheriting is broken. - */ -export function reportOrderFileBootstrap(cfg: Config): void { - if (!cfg.canary) return; // a release always generates — nothing to report - const message = - `No earlier build published ${orderFileArtifact(cfg)}, so this build is tracing its own binary and ` + - `relinking (one extra link). Expected once, to seed the chain. If every build on this branch says ` + - `this, inheriting is broken — check the "Inherit symbol order file" step.`; - console.log(`~ symbol order: ${message}`); - if (!isBuildkite) return; - utils.reportAnnotationToBuildKite({ - style: "warning", - priority: 5, - label: "symbol order file", - content: utils.formatAnnotationToHtml({ - filename: "scripts/build/ci.ts", - title: "symbol order file: nothing to inherit, generating from scratch", - content: message, - source: "build", - level: "warning", - }), - }); -} - /** * The trace failed. Ship the unordered binary — correct, just fatter in resident * pages — but annotate, so this cannot rot into a permanently unordered release. diff --git a/scripts/orderfile/generate.ts b/scripts/orderfile/generate.ts index 9b3267fc83c6..abee109af35e 100644 --- a/scripts/orderfile/generate.ts +++ b/scripts/orderfile/generate.ts @@ -31,9 +31,9 @@ * thousand that no other workload touches. * * The file is never committed. Release builds generate it from their own pass-1 - * binary and relink against it; canary builds inherit the last successful - * build's file and re-publish it (scripts/build/ci.ts — inheritOrderFile / - * packageAndUpload). Locally: + * binary and relink against it; canary builds inherit the file an earlier + * build's trace-order step traced from that build's binary (scripts/build/ci.ts + * inheritOrderFile; getTraceOrderStep in .buildkite/ci.mjs). Locally: * * bun run orderfile # uses build/release, writes build/release/linker.order * bun run orderfile -- --build-dir=build/release-lto diff --git a/test/js/bun/perf/linker-order.test.ts b/test/js/bun/perf/linker-order.test.ts index 39b270d74dad..b339b34de14b 100644 --- a/test/js/bun/perf/linker-order.test.ts +++ b/test/js/bun/perf/linker-order.test.ts @@ -2,12 +2,7 @@ import { describe, expect, it } from "bun:test"; import { bunEnv, bunExe, isMusl, isWindows, nodeExe, tempDir } from "harness"; import { readFileSync } from "node:fs"; import { join } from "node:path"; -import { - mustGenerateOrderFile, - orderFileEligible, - shouldGenerateOrderFile, - type OrderFileContext, -} from "../../../../scripts/build/ci.ts"; +import { orderFileEligible, shouldGenerateOrderFile, type OrderFileContext } from "../../../../scripts/build/ci.ts"; import type { Config } from "../../../../scripts/build/config.ts"; import { linkDepends, @@ -254,6 +249,9 @@ describe("deciding whether a build generates its own order file", () => { }); it("a canary does not by default — it inherits, and pays no second link", () => { + // cfg() can run what it links (linux-aarch64 builds on an aarch64 host), and + // that changes nothing: the next build's file comes from the target's + // trace-order step (.buildkite/ci.mjs), not from this lane. expect(shouldGenerateOrderFile(cfg(), ctx())).toBe(false); }); @@ -261,27 +259,18 @@ describe("deciding whether a build generates its own order file", () => { expect(shouldGenerateOrderFile(cfg(), ctx({ commitMessage: "perf: x [generate symbol order]" }))).toBe(true); }); - it("a pull request never does, and never publishes", () => { + it("a pull request never does, and does not inherit one either", () => { const pr = ctx({ pullRequest: true }); expect(orderFileEligible(cfg(), pr)).toBe(false); expect(shouldGenerateOrderFile(cfg({ canary: false }), pr)).toBe(false); - expect(mustGenerateOrderFile(cfg(), pr, false)).toBe(false); }); - it("a target that cannot run on the host never does", () => { + it("a target that cannot run on the host never does, not even for a release", () => { const cross = cfg({ canRunOnHost: false } as Partial); expect(shouldGenerateOrderFile(cfg({ ...cross, canary: false } as Partial), ctx())).toBe(false); - expect(mustGenerateOrderFile(cross, ctx(), false)).toBe(false); expect(orderFileEligible(cross, ctx())).toBe(true); // ...but it can still inherit one }); - it("a canary that inherited nothing generates anyway, seeding the chain", () => { - // Without this the first build publishes nothing, so the next inherits - // nothing, so it publishes nothing — and no canary is ever ordered. - expect(mustGenerateOrderFile(cfg(), ctx(), false)).toBe(true); - expect(mustGenerateOrderFile(cfg(), ctx(), true)).toBe(false); - }); - it("nothing happens off Buildkite", () => { expect(orderFileEligible(cfg(), ctx({ buildkite: false }))).toBe(false); });