bench: onthebench-style rig harness for same-rig baselines - #917
Merged
Conversation
Reproduce the public onthebench methodology on our own m7g.4xlarge: three disjoint pinned core groups (gateway 0-3, load 4-9, mock 10-15), default shipped config, pinned prebuilt load generator and mock upstream (sha256 verified against the engine-v1 release), fixed c=16/32/128 0-delay and c=768 10ms-TTFT grid with 25s windows and four valid fail=0 repetitions per point, rig-floor reference windows, per-window CPU and peak-RSS capture, and an on-CPU flamegraph at the c=128 saturation point. One command (bench/onthebench/bench.sh) rsyncs the working tree to the rig, provisions it idempotently, builds a native release binary, runs the full grid, and collects results with full metadata (commit, binary and instrument sha256s, core split, method parameters).
…plies rustup is installed with no default toolchain, so a cargo invocation outside the source tree has no version to run and rig provisioning failed at the inferno install. Every cargo call in setup now runs inside the synced tree, where rust-toolchain.toml pins the version; this also front-loads the toolchain download out of the build step.
…ocess aisix reads AISIX_* environment variables as config overrides, so the AISIX_COMMIT/AISIX_DIRTY metadata the driver passed over ssh reached the gateway's environment as unknown top-level config fields and the process refused to boot. The metadata now travels as BENCH_SRC_COMMIT/ BENCH_SRC_DIRTY, and the runner defensively unsets every AISIX_* variable before launching the gateway so nothing from the harness environment can reach the measured process.
…atal Non-interactive ssh shells never source the cargo env, so the inferno renderers were absent from PATH and the render step killed the whole run under set -e after the c=128 flamegraph window - before the 10ms-TTFT leg and the metadata were written. The runner now exports the cargo bin path itself, and a rendering failure downgrades to a warning that keeps perf.data for off-rig rendering instead of aborting the measurement. Sampling drops to -F 499 with the default dwarf stack size: same readability, a quarter of the perf.data size and render time.
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesBenchmark harness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant ARM64Rig
participant RigSetup
participant BaselineRunner
Developer->>ARM64Rig: Synchronize checkout
ARM64Rig->>RigSetup: Provision dependencies and instruments
ARM64Rig->>ARM64Rig: Build release binary
Developer->>BaselineRunner: Start baseline benchmark
BaselineRunner->>ARM64Rig: Run workloads and collect metrics
BaselineRunner->>ARM64Rig: Capture flamegraph and metadata
Developer->>ARM64Rig: Retrieve run results
``
</details>
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
---
<!-- pre_merge_checks_override_start -->
> [!IMPORTANT]
> ## Pre-merge checks failed
>
> Please resolve all errors before merging. Addressing warnings is optional.
<!-- pre_merge_checks_override_end -->
### ❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
| :---------------------: | :------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------- |
| E2e Test Quality Review | ⚠️ Warning | run-baseline.sh only warns when a point has fewer than 4 valid windows, and it only logs tpc_workers; invalid or wrong-topology runs can therefore be accepted. | Exit nonzero for incomplete points and unexpected tpc_workers (4); validate floor failures and prevent bench.sh from collecting incomplete results. |
| Security Check | ❓ Inconclusive | Investigation is still in progress; no verdict should be submitted yet. | Inspect the harness credential handling and applicable security categories before deciding. |
<details>
<summary>✅ Passed checks (4 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :-------------------------------------------------------------------------------------------------------------------- |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the new onthebench-style rig harness and its purpose for same-rig performance baselines. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches</summary>
<details>
<summary>📝 Generate docstrings</summary>
- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch
</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Commit unit tests in branch `perf/onthebench-rig`
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=api7/aisix&utm_content=917)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
<details>
<summary>❤️ Share</summary>
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
</details>
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
There was a problem hiding this comment.
Pull request overview
Adds a reproducible same-rig benchmark harness for performance baselines.
Changes:
- Provisions and pins benchmark tooling.
- Runs fixed load points with telemetry and flamegraph capture.
- Documents methodology and result artifacts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
bench/onthebench/bench.sh |
Orchestrates deployment, build, execution, and result collection. |
bench/onthebench/rig-setup.sh |
Provisions dependencies and pinned instruments. |
bench/onthebench/run-baseline.sh |
Executes benchmark windows and records metrics. |
bench/onthebench/README.md |
Documents usage, methodology, and outputs. |
Suppressed comments (1)
bench/onthebench/run-baseline.sh:209
- Exhausting all attempts only warns and then lets the run produce normal metadata and exit successfully with fewer than the promised four valid repetitions. Downstream comparisons can mistake this incomplete point for a completed baseline; fail the run once the retry budget is exhausted.
[ "$valid_n" -ge "$REPS" ] ||
echo "WARNING: $point got only $valid_n valid reps in $MAX_TRIES tries" >&2
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+22
to
+23
| rsync -az --delete --exclude=target --exclude=.git --exclude=docs/superpowers \ | ||
| "$SRC_LOCAL"/ "$RIG":aisix-src/ |
Comment on lines
+25
to
+28
| sudo -n DEBIAN_FRONTEND=noninteractive apt-get install -y -q \ | ||
| git curl build-essential pkg-config libssl-dev protobuf-compiler \ | ||
| linux-tools-common "linux-tools-$(uname -r)" 2>/dev/null || | ||
| sudo -n DEBIAN_FRONTEND=noninteractive apt-get install -y -q linux-tools-aws |
Comment on lines
+195
to
+196
| TPC_WORKERS=$(ps -T -p "$GW_PID" | grep -c 'tpc-' || true) | ||
| echo " pid=$GW_PID idle_rss=${RSS_IDLE}kB tpc_workers=$TPC_WORKERS" >&2 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bench/onthebench/bench.sh`:
- Line 18: Update the RUNID construction in the benchmark script to prevent
collisions between same-commit runs started within the same second. Add a
collision-resistant suffix or atomically create the remote output directory
before invoking run-baseline.sh, ensuring both remote and local result paths
remain unique.
- Line 29: Update the cargo build command in the benchmark script to include the
--locked flag while preserving the existing --release and --bin aisix options,
so the build uses the synchronized lockfile without resolving dependency
versions.
In `@bench/onthebench/README.md`:
- Around line 64-66: Update the run-output description in the README to state
that flamegraph-c128.svg is generated conditionally when Inferno rendering
succeeds, and document retained perf.data as an output when rendering fails.
- Around line 3-6: Update the public prose in the README to use the approved
product name “AISIX” wherever it currently says “aisix”; retain lowercase only
in binary names, configuration keys, and file paths, including the additional
occurrences referenced by the comment.
In `@bench/onthebench/rig-setup.sh`:
- Around line 32-34: Replace the unverified remote pipe in the Rust setup block
with a verified, versioned rustup-init download flow. In the cargo availability
check, download the platform-appropriate rustup-init binary and its official
Rustup SHA-256 checksum, validate the binary before execution, then run it with
the existing noninteractive default-toolchain-none options.
- Around line 44-46: Update the Inferno installation command in the rig setup to
pin the package to version 0.12.8, and validate the installed inferno-flamegraph
reports that version before flamegraph rendering. Preserve the existing
install-on-missing behavior while ensuring a different installed version is not
used.
In `@bench/onthebench/run-baseline.sh`:
- Line 126: Replace the CPU-count-only guard in the benchmark startup checks
with validation that the host architecture is aarch64 and its trusted identity
is m7g.4xlarge, using the launcher assertion or an IMDSv2 instance-type check.
Keep rejecting mismatches before measurement, and add a regression test covering
a nonmatching host identity.
- Line 23: Update the floor-measurement flow around FLOOR_REPS and the
floor-sample handling at lines 167-176 to match run_point’s retry policy:
collect four valid fail=0 samples, retry attempts up to MAX_TRIES, and mark
retained failed samples as invalid so they cannot become the rig reference. Add
regression coverage for failed floor samples and verify the valid-repetition
behavior.
- Around line 195-196: Validate TPC_WORKERS immediately after it is computed and
before the warmup begins, failing the baseline harness unless exactly four tpc-
workers are detected. Preserve the existing worker-count logging and use the
script’s established failure mechanism for the validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d351e9b5-425a-4d1e-a1c0-67039d8d7fbf
📒 Files selected for processing (4)
bench/onthebench/README.mdbench/onthebench/bench.shbench/onthebench/rig-setup.shbench/onthebench/run-baseline.sh
Address the audit of the initial harness (one HIGH, five MEDIUM, eight LOW findings): - Never commit a rig address: this is a public repository, so a default ssh target would publish a live passwordless-sudo box. The rig now comes from the first argument or BENCH_RIG, and usage fails loudly without one. - Split provisioning into two apt transactions so an unavailable kernel-versioned linux-tools package can no longer silently drop the build dependencies bundled with it; python3 is provisioned explicitly. - Write run metadata immediately after gateway startup and rewrite it with the memory high-water mark at the end, so an aborted run still leaves attributable results; the driver collects results even when the run fails, and warmup or floor loadgen failures no longer abort under set -e. - Assert the two failure modes the fail=0 gate cannot see: the mock must demonstrate the requested TTFT before any delayed window runs, and the gateway must show thread-per-core workers before measurement starts. - Sanitize stats lines before JSON interpolation so a quoted error from the load generator cannot corrupt results.jsonl; floor windows now carry the same validity flag as gateway windows. - Ignore and exclude the local bench-results directory so collected results stop tainting the dirty-tree metadata of later runs. - Smaller hardening: curl -f on instrument downloads, --max-time on readiness probes, a self-terminating RSS sampler, explicit fallbacks on loadgen captures, README/method alignment (sampling frequency, full method parameters in metadata) and a documented trust posture for executed third-party bytes.
Contributor
Author
|
Independent audit completed (correctness / reliability / security / leakage / breaking-changes / coverage angles). Findings and resolutions, all addressed in the follow-up commit: HIGH — committed default ssh target in a public repository. Fixed: the rig address now comes only from the first argument or MEDIUM (5) — all fixed:
LOW (8): seven fixed (curl |
- Sync the source tree through a gitignore filter so ignored files - .env files and other local material that may hold credentials - never reach the rig; tracked and untracked-but-not-ignored files still sync, preserving the measured-tree contract. The remote build cache remains protected from --delete as an excluded path. - Build with --locked so the rig cannot re-resolve dependencies away from the synchronized lockfile while metadata records the commit. - Suffix the run id with the driver pid so two same-commit runs started in the same second cannot share an output directory. - Replace the curl|sh rustup install with a version-pinned, sha256-verified rustup-init download, and pin inferno to an exact version validated on every provisioning pass - the same fixed-byte-sequence rule the bench instruments already follow. - Refuse to measure on the wrong host: assert aarch64 plus the expected instance type via IMDSv2 (recorded as unknown when IMDS is absent), and require exactly one thread-per-core worker per pinned gateway core, derived from the affinity mask rather than hardcoded. - Apply the record-mark-retry validity policy to floor windows, and exit nonzero when any point ends with fewer valid windows than promised so an incomplete run can never pass as a baseline; results and metadata are still written and collected. - README: state the flamegraph SVG is conditional with perf.data kept on rendering failure, document the floor validity policy and nonzero-exit contract, and use the product name in prose.
Contributor
Author
|
Reviewer findings resolved in d918740. Note both reviews ran against 7ffc097; four findings were already addressed by 906deb4 (the independent-audit commit) — called out below. Copilot:
CodeRabbit:
Validation for this commit: |
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Item 1 of the post-thread-per-core performance program (api7/AISIX-Cloud#1259) requires reproducing the public onthebench benchmark methodology on our own m7g.4xlarge, so that every optimization item in the program can be gated on same-rig before/after numbers instead of third-party published figures.
What
A reproducible one-command harness under
bench/onthebench/:bench.sh— rsyncs the working tree to the rig, provisions it, builds a native release binary, runs every load point, and collects results. The tree you run it from is the tree that gets measured; commit and dirty state are recorded in the run metadata.rig-setup.sh— idempotent provisioning (build deps, perf, rust toolchain via the repo'srust-toolchain.toml, inferno). The load generator and mock upstream are the prebuilt, pinned instruments from the public benchmark rig release (engine pinf3adbb1315b26129f5e317af5279decefb1cea8f, https://github.com/GetBusbar/benchmarking); their sha256s are frozen in the script, so a rebuilt rig either runs the byte-identical instrument or fails loudly.run-baseline.sh— the on-rig runner: three disjoint pinned core groups (gateway 0-3, load 4-9, mock 10-15), default shipped config, 25s windows, 5s warmup, 4 valid repetitions per point withfail=0as the validity gate (invalid windows are recorded and retried, never silently dropped), rig-floor reference windows against the mock directly, per-window gateway CPU% and peak-RSS capture, and one on-CPU flamegraph at the c=128 saturation point (perf + inferno, the Establish the on-CPU profiling workflow: adopt cargo-flamegraph and add a dedicated profiling build profile #847 workflow, with a stripped-binary refusal gate).Results land as one directory per run:
results.jsonl(one JSON object per window),meta.json(commit, binary and instrument sha256s, core split, kernel, method parameters), the flamegraph SVG, generated configs, and logs.Default-config discipline
A config line exists only if the process cannot run the benchmark without it, following the published methodology's rule; the full claim set and per-line justification are in the README and the runner. Runtime deviations are limited to
ulimit -n 65536(c=768 exceeds the 1024 default) andkernel.perf_event_paranoid=1(flamegraph sampling only, reverts on reboot).Two hardening details worth review attention:
BENCH_SRC_*variables and defensively unsets everyAISIX_*variable before launching the gateway: aisix readsAISIX_*environment variables as config overrides, so harness metadata in the measured process's environment would either pollute the measurement or refuse boot.perf.data; a rendering problem must never abort or invalidate the measurement itself.Validation
One full pipeline run end-to-end on the rig (provision from a fresh box, native build, all four load points, both rig floors, flamegraph rendered): every measured window passed the
fail=0validity gate on the first attempt, and the thread-per-core startup self-check (tpc_workers) confirmed the intended serving mode. Baseline numbers are tracked in the program's internal notes rather than in this PR.Not in scope
Multi-gateway comparison orchestration (program item 1's side-by-side legs) builds on this harness in a follow-up.
Summary by CodeRabbit
New Features
Documentation