From 68d6f9080aeab81ef1ae54147bb2d52ecdac7c7b Mon Sep 17 00:00:00 2001 From: Alireza Date: Thu, 9 Jul 2026 13:22:39 -0400 Subject: [PATCH 1/2] chore(charls): update extern/charls submodule to upstream CharLS 2.4.4 Advances from 38d95d0 (~2.4.1-era, Jan 2023) to upstream 2.4.4 (2026-06-08). No custom fork patches (clean version advance). Fork PR: cornerstonejs/charls#1. CI is the first build/validation of 2.4.4 against our glue. --- packages/charls/extern/charls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/charls/extern/charls b/packages/charls/extern/charls index 38d95d00..9930a2a2 160000 --- a/packages/charls/extern/charls +++ b/packages/charls/extern/charls @@ -1 +1 @@ -Subproject commit 38d95d00671f4cddfa61f3f51eaf81b8bac34543 +Subproject commit 9930a2a2fa75f516c4a08708180c9907fa501a97 From 00283a9764cc08b8e38a3911d00bce4bb75fa1fa Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Tue, 8 Sep 2026 15:17:54 -0400 Subject: [PATCH 2/2] ci: run the walltime bench after the simulation gate; give pr-checks a per-commit push group The CodSpeed app computes its single "CodSpeed Performance Analysis" check from the FIRST upload a commit produces, and never re-evaluates it. While codspeed-walltime ran beside codspeed-bench in pr-checks.yml it finished 2-7 minutes earlier on every commit measured, so the advisory instrument decided the check and the simulation gate never spoke: commit walltime check codspeed-bench verdict ab49563 18:36:45 18:37:16 18:39:29 failure -32.65% bac71dd 17:57:33 17:57:46 18:00:03 failure -19.77% 4ce83c9 17:52:55 17:53:20 17:56:29 failure 073884c 19:44:51 19:45:17 19:47:24 failure 5bfa7ff 21:27:58 21:28:22 21:34:56 success continue-on-error: true does not prevent this. It sets that JOB's conclusion, while the app posts an independent check run that no job setting can mark advisory. Ordering is the only lever available in the repo, so codspeed-walltime moves to bench.yml with needs: codspeed-bench. Ordering via needs: rather than a poll inside the job also keeps the metered macro runner unallocated while it waits. pr-checks.yml kept a branch-level push concurrency group after #93 gave bench.yml a per-commit one. On a push head_ref is empty, so every main push shared one group with cancel-in-progress: true, and the release workflow's version commit cancelled the merge commit's run before being skipped itself by the detect-changes guard -- GitHub applies concurrency when a run is queued, before it evaluates any if:. 18:33 c9ffa62 fix(release): preflight the registry... cancelled 18:39 c44693e chore(release): publish skipped The two faults compounded. Cancelling pr-checks killed walltime, so simulation won the first-upload race by default and the check compared bac71dd's walltime number against c9ffa62's simulation number: JPEG XL Lossless (.110) as 158.5 ms -> 991.8 ms. That 6.3x ratio is the documented 5-15x simulation-inflation band for wasm decode, not a regression. BENCHMARKING.md records both faults, corrects the job locations, and drops the stale claim that simulation "keeps --parallel" (#89 serialised it). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bench.yml | 190 +++++++++++++++++++++++++++++++- .github/workflows/pr-checks.yml | 181 +++++++++--------------------- BENCHMARKING.md | 53 +++++++-- 3 files changed, 284 insertions(+), 140 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 04fc7075..14d8b0bf 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -1,17 +1,24 @@ name: Bench -# CodSpeed simulation bench (the blocking perf regression gate), split out of +# Both CodSpeed instruments, in the order they must run. Split out of # pr-checks.yml so the workflow that talks to the self-hosted bench runner is # small and changes rarely. It reuses the dist artifacts built by the # "PR checks" run for the same commit (the `wait` step below), so nothing is # compiled twice. # -# Two jobs: +# Three jobs: # gate — hosted VM, metadata only (never checks out or executes PR code). # Decides whether the bench should run and what to bench, then waits # for the PR checks build artifacts. -# codspeed-bench — the bench itself on the self-hosted box (see the comments -# on the job). +# codspeed-bench — the simulation gate, the blocking perf regression check, +# on the self-hosted box (see the comments on the job). +# codspeed-walltime — the advisory real-time instrument, on a CodSpeed macro +# runner. It lives HERE, after codspeed-bench, rather than in +# pr-checks.yml beside it, because the CodSpeed app reports whichever +# instrument uploads FIRST. See the comment on that job. +# +# Only codspeed-bench runs on the shared nashua box. The macro-runner job is in +# this file for the `needs:` ordering, and touches nothing that box provides. # # PRs that modify CI-defining files (workflows, tools/ci/, root manifests) are # only benched pre-merge when they come from a branch in THIS repo; from a fork @@ -476,3 +483,178 @@ jobs: # under contention, so the first main run after merge is the new # baseline -- expect one round of large apparent deltas there. run: bash tools/ci/with-nashua-lock.sh pnpm --workspace-concurrency=1 $SCOPE_FLAGS run bench + + codspeed-walltime: + # Moved here from pr-checks.yml, where it ran BESIDE codspeed-bench rather + # than after it. That ordering decided which instrument GitHub reported, + # because the CodSpeed app computes its "CodSpeed Performance Analysis" + # check from the FIRST upload a commit produces and never re-evaluates it. + # Walltime finished 2-7 minutes earlier every time, so the ADVISORY + # instrument always won the race and the simulation gate never spoke: + # + # commit walltime check codspeed-bench verdict + # ab49563 18:36:45 18:37:16 18:39:29 failure -32.65% + # bac71dd 17:57:33 17:57:46 18:00:03 failure -19.77% + # 4ce83c9 17:52:55 17:53:20 17:56:29 failure + # 073884c 19:44:51 19:45:17 19:47:24 failure + # 5bfa7ff 21:27:58 21:28:22 21:34:56 success + # + # `continue-on-error: true` below does NOT prevent this: it sets this JOB's + # conclusion, while the CodSpeed app posts an independent check run that no + # job setting can mark advisory. Ordering is the only lever in the repo. + # + # It also mixed the two instruments within one series. On c9ffa62 the + # release commit cancelled this job (see pr-checks.yml's concurrency + # comment), simulation won the race by default, and the check compared + # bac71dd's walltime number against c9ffa62's simulation number: + # `JPEG XL Lossless (.110)` 158.5 ms -> 991.8 ms, a 6.3x ratio that sits + # inside the documented 5-15x simulation-inflation band for wasm decode. + # + # `needs: codspeed-bench` is what fixes both: simulation always uploads + # first, so it always decides the check, and this job is what it was + # documented to be. The cost is that a failed or cancelled bench takes + # walltime with it -- accepted, because an out-of-order walltime run is + # worse than a missing one. + needs: [gate, codspeed-bench] + # Deliberately NOT `always()`. Waiting for the bench is the entire point, + # and `always()` would let this job run when codspeed-bench was cancelled + # or skipped -- exactly the case that produced the 991.8 ms comparison. + # + # Gated behind a repository variable, as it was in pr-checks.yml: a + # `runs-on: codspeed-macro` job queues forever (up to 24h) when macro + # runners aren't provisioned for the org, and GitHub's job timeout only + # covers execution, not queue time. Enable macro runners for the org on + # app.codspeed.io first (the repo is public -- also make sure the runner + # group allows public repositories), then: + # gh variable set CODSPEED_MACRO_ENABLED --body true + # Turning this variable off leaves simulation as the ONLY instrument, which + # is a baseline re-seed event in both directions: the stored numbers change + # instrument, so expect one round of large apparent deltas either way. + if: needs.gate.outputs.ready == 'true' && vars.CODSPEED_MACRO_ENABLED == 'true' + # Advisory instrument: real wall-clock numbers (V8 JIT active, real + # cache/branch behavior) that complement the simulation gate above -- + # simulation catches small algorithmic slips deterministically, + # walltime keeps the numbers honest on real hardware and covers the + # pure-JS packages where the no-JIT simulation model is furthest from + # production. Failures here must not block the PR while this beds in. + continue-on-error: true + timeout-minutes: 30 + permissions: + contents: read + actions: read # cross-run artifact download from the PR checks run + pull-requests: write # CodSpeed action posts a sticky PR comment + id-token: write # OIDC token used by CodSpeedHQ/action for auth + # CodSpeed-managed 16-core ARM64 bare-metal machine, tuned for + # low-noise walltime measurement. Requires the CodSpeed GitHub app on + # an organization account. NOTE: ARM64 -- anything cached must be + # arch-qualified (see the cache key below). + # + # These runners are METERED, unlike the self-hosted bench box. Ordering via + # `needs:` rather than a poll inside this job is what keeps that cheap: + # GitHub allocates the runner when the job STARTS, so the wait for the + # bench costs nothing. A wait step here would hold a billed macro runner + # idle for the whole 7-12 minute bench. + runs-on: codspeed-macro + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + # Pinned for the same reason as the simulation job above: V8 changes + # between patch releases move the numbers, and walltime is if anything + # more sensitive than instruction counts. Keep this in step with the + # codspeed-bench pin so the two instruments stay comparable. + node-version: '24.20.0' + - name: Provide pnpm via Corepack + run: | + corepack enable pnpm + corepack prepare --activate + pnpm --version + - name: Download all built dists + uses: actions/download-artifact@v4 + with: + pattern: dist-* + path: tmp/ + # Cross-run download, new since this job moved out of pr-checks.yml: + # the artifacts live on the PR checks run the gate waited for, not on + # this run. Same three arguments codspeed-bench uses above. + run-id: ${{ needs.gate.outputs.run_id }} + github-token: ${{ github.token }} + - name: Replay dists into packages//dist + run: | + set -e + for d in tmp/dist-*; do + [ -d "$d" ] || continue + pkg=$(basename "$d" | sed 's/^dist-//') + mkdir -p "packages/$pkg/dist" + shopt -s dotglob nullglob + cp -r "$d"/* "packages/$pkg/dist/" 2>/dev/null || true + done + - name: Restore node_modules cache + id: modules-cache + uses: actions/cache@v4 + with: + path: | + node_modules + packages/*/node_modules + # runner.arch matters: this job runs on ARM64 while every other + # job is x64; sharing a key would restore x64 native binaries + # (esbuild/rollup) and break vitest. + # Manifests + workspace config in the key -- see pr-checks.yml's build + # job cache step for why the lockfile alone is not enough. + key: pnpm-modules-node24-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('package.json', 'packages/*/package.json', 'pnpm-lock.yaml', 'pnpm-workspace.yaml') }} + - name: Install dependencies + if: steps.modules-cache.outputs.cache-hit != 'true' + run: pnpm install --frozen-lockfile + - name: Log CPU info + run: lscpu | grep -E "Model name|Cache|Flags" | head -5 || true + - name: Compute bench scope + # Reads the gate's scope, as codspeed-bench does, rather than + # detect-changes' `bench` output in the other workflow. The two lists + # are kept in sync by hand (see the gate job), so taking this one keeps + # both instruments measuring exactly the same package set. + id: scope + env: + BENCH: ${{ needs.gate.outputs.bench }} + run: | + set -euo pipefail + flags="" + for pkg in $(echo "$BENCH" | jq -r '.[]'); do + # Untrusted: fork PRs control this value (it comes from the PR's own + # packages//package.json). Validate against npm's name grammar + # before it reaches GITHUB_OUTPUT or any command line. Must be a + # whole-string check: keep it in node rather than a line-based tool. + # Keep in step with the same check in the codspeed-bench job above. + name=$(node -e ' + const pkg = process.argv[1]; + const { name } = require(`./packages/${pkg}/package.json`); + if (typeof name !== "string" || + !/^(@[a-z0-9][a-z0-9._-]*\/)?[a-z0-9][a-z0-9._-]*$/.test(name)) { + console.error(`::error::Rejected package name for ${pkg}`); + process.exit(1); + } + process.stdout.write(name); + ' "$pkg") + flags="$flags --filter $name" + done + echo "Bench scope flags:$flags" + echo "flags=$flags" >> "$GITHUB_OUTPUT" + - name: Run CodSpeed benchmarks (walltime) + # Walltime measures actual elapsed time, so parallel benchmark + # processes would contend for cores and add noise -- run packages + # sequentially (--workspace-concurrency=1). The simulation job above + # now does the same: it was left parallel on the premise that + # instruction counting is immune to contention, and #76 showed it is + # not. See the comment on that job's run step. + # + # No nashua lock here: this runs on a CodSpeed macro runner, not on the + # shared box, so there is nothing to serialise against. + uses: CodSpeedHQ/action@4e969336ab9acd4f6f8d025fdd793292b0835df0 # v4.18.2 + env: + # Keep this in env, NOT `${{ }}` in the run: below -- an env value is + # expanded by the shell after the command line is parsed, so it stays + # data. Unquoted below on purpose: the flags must word-split into + # repeated `--filter ` pairs. + SCOPE_FLAGS: ${{ steps.scope.outputs.flags }} + with: + mode: walltime + run: pnpm --workspace-concurrency=1 $SCOPE_FLAGS run bench diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 192a9adf..e2af0554 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -2,10 +2,10 @@ name: PR checks # Pipeline: builds run as a per-package parallel matrix (wasm compiles are # the slow part), then a single test job runs the whole vitest workspace -# against the built dists. The CodSpeed simulation bench lives in bench.yml, +# against the built dists. BOTH CodSpeed instruments now live in bench.yml, # which waits for THIS workflow's dist artifacts and benches the packages -# the PR touched; the advisory walltime bench stays here (codspeed-walltime -# below). +# the PR touched: the simulation gate first, then the advisory walltime job +# after it. # # Toolchain bumps (emsdk image tag below, root package.json/pnpm-lock.yaml, # this workflow itself) force the FULL pipeline including a full bench sweep. @@ -39,12 +39,49 @@ on: # dashboard (to seed initial perf data after the repo is connected). workflow_dispatch: -# Cancel in-flight runs when a new push lands on the same PR / branch. +# Cancel in-flight runs when a new push lands on the same PR. +# +# Keyed per-commit off a PR, the same way bench.yml is, and because of the same +# incident. On a push `head_ref` is empty, so `github.head_ref || github.ref` +# put every main push into the single group +# `pr-checks-PR checks-refs/heads/main` with cancel-in-progress: true. The +# release workflow's version commit -- pushed about five minutes after the +# merge that triggered it -- entered that group and CANCELLED the merge +# commit's run, then skipped its own jobs through the detect-changes guard +# below. GitHub applies concurrency when a run is QUEUED, before it evaluates +# any `if:`, so a run that goes on to do nothing still cancels its +# predecessor: +# +# 18:33 c9ffa62 fix(release): preflight the registry... cancelled +# 18:39 c44693e chore(release): publish skipped +# +# On c9ffa62 that killed `codspeed-walltime` at 18:39:11 with every other job +# already green. The cost is not one lost job: the CodSpeed check is computed +# from the FIRST upload a commit produces and is never re-evaluated, so +# cancelling walltime hands the check to the simulation bench instead. The two +# instruments differ by 5-15x on wasm decode (see BENCHMARKING.md), so +# c9ffa62 stored a simulation-sourced number for a benchmark whose predecessor +# had stored a walltime-sourced one, and the check reported `JPEG XL Lossless +# (.110)` as 158.5 ms -> 991.8 ms on a diff that touches only tools/release/ +# and workflows. +# +# c9ffa62 fixed this in bench.yml but not here, and the bench.yml fix is not +# sufficient on its own: bench.yml's gate waits on THIS workflow's build +# artifacts, so a run cancelled here starves the bench there. +# +# Keying non-PR runs by github.sha gives every main commit its own group, so no +# main push can cancel another. cancel-in-progress stays true for +# pull_request, where superseding an in-flight run with a newer push is +# exactly what is wanted. See bench.yml's concurrency comment for why +# per-commit keying is needed rather than cancel-in-progress: false alone -- a +# group holds one running run and at most one PENDING run, and queueing a +# third evicts the pending one whatever cancel-in-progress says. concurrency: - group: pr-checks-${{ github.workflow }}-${{ github.head_ref || github.ref }} - cancel-in-progress: true + group: pr-checks-${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} -# Job-level permissions where a job needs more (codspeed-walltime). +# Job-level permissions where a job needs more (detect-changes reads the PR's +# changed files). permissions: contents: read @@ -74,10 +111,12 @@ jobs: # bench — only the packages that actually changed. CodSpeed benches # run under valgrind (the slowest thing in CI), and a PR only needs # deltas for what it touched; main re-benches everything to keep - # full baselines. Consumed by codspeed-walltime below; the - # simulation bench in bench.yml computes the same scope from the - # PR's changed-file list — keep TOOLCHAIN_PATHS below in sync with - # the path lists in bench.yml's gate job. + # full baselines. No job in THIS workflow consumes it any more — + # both bench jobs live in bench.yml, whose gate job computes the + # same scope from the PR's changed-file list. It is kept because + # the two lists must agree: keep TOOLCHAIN_PATHS below in sync with + # the path lists in bench.yml's gate job, and this output is what + # makes a divergence visible in this run's own log. # # Bounded like every other job here: without an explicit timeout a job that # wedges runs to GitHub's 6-hour default. 13s typical, so 10 is ~45x @@ -482,120 +521,6 @@ jobs: - name: Browser smoke decode run: node tools/browser-smoke/run.js - codspeed-walltime: - needs: [detect-changes, build] - # Gated behind a repository variable: a `runs-on: codspeed-macro` job - # queues forever (up to 24h) when macro runners aren't provisioned for - # the org, and GitHub's job timeout only covers execution, not queue - # time. Enable macro runners for the org on app.codspeed.io first - # (the repo is public — also make sure the runner group allows public - # repositories), then: gh variable set CODSPEED_MACRO_ENABLED --body true - if: needs.detect-changes.outputs.any == 'true' && vars.CODSPEED_MACRO_ENABLED == 'true' - # Advisory instrument: real wall-clock numbers (V8 JIT active, real - # cache/branch behavior) that complement the simulation gate (bench.yml) — - # simulation catches small algorithmic slips deterministically, - # walltime keeps the numbers honest on real hardware and covers the - # pure-JS packages where the no-JIT simulation model is furthest from - # production. Failures here must not block the PR while this beds in. - continue-on-error: true - timeout-minutes: 30 - permissions: - contents: read - pull-requests: write # CodSpeed action posts a sticky PR comment - id-token: write # OIDC token used by CodSpeedHQ/action for auth - # CodSpeed-managed 16-core ARM64 bare-metal machine, tuned for - # low-noise walltime measurement. Requires the CodSpeed GitHub app on - # an organization account. NOTE: ARM64 — anything cached must be - # arch-qualified (see the cache key below). - runs-on: codspeed-macro - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - # Pinned for the same reason as the simulation job above: V8 changes - # between patch releases move the numbers, and walltime is if anything - # more sensitive than instruction counts. Keep this in step with the - # codspeed-bench pin so the two instruments stay comparable. - node-version: '24.20.0' - - name: Provide pnpm via Corepack - run: | - corepack enable pnpm - corepack prepare --activate - pnpm --version - - name: Download all built dists - uses: actions/download-artifact@v4 - with: - pattern: dist-* - path: tmp/ - - name: Replay dists into packages//dist - run: | - set -e - for d in tmp/dist-*; do - [ -d "$d" ] || continue - pkg=$(basename "$d" | sed 's/^dist-//') - mkdir -p "packages/$pkg/dist" - shopt -s dotglob nullglob - cp -r "$d"/* "packages/$pkg/dist/" 2>/dev/null || true - done - - name: Restore node_modules cache - id: modules-cache - uses: actions/cache@v4 - with: - path: | - node_modules - packages/*/node_modules - # runner.arch matters: this job runs on ARM64 while every other - # job is x64; sharing a key would restore x64 native binaries - # (esbuild/rollup) and break vitest. - # Manifests + workspace config in the key — see the build job's cache - # step for why the lockfile alone is not enough. - key: pnpm-modules-node24-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('package.json', 'packages/*/package.json', 'pnpm-lock.yaml', 'pnpm-workspace.yaml') }} - - name: Install dependencies - if: steps.modules-cache.outputs.cache-hit != 'true' - run: pnpm install --frozen-lockfile - - name: Log CPU info - run: lscpu | grep -E "Model name|Cache|Flags" | head -5 || true - - name: Compute bench scope - id: scope - env: - BENCH: ${{ needs.detect-changes.outputs.bench }} - run: | - set -euo pipefail - flags="" - for pkg in $(echo "$BENCH" | jq -r '.[]'); do - # Untrusted: fork PRs control this value (it comes from the PR's own - # packages//package.json). Validate against npm's name grammar - # before it reaches GITHUB_OUTPUT or any command line. Must be a - # whole-string check: keep it in node rather than a line-based tool. - # Keep in step with the same check in bench.yml. - name=$(node -e ' - const pkg = process.argv[1]; - const { name } = require(`./packages/${pkg}/package.json`); - if (typeof name !== "string" || - !/^(@[a-z0-9][a-z0-9._-]*\/)?[a-z0-9][a-z0-9._-]*$/.test(name)) { - console.error(`::error::Rejected package name for ${pkg}`); - process.exit(1); - } - process.stdout.write(name); - ' "$pkg") - flags="$flags --filter $name" - done - echo "Bench scope flags:$flags" - echo "flags=$flags" >> "$GITHUB_OUTPUT" - - name: Run CodSpeed benchmarks (walltime) - # Walltime measures actual elapsed time, so parallel benchmark - # processes would contend for cores and add noise — run packages - # sequentially (--workspace-concurrency=1). The simulation job in - # bench.yml now does the same: it was left parallel on the premise that - # instruction counting is immune to contention, and #76 showed it is - # not. See the comment on that job's run step. - uses: CodSpeedHQ/action@4e969336ab9acd4f6f8d025fdd793292b0835df0 # v4.18.2 - env: - # Keep this in env, NOT `${{ }}` in the run: below — an env value is - # expanded by the shell after the command line is parsed, so it stays - # data. Unquoted below on purpose: the flags must word-split into - # repeated `--filter ` pairs. - SCOPE_FLAGS: ${{ steps.scope.outputs.flags }} - with: - mode: walltime - run: pnpm --workspace-concurrency=1 $SCOPE_FLAGS run bench + # codspeed-walltime lived here until it was moved to bench.yml, so that it + # runs AFTER the simulation bench rather than beside it. See that job's + # comment for why the ordering decides which instrument GitHub reports. diff --git a/BENCHMARKING.md b/BENCHMARKING.md index 6753dd6f..e418ae2d 100644 --- a/BENCHMARKING.md +++ b/BENCHMARKING.md @@ -5,9 +5,9 @@ what the numbers mean, why they don't match real wall-clock time, and how to read warnings from the CodSpeed dashboard. Bench files live under `packages/*/bench/*.bench.js` and are driven by -`vitest bench` + `@codspeed/vitest-plugin@^5`. The full pipeline is in -`.github/workflows/pr-checks.yml` (jobs: `codspeed-bench` and -`codspeed-walltime`). +`vitest bench` + `@codspeed/vitest-plugin@^5`. Both bench jobs are in +`.github/workflows/bench.yml` (`codspeed-bench`, then `codspeed-walltime`); +`.github/workflows/pr-checks.yml` builds the dists they measure. ## TL;DR @@ -48,11 +48,29 @@ bare-metal machines. It covers simulation's two blind spots: real-time effects (branch prediction, actual caches) that instruction counting models away, and the pure-JS packages (`little-endian`/`big-endian`) where the no-JIT simulation model is furthest from production V8. -Walltime benches run packages sequentially (`--concurrency 1`) because -parallel processes contend for cores and add noise; simulation is immune -to contention so it keeps `--parallel`. Note the macro runners are ARM64: -walltime numbers are real milliseconds, but on different silicon than -most x86 production traffic. +Note the macro runners are ARM64: walltime numbers are real milliseconds, +but on different silicon than most x86 production traffic. + +**Both jobs run packages sequentially** (`--workspace-concurrency=1`). +Walltime measures elapsed time, so parallel processes contend for cores +and add noise. Simulation was left parallel on the premise that +instruction counting is immune to contention, and #76 disproved that: a +charls bench was reported as a 19.8 ms → 37.9 ms regression on a commit +whose only change was one vitest file. See the comment on that job's +`run:` step in `bench.yml`. + +**Ordering matters, and it is why both jobs live in `bench.yml`.** The +CodSpeed app computes its single `CodSpeed Performance Analysis` check +from the **first** upload a commit produces, and never re-evaluates it. +While walltime ran beside the bench in `pr-checks.yml` it finished 2–7 +minutes earlier every time, so the advisory instrument decided the check +on every commit and the gate never spoke. `continue-on-error: true` does +not help: it sets the job's conclusion, while the app posts an +independent check run that no job setting can mark advisory. +`codspeed-walltime` therefore `needs: codspeed-bench`, which also stops +the two instruments landing in one series — they differ by 5–15x on wasm +decode, so a commit measured by one and its predecessor by the other +produces a meaningless delta (see the next section). ## How the numbers get inflated @@ -168,6 +186,25 @@ its own — see the comment on `concurrency:` in [.github/workflows/bench.yml](.github/workflows/bench.yml). Merges of #70 and #73 produced no baseline at all. +There was a second cause, fixed at the same time as this note was written. +`pr-checks.yml` kept a branch-level push concurrency group after `bench.yml` +got a per-commit one, so the release commit still cancelled the merge +commit's **`pr-checks`** run — which is where `codspeed-walltime` used to +live. On `c9ffa62` that killed walltime with every other job already green, +simulation won the first-upload race by default, and the check compared +`bac71dd`'s walltime number against `c9ffa62`'s simulation number: +`JPEG XL Lossless (.110)` 158.5 ms → 991.8 ms. That 6.3x ratio is not a +regression, it is the 5–15x simulation-inflation band in the table above. +Two symptoms identify this case: the ratio sits near that band, and the +commit's `codspeed-walltime` job is `cancelled` rather than `success`. + +Drift also plays a part that a fresh baseline does not remove. `JPEG XL +Lossless colour (.110)` read 182.1 ms, then 193.4 ms, then 298.5 ms across +three commits whose `libjxl` was byte-identical, with a good walltime +baseline on both sides of each comparison. That is well outside the 1–3% +figure in the table, so treat walltime deltas on the `dicom-codec` dispatch +benches as advisory even when the baseline is sound. + **What to do**: confirm the diff cannot affect the measured path, then let the next successful **push to main** re-seed the baseline for all live benchmarks — that happens on its own and needs no dashboard access.