Add CI performance benchmark suite#223
Conversation
Performance results need a stable workload so source-data changes do not look like runtime regressions. Add a small committed project split across core, filesystem, and network modules for repeatable Git, ripgrep, zstd, and Make measurements. Keep the corpus separate because its contents define the benchmark and should change only when baselines are deliberately regenerated.
c606264 to
899d081
Compare
Functional tests do not reveal changes in syscall, process-startup, or application-level performance. Add a two-tier suite so pull requests produce repeatable measurements and expose regressions before merge. Run pinned lmbench microbenchmarks and representative developer workloads with warmups and median reporting. Store raw samples in JSON, compare them with captured elfuse, QEMU Linux, and OrbStack baselines, and treat missing baseline metrics as failures. Run the suite exclusively on the self-hosted Apple Silicon machine to avoid interference from sibling jobs. Keep comparison report-only until runner variance is characterized, while retaining artifacts for review and baseline refreshes. Closes sysprog21#195
899d081 to
3a0517a
Compare
| # Holder: acquire (blocking), report through the pipe, then keep | ||
| # the descriptor open until release kills us or we self-expire. | ||
| os.close(r) | ||
| os.setsid() |
There was a problem hiding this comment.
The detached lock holder calls setsid() and closes the pipe but never redirects fds 1/2, so it keeps the CI step's stdout/stderr open. GitHub Actions never sees pipe EOF on the "Acquire host lock" step and it hangs until the holder self-expires (CI_HOST_LOCK_MAX_HOLD_MIN, up to 120 min); "Release host lock" cannot run until acquire returns, so the job deadlocks.
Fix: after os.setsid(), redirect fds 0/1/2 to /dev/null (os.open + os.dup2 over 0,1,2) before writing readiness and sleeping.
| test_timeout: '' | ||
| job_timeout: 20 | ||
| job_timeout: 35 | ||
| lock_wait: 10 |
There was a problem hiding this comment.
The host lock is acquired at checkout and released only at job end, so every leg holds its lock for its entire runtime. The exclusive benchmark leg then blocks shared siblings far longer than this lock_wait=10 budget (its real work is ~15-25 min, up to a 60-min job), so a sibling that queues behind it exhausts 10 min of wait and fails spuriously.
Fix: acquire the exclusive lock immediately around make bench-ci only (held just for the measurement window), or raise every sibling's lock_wait above the benchmark leg's worst-case hold time.
|
|
||
| env = results.get("meta", {}).get("env", "elfuse-aarch64") | ||
| column = (baseline.get("environments") or {}).get(env) | ||
| if not column: |
There was a problem hiding this comment.
A missing, empty, or misspelled meta.env resolves to no baseline column and returns 0 before any regression/missing check, silently disabling all gating including --fail-on-missing. Also, .get("env", "elfuse-aarch64") only defaults on an absent key, not a present empty string.
Fix: treat a missing/empty column as a hard error (non-zero exit) when --fail-on-missing is set, and reject an empty meta.env explicitly.
| ;; | ||
| release) | ||
| if [ -s "$PID_FILE" ]; then | ||
| kill "$(cat "$PID_FILE")" 2> /dev/null || true |
There was a problem hiding this comment.
Release runs kill on the recorded PID with no check that it still names the lock holder. After the holder self-expires (CI_HOST_LOCK_MAX_HOLD_MIN) or crashes and the OS recycles its PID, this signals an unrelated process.
Fix: verify holder identity before killing (pgid/start-time), or kill the setsid process group and confirm it still owns the flock.
| """ | ||
| flat: dict[str, float] = {} | ||
|
|
||
| def walk(prefix: str, node) -> None: |
There was a problem hiding this comment.
float(node["median"]) accepts NaN/Infinity from Python's permissive JSON parser. A NaN median makes every ratio comparison false (nan > 1.0+threshold and nan < 1.0-threshold are both False), so a corrupt result passes as green.
Fix: validate each metric with math.isfinite() and reject non-finite (and non-positive baseline) values.
| try: | ||
| os.fchmod(fd, stat.S_IMODE(args.baseline.stat().st_mode)) | ||
| with os.fdopen(fd, "w", encoding="utf-8") as tmp: | ||
| tmp.write(json.dumps(baseline, indent=2) + "\n") |
There was a problem hiding this comment.
json.dumps(baseline, indent=2) uses sort_keys=False, so newly promoted metrics append in results iteration order and churn the hand-authored alphabetized baseline, producing noisy order-dependent diffs.
Fix: pass sort_keys=True.
| # drop BENCH_REPORT_ONLY=1 so regressions hard-fail. | ||
| if: ${{ matrix.run_bench }} | ||
| run: | | ||
| BENCH_REPORT_ONLY=1 python3 scripts/bench-compare.py --fail-on-missing |
There was a problem hiding this comment.
--fail-on-missing combined with BENCH_REPORT_ONLY=1 hard-fails on any missing metric, so a single transient lmbench ERR/hang (exactly what the retry+ERR machinery exists for) reds this leg despite the "soft signal for now" intent. This is deliberate per the PR description, but worth reconsidering during the ~2-week ramp: consider dropping --fail-on-missing until the flip that removes BENCH_REPORT_ONLY.
| run_tier2_tool python_startup_ms "" python3 -c pass | ||
| run_tier2_tool git_status_ms "" git -c 'safe.directory=*' \ | ||
| --no-optional-locks -C "${GUEST_WORK}/corpus" status --porcelain | ||
| run_tier2_tool ripgrep_ms "" rg --no-config -c syscall_entry_ \ |
There was a problem hiding this comment.
rg -c syscall_entry_ exits 1 when it finds zero matches, and the timeit wrapper treats any non-zero child status as fatal, so a future corpus edit that drops the literal would abort the whole benchmark run instead of recording a metric.
Fix: use a pattern guaranteed to match, or tolerate rg's exit-1 no-match case explicitly.
| os.close(r) | ||
| os.setsid() | ||
| fd = os.open(lock_path, os.O_RDWR | os.O_CREAT, 0o666) | ||
| fcntl.flock(fd, flags) |
There was a problem hiding this comment.
flock(2) provides no writer priority, so under overlapping workflow runs a steady stream of shared (normal-leg) acquirers can starve the benchmark leg's exclusive request until its wait budget expires.
Fix: gate new shared acquisitions behind a writer-waiting flag once an exclusive request is pending.
| # Run one timed sample: prints elapsed microseconds on stdout. | ||
| # Any non-zero exit (including timeout's 124) is fatal -- a failing | ||
| # sample must never be smoothed into a fast measurement. | ||
| bench_sample_us() |
There was a problem hiding this comment.
bench_sample_us() is defined but never called (Tier-2 timing goes through timeit_sample_us), leaving dead code that can rot out of sync with the real sampler.
Fix: delete bench_sample_us or wire it in where a host-side timed sample is actually needed.
Summary
Add a reproducible two-tier performance benchmark suite for detecting
regressions in pull requests.
filesystem latency.
against a deterministic committed corpus.
and OrbStack.
CI integration
Add a benchmark leg to the self-hosted Apple Silicon runtime matrix.
The leg runs exclusively on the host to avoid interference from sibling
jobs, compares results in report-only mode, and uploads the JSON artifact.
Missing baseline metrics are treated as failures, preventing broken or
timed-out benchmarks from silently passing. Benchmark preparation and
cleanup commands are also bounded by timeouts.
The checked-in baseline has not yet been captured on the designated
self-hosted CI machine. It will be refreshed in a follow-up after the
benchmark leg is deployed and runner variance has been characterized.
Comparison therefore remains report-only for now.
Validation
Closes #195
Summary by cubic
Adds a two-tier performance benchmark suite to catch syscall, process-startup, and app-level regressions before merge. Runs pinned
lmbenchand representative workloads, compares medians to baselines, and uploads JSON artifacts from a self-hosted Apple Silicon runner with a machine lock. (Issue #195)New Features
tests/bench-suite.sh: Tier 1lmbench(lat_syscall/lat_proc/lat_fs) and Tier 2 (git status,rg,make,zstd) with warmups; median+samples tobuild/bench-results*.json. Guest-side timing for QEMU viatests/bench-timeit.c.tests/bench-corpusfor stable Tier 2 inputs; excluded from format checks to preserve baselines. Fixtures fetch and cross-compile a pinnedlmbenchbuild.tests/bench-baseline.jsonforelfuse-aarch64,qemu-aarch64,orbstack;scripts/bench-compare.pyflags regressions with a tunable threshold and reports missing metrics as NEW (not gating). CI is report-only and uploads artifacts.make benchandmake bench-ci;BENCH_ENVselectselfuse-aarch64(default) |qemu-aarch64|orbstack|native. CI adds a Benchmark leg on the self-hosted Apple Silicon runner with exclusive host locking viascripts/ci-host-lock.sh; docs indocs/testing.md.Migration
make bench(orBENCH_ENV=qemu-aarch64 make bench-ci).scripts/bench-compare.py --report-only(tune viaBENCH_REGRESSION_THRESHOLD).make bench-ciper env, thenscripts/bench-promote.py; commit the updatedtests/bench-baseline.json.Written for commit 3a0517a. Summary will update on new commits.