From 645dc2e0da743d244c45c57be949f51b1e547f09 Mon Sep 17 00:00:00 2001 From: niko86 <287100+niko86@users.noreply.github.com> Date: Thu, 20 Aug 2026 18:23:45 +0100 Subject: [PATCH 1/2] ci(bench): measure what the macOS mimalloc pin costs Linux and Windows (#448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2 pin (#294/#301) exists for a fault that is **macOS-specific**: mimalloc v3 defaulted to Apple's fixed TLS slots 108/109, so two co-resident v3 instances shared them regardless of symbol visibility. The pin is not platform-gated, so Linux and Windows carry a macOS fix — and what it costs them has never been measured. #448's own re-run list says to re-measure rather than quote the figure from when the allocator landed. `tools/bench/allocator_ab.py` builds `lat` three ways — v2, v3, and no `#[global_allocator]` at all — and times them on a `forge scale` fixture, which is synthesised from a seed rather than taken from a delivery, so it reproduces anywhere and carries nobody's data. No macOS leg, deliberately. The question is WITHIN-platform: same box, three binaries. Comparing Linux-v3 against macOS-v2 would measure the machines. Four things stop it reporting a number it has not earned. - **The system arm is a positive control.** The perf ledger records mimalloc-vs-system as a real measured win, so if this harness cannot separate system from v2 it cannot resolve allocator effects at all — and "v2 and v3 look the same" would mean the instrument is broken, not that the allocators are equivalent. That verdict is computed, not left to the reader. - **A distinct-binary check.** If the feature swap fails to reach the compiler the three arms are one build measured three times; the run aborts on identical digests rather than reporting it. - **An exact-match patcher.** A manifest that did not change still builds and still runs, reporting a number under the wrong label. A near-miss fails loudly. - **A noise floor from two estimates, cleared by 2x.** A/B/A drift catches wander between arms; the widest within-arm spread catches a jittery box that lands both v2 passes together. An earlier revision called 1.5% "measurably faster" against a 1.0% floor — exactly the over-claim the harness exists to stop. The spawn-per-rep overhead is a constant shared by every arm, so it shrinks the measured gap rather than inflating it. The instrument under-claims, which is the right bias for deciding whether to unpin. Linux runs on the self-hosted pool because a dedicated box is the right instrument for a timing comparison; Windows has no self-hosted runner and takes `windows-2022`, which is noisier — and the harness's own floor is what says whether those numbers are readable rather than a reader having to guess. `workflow_dispatch`-only: it never fires on a push, a PR or a schedule, so it cannot fail a build or enter the required set. Inventoried in the reliquary in this same change, with removal keyed to the PR that records the measurement — "temporary" otherwise survives by nobody remembering. --- .github/workflows/allocator-bench.yml | 122 +++++++++++ ags-wiki/design/reliquary.json | 8 + ags-wiki/design/reliquary.md | 1 + tools/bench/allocator_ab.py | 303 ++++++++++++++++++++++++++ 4 files changed, 434 insertions(+) create mode 100644 .github/workflows/allocator-bench.yml create mode 100644 tools/bench/allocator_ab.py diff --git a/.github/workflows/allocator-bench.yml b/.github/workflows/allocator-bench.yml new file mode 100644 index 00000000..20dd0134 --- /dev/null +++ b/.github/workflows/allocator-bench.yml @@ -0,0 +1,122 @@ +# TEMPORARY — delete this file once #448 has its measurement. +# +# This is an INSTRUMENT, not a gate. It is `workflow_dispatch`-only: it never +# fires on a push, a PR or a schedule, so it cannot fail anyone's build and +# cannot silently become part of the required set. It is inventoried in +# `ags-wiki/design/reliquary.json` for exactly that reason — "temporary" survives +# by nobody remembering, and the reliquary is this repo's answer to that. +# +# ## What it measures, and why there is no macOS leg +# +# The mimalloc v2 pin (#294/#301) exists for a **macOS-specific** fault: v3 +# defaulted to Apple's fixed TLS slots 108/109, so two co-resident v3 instances +# shared them. The pin is not platform-gated, so Linux and Windows carry a macOS +# fix and nobody has measured what it costs them (#448). +# +# That question is WITHIN-platform — same box, three binaries — so a macOS +# baseline would answer a different question. Comparing Linux-v3 against +# macOS-v2 measures the machines. +# +# ## The two legs, and why they are not the same kind of runner +# +# Linux runs on the self-hosted pool: a dedicated box is the right instrument +# for a timing comparison, where GitHub's shared vCPUs are not. Windows has no +# self-hosted runner, so it takes `windows-2022` — which means the Windows +# numbers are noisier, and the harness's own drift probe is what says whether +# they are readable at all rather than a reader having to guess. +# +# The harness (`tools/bench/allocator_ab.py`) carries the method: A/B/A ordering, +# a system-allocator control arm that must separate or the run proves nothing, +# and a distinct-binary check so a failed feature swap cannot report three +# readings of the same build. + +name: "allocator-bench (temporary, #448)" + +on: + workflow_dispatch: + inputs: + reps: + description: "Timed runs per arm (more = steadier, slower)" + required: false + default: "15" + size: + description: "Fixture size for `forge scale` (e.g. 10MB, 25MB)" + required: false + default: "25MB" + +permissions: + contents: read + +jobs: + bench: + name: ${{ matrix.name }} + strategy: + fail-fast: false + matrix: + include: + # The self-hosted pool, by the same label set ci.yml uses. + - name: linux (self-hosted) + runner: '["self-hosted","linux","x64","laterite-linux"]' + # No self-hosted Windows exists; this is the runner release.yml already + # builds the Windows wheel on. + - name: windows (github-hosted) + runner: '"windows-2022"' + runs-on: ${{ fromJSON(matrix.runner) }} + timeout-minutes: 60 + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Installed rather than assumed. Both images probably carry a toolchain, + # but "probably" is the wrong footing for a measurement: an arm built by a + # different compiler than its sibling is not a comparison, and the pin is + # what makes the three arms the same build in every respect but the + # allocator. + - name: Rust toolchain + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + + # A reading is only attributable if the log says what produced it. + - name: State what is under test + shell: bash + run: | + echo "runner: ${{ matrix.name }}" + echo "os: $(uname -a 2>/dev/null || ver)" + rustc --version + cargo --version + "$(command -v python3 || command -v python)" --version + + # Deliberately NOT sccache/mold: the three arms must be built the same way + # as each other, and a shared compile cache that hits for one arm and + # misses for another changes nothing about the RUN but wastes the reader's + # trust in the build log. + - name: Measure + shell: bash + run: | + # `python3` is not a reliable name on the Windows image, where the + # interpreter is on PATH as `python`. Resolve it rather than picking + # one and finding out on the runner. + PY="$(command -v python3 || command -v python)" + "$PY" tools/bench/allocator_ab.py \ + --reps "${{ inputs.reps }}" \ + --size "${{ inputs.size }}" \ + 2>&1 | tee bench.log + + # The verdict line is the point of the run, so put it where the run is + # read rather than only in the log. + - name: Summary + if: always() + shell: bash + run: | + { + echo "## ${{ matrix.name }}" + echo + sed -n '/^| arm |/,$p' bench.log + } >> "$GITHUB_STEP_SUMMARY" + + - name: Keep the readings + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: allocator-bench-${{ strategy.job-index }} + path: bench.log + retention-days: 14 diff --git a/ags-wiki/design/reliquary.json b/ags-wiki/design/reliquary.json index 338d6880..6670ae24 100644 --- a/ags-wiki/design/reliquary.json +++ b/ags-wiki/design/reliquary.json @@ -324,6 +324,14 @@ "status": "spotted", "removed_in": "the release that bumps the engine tier off 0.9.0", "evidence": "Renamed to `is_clean` in #321, when a warning stopped failing a file and \"zero findings\" stopped being the verdict — two public `is_valid` in one crate that disagree on a warning-carrying file. Kept as a `#[deprecated]` delegating alias so the rename reaches a crates.io consumer as a warning naming both replacements rather than a build failure; 0 callers in this tree." + }, + { + "symbol": "`.github/workflows/allocator-bench.yml` + `tools/bench/allocator_ab.py`", + "verify": "`.github/workflows/allocator-bench.yml::allocator-bench (temporary, #448)`", + "axis": "code", + "status": "spotted", + "removed_in": "the PR that records the #448 measurement", + "evidence": "Added deliberately temporary, and inventoried in the same change rather than after it. It is an INSTRUMENT, not a gate: `workflow_dispatch`-only, so it never fires on a push, a PR or a schedule and cannot enter the required set. It answers one question — on Linux and Windows, does mimalloc v3 beat the v2 pin by enough to narrow that pin to `cfg(target_os = \"macos\")`? — and once #448 carries the answer there is nothing left for it to measure. A measurement harness kept past its measurement becomes a build cost nobody can justify and nobody dares delete, which is what this row exists to prevent." } ] } diff --git a/ags-wiki/design/reliquary.md b/ags-wiki/design/reliquary.md index 7559c42e..9d03751a 100644 --- a/ags-wiki/design/reliquary.md +++ b/ags-wiki/design/reliquary.md @@ -94,6 +94,7 @@ it, and name that PR in *Removed-in*). Verify `—` = display-only. | `build_ags4_ipc()` (wasm) | `rust-packages/laterite-ags4-wasm/src/lib.rs::pub fn build_ags4_ipc` | code | keep | — (published, feature-gated) | Inventoried by #349 as never imported by either worker — true (the app's Export pane uses the JSON door `build_ags4`). Not a relic: it is the **Arrow** door of the same verb, gated behind the `arrow` feature, documented in `wasm-api.md` as "build from source" and in `surfaces/browser.md`, and carried in the cross-surface modality register (`modality.json`, held by `packages/laterite/tests/test_modality_parity.py`) — so removing it would drop a modality row and fail that gate. `check-wasm-tier1.mjs` names it among the exports whose ABSENCE defines tier 1, which is a dependency on the name too. | | `dictionary()` (wasm) | `rust-packages/laterite-ags4-wasm/src/lib.rs::pub fn dictionary` | code | keep | — (published surface) | Made caller-less on the web by **this** change (#349): the app's Dictionary browser and Template generator read the static union `ags_dictionary.json` via `lib/dict.ts`, which #349 settled as the web's single dict source, so the worker op and client fn were removed. Recorded here per the relic discipline — add the row in the PR that strands the export — but `keep`, on the same grounds as its four siblings above: it is named in `EXPECTED_FUNCTIONS` in BOTH `tools/release/check-wasm-slim.mjs` and `check-wasm-tier1.mjs` and documented available in `web/docs-site/docs/reference/wasm-api.md`, so it is published npm surface. Its TYPE is still load-bearing web-side even with no caller: `web/src/lib/validator.ts` re-exports `StandardDict` from the engine so the local projection must keep conforming to what this export returns. | | `laterite_ags4_validator::is_valid()` | `rust-packages/laterite-ags4-validator/src/lib.rs::pub fn is_valid` | code | spotted | the release that bumps the engine tier off 0.9.0 | Renamed to `is_clean` in #321, when a warning stopped failing a file and "zero findings" stopped being the verdict — two public `is_valid` in one crate that disagree on a warning-carrying file. Kept as a `#[deprecated]` delegating alias so the rename reaches a crates.io consumer as a warning naming both replacements rather than a build failure; 0 callers in this tree. | +| `.github/workflows/allocator-bench.yml` + `tools/bench/allocator_ab.py` | `.github/workflows/allocator-bench.yml::allocator-bench (temporary, #448)` | code | spotted | the PR that records the #448 measurement | Added deliberately temporary, and inventoried in the same change rather than after it. It is an INSTRUMENT, not a gate: `workflow_dispatch`-only, so it never fires on a push, a PR or a schedule and cannot enter the required set. It answers one question — on Linux and Windows, does mimalloc v3 beat the v2 pin by enough to narrow that pin to `cfg(target_os = "macos")`? — and once #448 carries the answer there is nothing left for it to measure. A measurement harness kept past its measurement becomes a build cost nobody can justify and nobody dares delete, which is what this row exists to prevent. | ## Finding relics — tooling diff --git a/tools/bench/allocator_ab.py b/tools/bench/allocator_ab.py new file mode 100644 index 00000000..a5e2849b --- /dev/null +++ b/tools/bench/allocator_ab.py @@ -0,0 +1,303 @@ +#!/usr/bin/env python3 +"""Measure mimalloc v2 vs v3 vs the system allocator, on one machine. + +## The question this answers, and the one it does not + +`#294`/`#301` pinned mimalloc **v2** on all three native artifacts because v3 +co-resident with another mimalloc corrupts memory. `#448` records that upstream +fixed the mechanism in mimalloc 3.4.4 — but the mechanism is **macOS-specific** +(v3 defaulted to Apple's fixed TLS slots 108/109), while the pin is *not* +platform-gated. So Linux and Windows carry a macOS fix, and nobody has measured +what it costs them. + +That is the question here: **on this machine, does v3 beat v2 by enough to be +worth narrowing the pin to `cfg(target_os = "macos")`?** + +It is a WITHIN-platform comparison — same box, same fixture, three binaries — +which is why it needs no macOS baseline. Comparing Linux-v3 against macOS-v2 +would measure the machines, not the allocator. + +## Why the system arm exists + +It is the **positive control**, and without it a null result is worthless. The +perf ledger records mimalloc-vs-system as a real, measured win when the +allocator landed (`ags-wiki/concepts/perf-campaign.md`, ledger row 10). So if +this harness cannot separate `system` from `v2`, it cannot resolve allocator +effects at all, and "v2 and v3 look the same" means the instrument is broken — +not that the allocators are equivalent. + +## Why the numbers are conservative + +Each rep spawns `lat`, so process start-up sits inside every timing. That +overhead is a constant shared by all arms, so it **shrinks** the measured +relative gap rather than inflating it: a delta that clears the noise floor here +is at least that large at the parse level. Good bias for a "is it worth +unpinning" decision — this instrument under-claims. + +## The noise floor, and why a gap has to clear it twice over + +Criterion drifts run-to-run on the dev machine, and whether a runner is steadier +is itself unmeasured. So the arm order is **A/B/A** — v2, v3, v2 again — and the +noise floor is the WORSE of two independent estimates: that A/B/A drift, and the +widest within-arm spread. Drift alone misses a box that is jittery inside a +single arm but happens to land both v2 passes in the same place. + +A gap then has to clear **2x** the floor to be called a result. Clearing it by a +hair is not one: an earlier revision of this script reported 1.5% as "measurably +faster" against a 1.0% floor, which is the exact over-claim it exists to stop. +The verdict is computed here rather than left to the reader. + +Usage (repo root): + + uv run --no-sync python tools/bench/allocator_ab.py + uv run --no-sync python tools/bench/allocator_ab.py --reps 25 --size 25MB +""" + +from __future__ import annotations + +import argparse +import hashlib +import shutil +import statistics +import subprocess +import sys +import time +from dataclasses import dataclass, field +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent.parent +CRATES = REPO / "rust-packages" +CLI_MANIFEST = CRATES / "laterite-cli" / "Cargo.toml" +CLI_MAIN = CRATES / "laterite-cli" / "src" / "main.rs" +EXE = ".exe" if sys.platform == "win32" else "" + +#: The shipped line, and what each arm rewrites it to. Matched EXACTLY — a +#: near-miss must fail loudly rather than leave the arm silently unpatched, +#: which would report "v3" while measuring v2. +PIN_V2 = 'mimalloc = { version = "0.1", features = ["v2"] }' +PIN_V3 = 'mimalloc = { version = "0.1" }' # libmimalloc-sys defaults to v3 +ALLOC_ATTR = ( + "#[global_allocator]\nstatic GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;" +) +ALLOC_OFF = "// [allocator_ab] system allocator arm — attribute removed for this build" + + +@dataclass +class Arm: + """One built `lat`, and the readings taken from it.""" + + label: str + binary: Path | None = None + digest: str = "" + times: list[float] = field(default_factory=list) + + def report(self) -> tuple[float, float]: + """min, median — in that order, because min is the least noisy statistic + for a CPU-bound run and the median is the sanity check on it.""" + return min(self.times), statistics.median(self.times) + + +def run(cmd: list[str], cwd: Path) -> None: + print(f" $ {' '.join(cmd)}", flush=True) + subprocess.run(cmd, cwd=cwd, check=True) + + +def patch(path: Path, old: str, new: str) -> None: + """Rewrite `old` to `new`, failing if `old` was not there. + + The failure mode this guards is the quiet one: a manifest that did not + change still builds, still runs, and reports a number under the wrong + label.""" + text = path.read_text(encoding="utf-8") + if old not in text: + raise SystemExit( + f"allocator_ab: {path.relative_to(REPO)} does not contain the expected " + f"text — the source moved under this script:\n {old!r}" + ) + path.write_text(text.replace(old, new, 1), encoding="utf-8") + + +def build_arm(label: str, out_dir: Path) -> Arm: + """Patch → build → stash the binary → restore. Restores on any failure.""" + manifest = CLI_MANIFEST.read_text(encoding="utf-8") + main_rs = CLI_MAIN.read_text(encoding="utf-8") + try: + if label == "v3": + patch(CLI_MANIFEST, PIN_V2, PIN_V3) + elif label == "system": + patch(CLI_MAIN, ALLOC_ATTR, ALLOC_OFF) + elif label != "v2": + raise SystemExit(f"allocator_ab: unknown arm {label!r}") + + print(f"\n=== building arm: {label} ===", flush=True) + run(["cargo", "build", "--release", "-p", "laterite-cli"], cwd=CRATES) + + src = CRATES / "target" / "release" / f"lat{EXE}" + dst = out_dir / f"lat-{label}{EXE}" + shutil.copy2(src, dst) + digest = hashlib.sha256(dst.read_bytes()).hexdigest()[:16] + print(f" built {dst.name} sha256:{digest}", flush=True) + return Arm(label=label, binary=dst, digest=digest) + finally: + CLI_MANIFEST.write_text(manifest, encoding="utf-8") + CLI_MAIN.write_text(main_rs, encoding="utf-8") + + +def ensure_fixture(size: str, out: Path) -> Path: + """`forge scale` synthesises a valid AGS4 file to a target size, reproducibly + from a seed — so no real delivery is involved and any machine gets the same + bytes. Same generator `tools/gen-bench-fixtures.sh` uses.""" + if out.exists(): + print(f"fixture present: {out}", flush=True) + return out + forge = CRATES / "target" / "release" / f"laterite-ags4-forge{EXE}" + if not forge.exists(): + run(["cargo", "build", "--release", "-p", "laterite-ags4-forge"], cwd=CRATES) + out.parent.mkdir(parents=True, exist_ok=True) + run( + [ + str(forge), + "scale", + "--size", + size, + "--scaffold", + "wide", + "--seed", + "0", + "--out", + str(out), + ], + cwd=REPO, + ) + return out + + +def time_arm(arm: Arm, fixture: Path, reps: int) -> None: + """One warm-up (page cache, and the first run of any binary is not like the + rest), then `reps` timed runs of the same command.""" + assert arm.binary is not None + cmd = [str(arm.binary), "validate", str(fixture), "--no-warnings"] + subprocess.run(cmd, capture_output=True) + for _ in range(reps): + t0 = time.perf_counter() + subprocess.run(cmd, capture_output=True) + arm.times.append((time.perf_counter() - t0) * 1000.0) + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--reps", type=int, default=15, help="timed runs per arm") + ap.add_argument("--size", default="25MB", help="forge scale target size") + ap.add_argument( + "--out-dir", + type=Path, + default=REPO / "output" / "allocator-ab", + help="where the built binaries and the fixture go (gitignored)", + ) + args = ap.parse_args() + + args.out_dir.mkdir(parents=True, exist_ok=True) + fixture = ensure_fixture(args.size, args.out_dir / f"scale-{args.size}.ags") + + built = {label: build_arm(label, args.out_dir) for label in ("v2", "v3", "system")} + + # A build that did not actually change is the failure this catches: if two + # arms are byte-identical, the feature swap did not reach the compiler and + # every number below is the same binary measured three times. + digests = {a.label: a.digest for a in built.values()} + if len(set(digests.values())) != len(digests): + raise SystemExit(f"allocator_ab: arms are not distinct binaries — {digests}") + + # A/B/A: the second v2 pass is the drift probe, and it has to be a SEPARATE + # reading rather than a re-use of the first, so order it explicitly. + aba = Arm(label="v2 (again)", binary=built["v2"].binary, digest=built["v2"].digest) + order = [built["v2"], built["v3"], aba, built["system"]] + + print(f"\n=== timing: {args.reps} reps/arm on {fixture.name} ===", flush=True) + for arm in order: + time_arm(arm, fixture, args.reps) + lo, med = arm.report() + print(f" {arm.label:<12} min {lo:8.1f} ms median {med:8.1f} ms", flush=True) + + v2_lo = min(built["v2"].times) + v3_lo = min(built["v3"].times) + aba_lo = min(aba.times) + sys_lo = min(built["system"].times) + + # ONE sign convention for the whole report: every percentage is measured + # against v2, and NEGATIVE MEANS FASTER. Two ways of saying the same delta — + # a table column relative to v2 and a prose line phrased as "v2 → v3" — + # print the same number with opposite signs, which is how a reader ends up + # concluding the reverse of the result. + def vs_v2(value: float) -> float: + return (value - v2_lo) / v2_lo * 100.0 + + drift = abs(aba_lo - v2_lo) + delta = abs(v3_lo - v2_lo) + control = abs(sys_lo - v2_lo) + + # Two independent noise estimates, and the pessimistic one wins. The A/B/A + # drift catches slow wander between arms; the within-arm spread catches a + # box that is jittery inside a single arm, which drift alone can miss when + # both v2 passes happen to land in the same place. + spread = max(statistics.median(a.times) - min(a.times) for a in order) + noise = max(drift, spread) + + # A delta that clears the noise by a hair is not a result. Requiring 2x is + # arbitrary in the way every threshold is, but the alternative is worse: an + # earlier revision of this script called 1.5% "measurably faster" against a + # 1.0% noise floor, which is precisely the over-claim it exists to stop. + MARGIN = 2.0 + + lines = [ + "", + "| arm | min (ms) | median (ms) | vs v2 (− = faster) |", + "|---|---:|---:|---:|", + ] + for arm in order: + lo, med = arm.report() + rel = "—" if arm.label.startswith("v2") else f"{vs_v2(lo):+.1f}%" + lines.append(f"| `{arm.label}` | {lo:.1f} | {med:.1f} | {rel} |") + lines += [ + "", + f"- **noise floor: {noise:.1f} ms** ({noise / v2_lo * 100:.1f}%) — the worse of " + f"the A/B/A drift ({drift:.1f} ms) and the widest within-arm spread " + f"({spread:.1f} ms). A gap must clear **{MARGIN:g}x** this to count.", + f"- **v3 vs v2** — {'faster' if v3_lo < v2_lo else 'slower'} by {delta:.1f} ms " + f"({abs(vs_v2(v3_lo)):.1f}%).", + f"- **control, system vs v2** — {'faster' if sys_lo < v2_lo else 'slower'} by " + f"{control:.1f} ms ({abs(vs_v2(sys_lo)):.1f}%). mimalloc must come out ahead " + f"here, or the harness is not resolving allocators at all.", + "", + ] + + if sys_lo < v2_lo or control < MARGIN * noise: + lines.append( + "**VERDICT: the instrument proved nothing.** The system allocator did not " + "come out behind mimalloc by more than this run's own drift, so the harness " + "cannot resolve allocator effects at this scale. The v2/v3 comparison above " + "it is meaningless — more reps, a quieter box, or a larger rung, before " + "reading anything into it." + ) + elif delta < MARGIN * noise: + lines.append( + "**VERDICT: v2 and v3 are indistinguishable here.** The control separated, " + "so the instrument works; the v2/v3 gap does not clear the noise floor by " + "the required margin. On the evidence of this run, narrowing the pin to " + "macOS would buy this platform nothing measurable." + ) + else: + faster = "v3" if v3_lo < v2_lo else "v2" + lines.append( + f"**VERDICT: {faster} is measurably faster**, by more than this run's drift. " + "Remember the spawn overhead makes this conservative — the parse-level gap " + "is at least this large. Worth taking to #448." + ) + + out = "\n".join(lines) + print(out, flush=True) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From e30b8a834539179ab2127fe791dbe8925ba27f12 Mon Sep 17 00:00:00 2001 From: niko86 <287100+niko86@users.noreply.github.com> Date: Thu, 20 Aug 2026 19:00:29 +0100 Subject: [PATCH 2/2] ci(bench): point the Windows leg at the self-hosted VM, and stop assuming a shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows runner is registered on this repo now (`laterite-windows`), so the leg becomes switchable rather than pinned to `windows-2022`: a `windows_runner` dispatch input picks between them, defaulting to the github-hosted one so the job still works if the VM is offline. The runner expression is one line on purpose — a folded scalar keeps its newlines inside the `${{ }}`, and a bad runner label does not error, it queues until the timeout. Two corrections that `release.yml` had already paid for on this same VM, and that this job would otherwise have rediscovered: - **bash is not on PATH for the Windows runner service.** Four steps here used `shell: bash`. Split per-OS, as release.yml does for exactly this reason. - **The default Windows shell is `pwsh`, which is not pre-installed.** A step with no `shell:` fails there. `powershell` is the built-in 5.1, and `-ExecutionPolicy Bypass` gets past the Win11 default of Restricted per-step rather than machine-wide. Python comes from uv rather than from the box — it is not on PATH there, and installing it would make the job depend on hand-configured machine state, which the github-hosted fallback leg would not share. It is also the house pattern: nothing here uses `setup-python`, and release.yml already proves the uv path on this VM. The rest is the harness taking back what the workflow could not say portably. It now prints its own toolchain banner, writes its own `bench.log`, and appends its own table to `GITHUB_STEP_SUMMARY` — so the `tee`/`sed` pipeline is gone along with the third shell-shaped step, and a local run produces the same artifacts as a CI one. Verified by running it with `GITHUB_STEP_SUMMARY` set: log written, summary written, and the verdict correct. At 4 reps on the 1 MB rung it reported *"the instrument proved nothing"* — the control did not clear 2x the noise floor — which is the guard working on a deliberately under-powered run rather than reporting the 2.2% v3 gap sitting above it. --- .github/workflows/allocator-bench.yml | 116 ++++++++++++++++++-------- tools/bench/allocator_ab.py | 40 ++++++++- 2 files changed, 118 insertions(+), 38 deletions(-) diff --git a/.github/workflows/allocator-bench.yml b/.github/workflows/allocator-bench.yml index 20dd0134..295a7db7 100644 --- a/.github/workflows/allocator-bench.yml +++ b/.github/workflows/allocator-bench.yml @@ -43,6 +43,14 @@ on: description: "Fixture size for `forge scale` (e.g. 10MB, 25MB)" required: false default: "25MB" + windows_runner: + description: "Windows leg: github-hosted, or the self-hosted box once it is registered on THIS repo" + required: false + default: github-hosted + type: choice + options: + - github-hosted + - self-hosted permissions: contents: read @@ -57,10 +65,31 @@ jobs: # The self-hosted pool, by the same label set ci.yml uses. - name: linux (self-hosted) runner: '["self-hosted","linux","x64","laterite-linux"]' - # No self-hosted Windows exists; this is the runner release.yml already - # builds the Windows wheel on. - - name: windows (github-hosted) - runner: '"windows-2022"' + # Windows is switchable, because the good runner is not reachable from + # here YET. A self-hosted Windows box exists — `ags5-win11-runner` — + # but it is registered on the PRIVATE satellite, and `niko86` is a User + # account rather than an Organization, so there are no runner groups to + # share it through: a runner belongs to the repo it was configured + # against. Asking for those labels here does not fail, it QUEUES, until + # the job times out an hour later. + # + # Registering the same machine a second time against this repo is the + # fix (one box can host several runner services). Label it + # `laterite-windows`, to match `laterite-linux` and because + # `ags5-win11` carries a name that outlived what this repo holds. Then + # dispatch with `windows_runner: self-hosted` — no edit needed here. + # + # Until then the default is `windows-2022`, which release.yml already + # builds the Windows wheel on. Its numbers are noisier; the harness's + # own noise floor is what says whether they are readable. + # + # One line on purpose: a folded scalar here keeps its newlines inside + # the `${{ }}` (the continuations are more-indented, so YAML does not + # fold them), and this is the wrong place to find out whether the + # expression parser minds — a bad runner label does not error, it + # queues until the timeout. + - name: windows (${{ inputs.windows_runner }}) + runner: "${{ inputs.windows_runner == 'self-hosted' && '[\"self-hosted\",\"windows\",\"x64\",\"laterite-windows\"]' || '\"windows-2022\"' }}" runs-on: ${{ fromJSON(matrix.runner) }} timeout-minutes: 60 @@ -75,48 +104,63 @@ jobs: - name: Rust toolchain uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable - # A reading is only attributable if the log says what produced it. - - name: State what is under test + # Python comes from uv rather than from the box. The Windows runner has no + # interpreter on PATH, and "install Python on the runner" would make the + # job depend on hand-configured machine state — the thing a self-healing + # pool exists to avoid, and something the github-hosted fallback leg would + # not share. This is also the house pattern: nothing here uses + # `setup-python`; every workflow gets its interpreter from uv, and + # release.yml already proves that path on a self-hosted Windows VM. + # + # The harness is stdlib-only on purpose, so `--no-project` is enough — no + # sync, no lockfile, no wheel build. + - name: Install uv + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 + with: + enable-cache: true + + # SPLIT BY SHELL, copying release.yml, which learned both of these on the + # very Windows VM this job now targets: + # + # * bash is not on PATH for the Windows runner service, so `shell: bash` + # is not portable across these legs; + # * the DEFAULT Windows shell is `pwsh` (PowerShell 7+), which is not + # pre-installed — so a step with no `shell:` fails there. `powershell` + # is the built-in 5.1, and `-ExecutionPolicy Bypass` gets past the + # Win11 default of Restricted, per-step rather than machine-wide. + # + # Everything else the job needs to say — the toolchain versions, the + # results table, the job summary — the harness does for itself, precisely + # so there is no third shell-shaped step to keep portable. + - name: Install Python (Linux / macOS) + if: runner.os != 'Windows' shell: bash - run: | - echo "runner: ${{ matrix.name }}" - echo "os: $(uname -a 2>/dev/null || ver)" - rustc --version - cargo --version - "$(command -v python3 || command -v python)" --version + run: uv python install 3.14 + + - name: Install Python (Windows) + if: runner.os == 'Windows' + shell: powershell -NoProfile -ExecutionPolicy Bypass -File {0} + run: uv python install 3.14 # Deliberately NOT sccache/mold: the three arms must be built the same way # as each other, and a shared compile cache that hits for one arm and - # misses for another changes nothing about the RUN but wastes the reader's - # trust in the build log. - - name: Measure + # misses for another changes nothing about the RUN but plenty about how + # much the build log can be trusted. + - name: Measure (Linux / macOS) + if: runner.os != 'Windows' shell: bash - run: | - # `python3` is not a reliable name on the Windows image, where the - # interpreter is on PATH as `python`. Resolve it rather than picking - # one and finding out on the runner. - PY="$(command -v python3 || command -v python)" - "$PY" tools/bench/allocator_ab.py \ - --reps "${{ inputs.reps }}" \ - --size "${{ inputs.size }}" \ - 2>&1 | tee bench.log + run: uv run --no-project python tools/bench/allocator_ab.py --reps "${{ inputs.reps }}" --size "${{ inputs.size }}" - # The verdict line is the point of the run, so put it where the run is - # read rather than only in the log. - - name: Summary - if: always() - shell: bash - run: | - { - echo "## ${{ matrix.name }}" - echo - sed -n '/^| arm |/,$p' bench.log - } >> "$GITHUB_STEP_SUMMARY" + - name: Measure (Windows) + if: runner.os == 'Windows' + shell: powershell -NoProfile -ExecutionPolicy Bypass -File {0} + run: uv run --no-project python tools/bench/allocator_ab.py --reps "${{ inputs.reps }}" --size "${{ inputs.size }}" - name: Keep the readings if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: allocator-bench-${{ strategy.job-index }} - path: bench.log + path: output/allocator-ab/bench.log retention-days: 14 + if-no-files-found: warn diff --git a/tools/bench/allocator_ab.py b/tools/bench/allocator_ab.py index a5e2849b..f4e506e8 100644 --- a/tools/bench/allocator_ab.py +++ b/tools/bench/allocator_ab.py @@ -57,6 +57,8 @@ import argparse import hashlib +import os +import platform import shutil import statistics import subprocess @@ -184,6 +186,40 @@ def time_arm(arm: Arm, fixture: Path, reps: int) -> None: arm.times.append((time.perf_counter() - t0) * 1000.0) +def state_the_toolchain() -> None: + """A reading is only attributable if the output says what produced it. + + This lives here rather than in the workflow because the workflow cannot say + it portably: the Linux legs have bash and the self-hosted Windows VM has + neither bash nor pwsh on PATH (`release.yml` records both facts). Anything + the harness can say about itself, it should.""" + print(f"platform: {platform.platform()} python {platform.python_version()}") + for tool in ("rustc", "cargo"): + try: + out = subprocess.run( + [tool, "--version"], capture_output=True, text=True, check=True + ) + print(f"{tool}: {out.stdout.strip()}") + except (OSError, subprocess.CalledProcessError) as e: + raise SystemExit(f"allocator_ab: {tool} is not usable here: {e}") from e + + +def emit(text: str, log: Path) -> None: + """stdout for the run log, the file for the artifact, and — on a runner — + the job summary, so the verdict lands where the run is actually read rather + than only at the bottom of a log nobody opens. + + Done here rather than by piping through `tee` and `sed` in the workflow for + the same portability reason as above: that pipeline needs a shell both + platforms do not share.""" + print(text, flush=True) + log.write_text(text, encoding="utf-8") + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with Path(summary).open("a", encoding="utf-8") as fh: + fh.write(text + "\n") + + def main() -> int: ap = argparse.ArgumentParser(description=__doc__) ap.add_argument("--reps", type=int, default=15, help="timed runs per arm") @@ -197,6 +233,7 @@ def main() -> int: args = ap.parse_args() args.out_dir.mkdir(parents=True, exist_ok=True) + state_the_toolchain() fixture = ensure_fixture(args.size, args.out_dir / f"scale-{args.size}.ags") built = {label: build_arm(label, args.out_dir) for label in ("v2", "v3", "system")} @@ -294,8 +331,7 @@ def vs_v2(value: float) -> float: "is at least this large. Worth taking to #448." ) - out = "\n".join(lines) - print(out, flush=True) + emit("\n".join(lines), args.out_dir / "bench.log") return 0