Skip to content

bench-regress compares across runners — baseline and PR run on different machines #370

Description

@chrishayuk

The bench gate records a Criterion baseline on a main push, caches it, and
restores it onto a different macos-14 runner for the PR comparison. It is
therefore comparing absolute timings across two physical machines at different
times.

Why that is not a measurement, by this repo's own standard

crates/larql-vindex/src/format/vindex3/opplan/exec/tests/q2a_decode_bench.rs
states the discipline explicitly and follows it:

Both arms in one process, interleaved in blocks, the order alternating block
to block — the BF16 figure is re-earned beside the candidate, never compared
against a number from another session or day (machine state shifts move e2e
timings by ±6%).

The CI gate does the opposite of all three: different process, different
machine, single shot, no interleaving, no min-of-N.

Observed consequence

On 2026-09-01 it twice reported a regression in ridge_decomposition_solve
(crates/larql-compute) — including +27.8% … +34.4%, p = 0.00 — on branches
that touched only crates/larql-vindex. The changed code could not be reached
from the benchmarked crate. Two independent samples, so not a one-off.

Interim mitigation (landed)

A paths: filter now restricts the PR trigger to larql-compute,
larql-compute-metal, the bench script, the workflow and Cargo.lock — the
surface BENCHES in scripts/bench-regress.sh actually covers. A PR that
cannot reach those crates no longer produces a red. workflow_dispatch remains
so a reviewer can request the check on a skipped PR.

That removes the false attribution but does not fix the cross-runner
comparison: a PR that does touch those crates still gets a cross-machine
number.

Proper fix

Benchmark main and the PR head back to back on the same runner in one job:
checkout origin/main, make bench-save, checkout the PR head, then check.
Roughly doubles bench wall-clock (current timeout is 90 min), and removes the
machine as a variable. Ideally also min-of-N per arm rather than a single
Criterion sample, matching q2a_decode_bench.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions