[xalestar]

Supersedes #223, based on the benchmark suite @Max042004 wrote there.

make bench runs tests/bench-suite.sh in three tiers and writes build/bench-results.json:

• Tier 1: lmbench lat_syscall, lat_proc and lat_fs, built from a pinned source snapshot by tests/fetch-fixtures.sh.
• Tier 2: application workloads over the Alpine fixture tools (startup, python startup, git status, ripgrep, zstd, make over tests/bench-corpus).
• Tier 3, new here: the python, node, C compile and JVM rows of elfuse real-workload syscall frequency ranking (Top 50) #224, each run in an OCI image pinned by digest through scripts/elfuse-oci.py. Image pulls happen before the timed samples. BENCH_OCI=0 skips the tier. The go row is left out: gofmt there can need a 65th thread, and elfuse gives a guest process 64 (Guest processes are limited to 64 threads #494).

The Tier 2 tools land in the shared fixture rootfs, so every qemu boot carries them: the initramfs grows from 35.4 MB to 51.8 MB gzip (61 MB to 107 MB unpacked, out of the 2048 MB guest), and qemu boot to ssh stays at 1.1 to 1.2 s over 3 boots each.

The Build workflow gains a Benchmark leg that runs make bench-ci and compares against tests/bench-baseline.json in report-only mode; bench-report.yml posts the result as a PR comment. scripts/ci/host-lock.sh keeps the other legs off the host while the Benchmark leg measures: they hold it shared for their own run, and the Benchmark leg holds it exclusive only around make bench-ci.

Changes from the last revision of #223, for the open review threads there:

• The lock holder binds a control socket unique to each acquire and records its path in the pid file. Release reads that path, cleans up when the holder is gone, and keeps the lock state and exits 1 when a live holder does not answer, so a timeout no longer orphans a holder.
• qemu and orbstack corpus prep run under their own BENCH_PREP_TIMEOUT (default 900 s) instead of the per-sample bound.
• The result emitter iterates rows by metric and rejects a duplicate metric.
• bench-timeit rejects an elapsed time under 1 us instead of printing 0.
• The commentflow, format and pre-commit checks exclude tests/bench-corpus, and the corpus files have descriptive names.
• The workflow keeps the existing comments on unrelated steps, and docs/testing.md only adds a benchmark section.
• The fixture cache is written to a temporary copy and renamed into place.
• When a later run is back inside the threshold, the earlier report comment is updated to say so.
• An empty BENCH_LMBENCH_ENOUGH restores lmbench auto-calibration.
• A failed lmbench download or build makes fetch-fixtures.sh skip lmbench instead of failing the whole fetch.

Publishing results to GitHub Pages is left for a follow-up PR.

Tested on current main (170b5df): make check passes, and make bench-ci passes with all three tiers producing every metric, in about 12.5 minutes. The four Tier 3 metrics have no baseline yet; a baseline from the CI host should be promoted with scripts/bench-promote.py after the first runs there.

Closes #195

[cubic-dev-ai]

4 issues found across 74 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/bench-oci/jvm.sh">

<violation number="1" location="tests/bench-oci/jvm.sh:13">
P1: Create the compiler output directory before invoking `javac`; otherwise every JVM benchmark sample fails with a missing destination-directory error.</violation>
</file>

<file name="tests/bench-corpus/Makefile">

<violation number="1" location="tests/bench-corpus/Makefile:5">
P3: The default `OUT` violates the Makefile’s own requirement and writes generated artifacts into the fixed corpus for ordinary `make` invocations. Use an external temporary default or require callers to provide `OUT`.</violation>
</file>

<file name="scripts/bench-promote.py">

<violation number="1" location="scripts/bench-promote.py:211">
P2: On a partial capture, the new medians are merged but `captured.<env>` keeps the previous run's date and host. The values no longer match the provenance recorded for them. Either update `captured` whenever any values change, or record the partial capture's provenance separately.</violation>
</file>

<file name=".github/workflows/bench-report.yml">

<violation number="1" location=".github/workflows/bench-report.yml:78">
P3: When a later run produces no report, this step is skipped and an earlier regression comment stays up. The file header says a later commit must rewrite that comment, so a missing-report run should also clear or mark the old comment as stale.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Create the compiler output directory before invoking javac; otherwise every JVM benchmark sample fails with a missing destination-directory error.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/bench-oci/jvm.sh, line 13:

<comment>Create the compiler output directory before invoking `javac`; otherwise every JVM benchmark sample fails with a missing destination-directory error.</comment>

<file context>
@@ -0,0 +1,15 @@
+src="$1"
+work="$(mktemp -d /tmp/bench-jvm.XXXXXX)"
+cp "$src"/*.java "$work/"
+javac -d "$work/classes" "$work"/*.java
+java -cp "$work/classes" Bench > /dev/null
+rm -rf "$work"
</file context>

Suggested change

	javac -d "$work/classes" "$work"/*.java
	mkdir -p "$work/classes"
	javac -d "$work/classes" "$work"/*.java

[xalestar]

javac creates the -d directory itself when it is missing. The jvm workload ran to exit 0 under elfuse with this script in eclipse-temurin:21 (all 12 samples in the local make bench-ci run, and one direct run after this review), so there is no failure to fix.

[jserv]

The renderer is given a hardcoded --threshold 0.20, and the comment text above says ±20%, while bench-compare.py honors BENCH_REGRESSION_THRESHOLD and records the value it used as threshold in the artifact. Change the env var and the console log and the PR comment disagree about what counts as a regression. Have scripts/bench-report.py read threshold from the validated report (bounded to a sane range) instead of taking it from the workflow, and drop the flag here.

[xalestar]

Done in 0073af6. bench-report.py reads threshold from the validated report, limited to 0 through 1 because the comment shows it as ±N%, and the --threshold flag is gone. It also writes the all-inside text itself, so the workflow no longer holds a percentage: the script prints clean or changes, and the comment step only updates an existing comment on clean.

[jserv]

comparison_metrics only holds metrics present in both the baseline and the results, so a metric whose measurement failed never reaches the artifact. With --fail-on-missing the step fails, but the artifact is still uploaded and bench-report.yml still runs: if the remaining metrics are inside the threshold, the render is empty and the existing regression comment is overwritten with "No benchmark metric is outside ±20%". Put the missing metrics in the report (a status per entry) and have scripts/bench-report.py render them as failures, or skip the comment update when the comparison was incomplete.

[xalestar]

Done in 0073af6. bench-compare.py now writes each missing metric to the report with status: missing (compared ones carry status: compared), and bench-report.py lists them under "Missing measurements" and reports changes. A run with a failed measurement therefore never renders the all-inside text. Checked by dropping lat_fs.create from a results file: the comment lists it as missing, and the status is changes. b68579c extends this to a missing metric whose baseline is zero.

[cubic-dev-ai]

All reported issues were addressed across 15 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[jserv]

Once this leg publishes its writer marker, host-lock.sh makes every new shared acquirer wait until the exclusive lock is granted, and that wait can run the full 60 minutes behind a TSAN leg. Legs from another run (and oci-smoke) that start in that window budget only 15 minutes, so they fail with a lock timeout that reads like a CI failure. Either make the shared wait cover exclusive_lock_wait plus the measurement window (raising job_timeout to match), or cap this wait at what the readers can absorb.

[xalestar]

Done in 45a63f0, the first option. The bench step now has timeout-minutes: 25 (it took 15 on the last run), exclusive_lock_wait is 45, the runtime budget of the longest sibling (TSAN), and every shared_lock_wait, oci-smoke included, is 75 = 45 + 25 + 5. Each job_timeout grows by the same 60 minutes, so Benchmark goes to 160. The comment above timeout-minutes states how the numbers relate.

[jserv]

The threshold and every baseline value come from the artifact, which the PR run built from its own tests/bench-baseline.json. A PR that refreshes the baseline with bench-promote.py is compared against itself, yet the comment still says "checked-in baseline". This job already checks out the default branch: load tests/bench-baseline.json and the threshold from that checkout, take only current from the artifact, and treat a trusted baseline metric absent from the artifact as missing.

[xalestar]

Done in ba4892d. The artifact (schema 2) now carries only the environment and the measured current values. bench-report.py loads bench-compare.py from its own checkout, the default branch, for flatten and DEFAULT_THRESHOLD, and compares against that checkout's tests/bench-baseline.json. A trusted baseline metric that the artifact lacks is listed under Missing measurements. The comment text now says "the default branch's baseline". The threshold therefore no longer comes from the artifact either, which replaces what 0073af6 did.

[jserv]

If the job is cancelled while this call blocks (a new push does that through cancel-in-progress), the runner kills the acquire parent but not this setsid holder. It stays in flock(LOCK_EX) with no deadline and keeps its writer marker locked, so every new shared acquirer stalls behind it, and the parent's wait_seconds cleanup never runs. Record the parent pid before the fork and poll here with LOCK_EX | LOCK_NB, giving up once os.getppid() no longer matches. The shared loop below needs the same check.

[xalestar]

Done in 922bd60. The holder records the parent pid before the fork and polls with LOCK_EX | LOCK_NB. The shared loop polls with LOCK_SH | LOCK_NB. Once os.getppid() differs, both raise SystemExit, so the finally block removes the writer marker and the socket. Tested locally with a shared holder, an exclusive waiter and a shared waiter queued behind its marker: after kill -9 of both acquire parents, both holders were gone and no marker was left. The previous script kept the exclusive holder and its marker.

[jserv]

value is accepted whatever rc is, so an attempt that printed its result line and then exited non-zero (a guest crash on exit, or timeout firing after the print) counts as a good sample in the median. lmbench_lat_fs has the same gap.

Suggested change

	if [ -n "$value" ]; then
	if [ "$rc" -eq 0 ] && [ -n "$value" ]; then

[xalestar]

Done in 041aa46, in lmbench_metric and lmbench_lat_fs: the parsed value is cleared when rc is non-zero, so that attempt is retried and then recorded as ERR.

[jserv]

This overwrites any existing /tmp/hello-s and removes it only on the success path. A bench_die later in Tier 1, or an interrupt, leaves both it and $fs_host behind because cleanup knows about neither. Keep both in globals that cleanup removes, and fail when /tmp/hello-s already exists instead of clobbering it.

[xalestar]

Done in a4bf814. T1_FS_HOST and T1_HELLO are globals that cleanup removes through tier1_unstage, which the success path calls too. /tmp/hello-s is created with set -C, so the suite fails when the file already exists, in the OrbStack machine as well. Tested: with an existing /tmp/hello-s, the suite stops with that message, the file is left as it was, and no elfuse-bench-fs directory stays behind.

[jserv]

One failed sample in Tier 3 (or in Tier 2 via timeit_sample_us) exits the suite before emit_json, so a single JVM flake discards every Tier 1 and Tier 2 result. Compare against baseline then never runs, no bench-pr-report artifact is uploaded, and an earlier regression comment on the PR stays as it was. Record the metric as ERR the way lmbench_metric does and continue, so bench-compare.py --fail-on-missing reports it as missing and the leg still fails.

[xalestar]

Done in 135d888. timeit_sample_us returns 1 instead of exiting. run_tier2_tool, run_startup_metric and Tier 3 then record the metric as ERR and move on, and emit_json writes {"status": "ERR"} for it. Tested by making rg and the node workload fail: the results file was written with ripgrep_ms and node_ms as ERR, and bench-compare.py --fail-on-missing exited 1 listing ripgrep_ms (node_ms has no baseline yet).

[jserv]

These packages land in the shared fixture rootfs, and initramfs.cpio.gz is built from that tree. So python3, git, ripgrep and their libraries now ship in every test-matrix qemu boot, not only in the Benchmark leg, and the description does not mention that cost. Please state the initramfs size and boot-time change, or stage the bench tools as an overlay that only the bench path adds.

[xalestar]

Measured, and now stated in the description. The initramfs grows from 35.4 MB to 51.8 MB gzip, and the unpacked rootfs from 61 MB to 107 MB of the 2048 MB guest. qemu boot to ssh stays at 1.1 to 1.2 s with and without the tools, over 3 boots each (the runner polls once a second). The qemu bench column runs these tools inside the VM, so an overlay would save only that memory. I can split it out if you want the boot image kept minimal.

[jserv]

Rebase the current branch onto the upstream default branch and rework the series into functionally minimal commits, folding similar ones and enforcing the project's commit message rules.

[jserv]

timeout-minutes: 25 bounds the whole step, including acquire exclusive, which is allowed to wait 45 minutes (exclusive_lock_wait). When a TSAN leg keeps its shared lock for longer than 25 minutes minus the bench runtime, the step is killed while still queueing, so the exclusive wait the job budget is built around is never available. Move release and acquire exclusive into their own step with timeout-minutes: 50, keep this one bounding only make bench-ci, and add a release step with if: always() right after it so Compare and the uploads do not run under the exclusive lock.

[xalestar]

Split in 6544180. Acquire exclusive host lock is now its own step with timeout-minutes: 50, the bench step bounds only make bench-ci at 25, and Release exclusive host lock runs with always() right after it, so Compare and the uploads run without the exclusive lock.

[jserv]

flatten drops every {"status": "ERR"} node, so a failed metric surfaces only through missing, which is computed from the baseline. The four Tier 3 metrics have no baseline yet, so an OCI row that errors or times out leaves --fail-on-missing passing and the PR comment clean, which is not what the "Compare against baseline" step comment promises. Collect the ERR paths during the walk and count them as missing in main, independent of the baseline.

[xalestar]

Fixed. flatten now collects the path of each {"status": "ERR"} leaf, and bench-compare.py counts each one as missing whether or not it has a baseline (1b32aba). The report artifact carries them as failed, and bench-report.py lists them under Missing measurements with - as the baseline (b255cdd). With oci.python_ms ERR and no baseline, --fail-on-missing now exits 1 where it exited 0 before, and the self-test covers the comment.

[jserv]

The holder already owns the lock when the state file is written. If the job is cancelled in that window or the write fails (full $RUNNER_TEMP), release finds no state, prints "nothing to release" and exits 0, and the orphaned holder keeps the lock for CI_HOST_LOCK_MAX_HOLD_MIN (120 minutes), timing out every Benchmark leg's exclusive wait meanwhile. child and control_path are both known right after fork, so write the state file before the select and remove it on the not-acquired path: the holder already listens before it locks, and it exits by itself when the parent dies while waiting.

[xalestar]

Fixed in 6544180. The state file is written right after fork, before the select, and removed on the timeout and holder-failure paths. Checked locally: the file exists during the wait, a kill -9 of the acquire makes the holder exit and release clean up, and a timed-out acquire leaves no state file.

[jserv]

$CORPUS goes into the orb run sh -c line unquoted, so a checkout path containing a space or a shell metacharacter splits or expands inside the machine and prep dies with "orbstack corpus prep failed". sh_quote already exists for this.

Suggested change

	prep="$prep && cp -R $CORPUS $ORB_WORK/corpus"
	prep="$prep && cp -R $(sh_quote "$CORPUS") $ORB_WORK/corpus"

[xalestar]

Applied in 1b32aba. Not run under OrbStack here.

[jserv]

Every writer of $SAMPLES passes the metric name twice (startup_ms startup_ms, and record_err writes "$2" "$2"), so bench always equals metric, and the duplicate-metric check further down can never fire because each metric sits under its own bench key. Drop the bench column from the row format and key data by section and metric; repeated rows for one metric are then just its samples, and the dead check goes away.

[xalestar]

Fixed in 1b32aba. Rows are now <section> <metric> <value>, the emitter keys the samples by section and metric, and the duplicate check is gone.

[jserv]

The workload is forked before these handlers exist. A cancellation or timeout signal landing between fork and here kills the wrapper with the default action, while the child has already moved to its own process group and survives into the next sample, which is what the forwarder is meant to prevent. Block SIGTERM/SIGINT/SIGHUP/SIGQUIT with sigprocmask before fork, restore the old mask in the child before execvp, and unblock in the parent once the forwarders are installed.

[xalestar]

Fixed in 1b32aba. The four signals are blocked before fork, the child restores the old mask before execvp, and the parent unblocks once the forwarders are installed; a signal held across fork makes the wrapper return 128 + signo. With SIGTERM sent 0 to 4 ms after start, 400 runs each, the old binary left 41 workloads running and the new one none.

[xalestar]

Rebased onto main (170b5df) and folded the 30 commits into four: bceac52 (the fixture cache swap, a race that exists without the benchmark too), 1b32aba (the suite), 6544180 (the CI leg and the host lock), b255cdd (the PR comment reporter). Apart from one reflowed docstring, the tree is the same as before the fold.

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[xalestar]

After the fold, the commit ids in my earlier replies no longer exist on the branch. Where each change lives now:

Earlier ids	Now in
0a36707, a08b002, bceac52	c8d951d Serialize the fixture cache swap
04d079d, bb379f7, 041aa46, a4bf814, 135d888, 1f688e6, 268203e, 1b32aba, a795a9a	31ff5bd Add a three-tier benchmark suite
077d205, 922bd60, 45a63f0, 6544180, f63a51b	0bf29ae Run the benchmark suite in CI
0073af6, b68579c, ba4892d, bbf7923, 7af0e7d, c7fb27a, b255cdd	c670b6d Comment benchmark changes on pull requests
b51bb1e, 6c8bfd2, f144a3c	31ff5bd (bench-compare.py and docs) and c670b6d (bench-report.py and the report write)

[jserv]

When make bench-ci fails or the exclusive acquire times out, Compare against baseline is skipped, no bench-pr-report artifact is uploaded, and this job returns early. That leaves the previous commit's comment up, possibly listing regressions or a clean result the current head never produced. When no artifact is found and workflow_run.conclusion is not success, rewrite the existing marked comment to say the benchmark did not complete at head_sha, with a link to the run.

[jserv]

The exclusive holder outlives a crashed runner for the default CI_HOST_LOCK_MAX_HOLD_MIN of 120 minutes, while every shared acquirer gives up after 75. One runner dying during the measurement therefore fails every leg on the host for two hours. The measurement step is capped at 25 minutes, so cap the hold near that.

Suggested change

	CI_HOST_LOCK_WAIT_MIN=${{ matrix.exclusive_lock_wait }} \
	scripts/ci/host-lock.sh acquire exclusive
	CI_HOST_LOCK_WAIT_MIN=${{ matrix.exclusive_lock_wait }} \
	CI_HOST_LOCK_MAX_HOLD_MIN=30 \
	scripts/ci/host-lock.sh acquire exclusive

[jserv]

A median of 0 is promoted into the baseline, but bench-compare.py and bench-report.py both reject a baseline <= 0, so the next comparison reports that metric as unusable instead of comparing it. Apply the same rule here.

Suggested change

	if not math.isfinite(float(value)) or value < 0:
	raise BenchPromoteError(
	f"results {tag}.median must be finite and non-negative")
	if not math.isfinite(float(value)) or value <= 0:
	raise BenchPromoteError(
	f"results {tag}.median must be finite and positive")