[unbalancedparentheses]

What and why

The 0.8 release gate found async burst −37% and default request() −24% against main, by hand, on fede. This adds a check that catches that class of regression on every PR touching the hot path.

• benchmarks/hotpath/driver.rs: one portable driver (#[actor] handlers, request(), send() only, so it builds on any recent revision). hotpath.py copies the head's driver into both trees, so base and head run the same workload.
• benchmarks/hotpath.py gate (CI): instructions per operation under callgrind, at 2,000 and 6,000 operations (differenced, so process start and setup cancel), median of 3, head vs merge base. Fails if any benchmark grows more than 3% (--threshold, HOTPATH_THRESHOLD in the workflow). Per-benchmark table in the job summary.
• Gated benchmarks are the deterministic ones: async on a Tokio current-thread runtime (burst, sequential request(), 16-task request(), send from 1 and 2 producers: identical counts run to run) and threads-mode sequential request() (within 0.5% on fede, 0.9% on the runner). Multi-thread runtimes and free-running OS-thread producers vary up to ±20% in instructions from run to run, so threads-mode burst/send and the multi-thread async variants are wall-clock only.
• benchmarks/hotpath.py abba (manual, pre-release): wall-clock ABBA A/B on fixed cores, median and range; added to the release checklist.
• .github/workflows/perf-gate.yml: on every PR and merge_group, but it only builds and measures when concurrency/src, rt/src, macros/src or the driver changed; otherwise it passes with a "skipped" summary, so it can be a required check (a paths: filter would leave a required check pending).

How it was verified

Self-test on fede (nice 19).

main vs main (two runs), all PASS: every async benchmark 0.0% (spread ±0), threads_request_seq +0.1% / +0.2%. Same on the GitHub runner (this PR's own perf-gate run): async 0.0%, threads −0.9%.

#240 head 7f716152 vs main 0791ace5: FAIL, exit 1:

benchmark	base Ir/op	head Ir/op	change	max spread	result
async_ct_burst	3,825	4,467	+16.8%	±0	FAIL
async_ct_request_seq	3,395	4,059	+19.6%	±1	FAIL
async_ct_request_conc16	2,512	3,278	+30.5%	±0	FAIL
async_ct_send_p1	839	1,015	+21.0%	±0	FAIL
async_ct_send_p2	833	1,011	+21.3%	±0	FAIL
threads_request_seq	3,309	2,397	−27.6%	±23	ok

Limit: instruction counts miss contention, wake-ups and cache-line traffic, so the wall-clock A/B stays a release step (docs/TESTING.md, "Performance gate").

[github-actions]

🤖 Claude Code Review

Review: PR #270, hot-path perf gate

The approach is sound: callgrind on deterministic workloads, differencing two sizes to cancel setup, the same driver in both trees, and an always-reporting required check. These are the issues I found, most important first.

Correctness / robustness

1. The head driver is compiled against the base tree (hotpath.py build(), driver.rs header).

• The base must compile the head's driver, which depends on tasks::ActorStart, threads::ActorStart, .start(), #[request_handler]/#[send_handler], and the tokio/futures deps.
• The CHANGELOG already announces a unified start API redesign for 0.9. That will probably change start()/ActorStart.
• Once it lands, every later PR fails the gate with a compile error in the base worktree, not with a perf result.
• Options:
  • Fall back to the base's own benchmarks/hotpath/driver.rs when it exists and the head's copy fails to build.
  • Or emit an explicit "skipped: driver incompatible with base" verdict instead of a traceback.
• Either way, document the procedure for PRs that intentionally change these APIs.

2. Scope filter misses dependency changes (perf-gate.yml, the grep -qE line).

• A tokio bump in Cargo.lock, or a feature or profile change in Cargo.toml / concurrency/Cargo.toml, can move hot-path instruction counts without triggering the gate.
• Consider adding ^Cargo\.(toml|lock)$|^(concurrency|rt|macros)/Cargo\.toml$ to the pattern.

3. instructions() gives an opaque error on unexpected output (hotpath.py ~L81).

• IR.search(out.stderr).group(1) raises AttributeError if callgrind's output format differs.
• Check for None and sys.exit with the stderr tail, as is already done for non-zero exit.

4. build() hides git and build errors.

• capture_output=True on git worktree add means a failure shows only CalledProcessError without git's stderr.
• With --work reused locally, a stale work/base directory makes worktree add fail even with --force, because the directory is non-empty.
• Suggested fixes:
  • Run git worktree remove --force (plus git worktree prune) before adding.
  • Print e.stderr on failure.

5. Ops accounting assumes n is divisible by the worker count (driver.rs n / tasks, n / producers, n / workers).

• The defaults (2000, 6000, 500000) happen to divide by 16 and 2. Any other --small, --large or --ops silently skews Ir/op and ops/s.
• Either have the driver print the actual op count (n / k * k) and use that in per_op/wallclock, or assert divisibility.

6. Division by b (gate(), delta = 100 * (h / b - 1)).

• This is fine in practice, but a zero or negative per-op difference would crash or produce nonsense.
• A guard is cheap.

Minor

• abba(): csv = open(...) is never closed. Use with, or close it in finally.
• driver.rs comment in the async_send_p* current-thread branch: "Two producer tasks" is wrong for p1. It should say "Producer tasks".
• driver.rs main: args[1] and args[2] panic with an index error when arguments are missing. A usage message would be friendlier, though this matters little for an internal tool.
• perf-gate.yml build time: two cold release builds with no cache, under a 40-minute timeout. Swatinem/rust-cache (keyed per side) or caching ~/.cargo/registry would cut wall time noticeably.
• Release checklist: the 5% wall-clock threshold differs from the 3% gate threshold. That is reasonable, but a one-line rationale in TESTING.md would avoid confusion.

Looks good

• The determinism split (list-gate vs list) is well reasoned, and the TESTING.md section documents why.
• Fence after sends on the multi-thread path: the producers are joined before the fence request, so FIFO ordering through the single mailbox holds.
• std::process::exit(0) to skip teardown is fine, since teardown sits outside every measurement.
• Separate CARGO_TARGET_DIR per side, to avoid crate-hash collisions, is a good catch.

---

Automated review by Claude · claude-opus-5-5 · custom prompt

[github-actions]

🤖 Gemini Code Review

This PR is well-designed and clearly documented. Using Callgrind instruction differencing across two workload sizes on a current_thread runtime to cancel startup/setup overhead is a solid approach for deterministic CI gating.

A few minor suggestions:

1. .github/workflows/perf-gate.yml:39 — Trigger gate when the workflow file itself changes

   if git diff --name-only "$base" HEAD | grep -qE '^(concurrency/src|rt/src|macros/src)/|^benchmarks/hotpath|^\.github/workflows/perf-gate\.yml$'; then

   Currently, a PR that only edits .github/workflows/perf-gate.yml (e.g., changing flags, runner config, or toolchain version) will skip the build and measurement steps.

2. benchmarks/hotpath.py:148-157 — Interleaving granularity in abba mode

       for rep in range(args.reps):
           order = [("base", base_bin), ("head", head_bin)]
           if rep % 2:
               order.reverse()
           for side, binary in order:
               for name in names:

   Iterating for name in names: outside for side, binary in order: (or reversing order per benchmark) would run base and head back-to-back for each individual benchmark, rather than separating a benchmark's paired base and head measurements by the time taken to run all 15 benchmarks.

3. benchmarks/hotpath.py:145 — Unclosed file handle when --csv is used
   csv = open(args.csv, "w") is not explicitly closed (or flushed per repetition if inspecting progress during a long run). Consider using contextlib.ExitStack or closing it before returning.

---

Automated review by Gemini · gemini-4-argon · custom prompt

[github-actions]

🤖 Codex Code Review

• [P2] Dependency changes bypass the performance gate — .github/workflows/perf-gate.yml:39. The filter excludes Cargo.lock and Cargo manifests. A Tokio upgrade or feature change can affect mailbox, scheduling, and request performance while this required check passes without measuring anything. Include the workspace lockfile and relevant manifests in the trigger.

Rust validation was blocked by the read-only rustup directory; Valgrind is also unavailable locally.

---

Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt

[github-actions]

🤖 Grok Code Review

The gate design is sound: one shared driver, callgrind differencing to cancel setup, and only the deterministic cases are required. A few issues can flake or break the check.

Correctness

• benchmarks/hotpath.py writes callgrind output to /dev/null (instructions(), around the valgrind invocation). Callgrind seeks in that file to patch offsets; /dev/null is not seekable, so the run can fail or the Collected line can be missing. IR.search(...).group(1) then raises AttributeError instead of a useful error. Use a temp file and fail if the regex misses.
• driver.rs runtime() calls enable_all(). That starts the IO/time driver (extra threads) even for async_ct_*, which the gate treats as single-thread deterministic. Those benches do not need IO or time; build a bare current-thread runtime so park/wake noise is not in the instruction delta.
• async_ct_send_* sends 64 messages before yield_now() (driver.rs send loop). On a current-thread runtime the actor cannot drain until that yield. If send blocks or returns Err when the mailbox is full and capacity is under 64, this deadlocks or panics. Yield more often than the bound, or assert capacity.
• process::exit(0) (driver.rs main) skips teardown so Drop cost stays out of the count, which is right, but other threads can still be running. That adds callgrind noise on threads_request_seq, and for abba a late log line can steal wallclock()'s last stdout line (ops / float(out.split(",")[2])). Print a marked record after a quiet shutdown, or parse a known prefix.
• Ops that are not divisible by the worker count are silently truncated (n / tasks, n / producers, n / workers) while the tool still divides by the requested n. Defaults happen to divide evenly; assert divisibility so --ops cannot lie.
• The gate copies the head driver into the base worktree (build()). Any PR that changes #[actor] / request / send — this repo is still doing that — fails at the base build even when the change is intentional. Fall back to each revision's own driver, or skip with an explicit override.

CI reliability

• gate() fails only on delta > threshold and ignores spread. With 3 reps, a noisy threads_request_seq (two threads under callgrind, docs already allow ~0.5%) can flake a required check, or a real regression can hide inside the spread. Fail or rerun when spread is a large fraction of the threshold, and treat a negative per-op sample as a bad measurement.
• Path filter does not include Cargo.toml / profile / dependency bumps, so a hot-path regression from those never runs. The workflow header also omits benchmarks/hotpath, which the grep does match.
• Confirm concurrency/examples/ exists on the base revision and that tokio and futures are dependencies visible to examples; otherwise cargo build --example hotpath --locked fails. Also confirm two #[actor] impl Sink blocks do not emit colliding generated symbols — rename one actor if the macro keys only on the type name.

Smaller

• abba always returns 0, so the release checklist's 5% rule is easy to miss. Exit non-zero when the paired median exceeds that.
• async_burst at --ops 500_000 holds every reply future at once; that can blow memory on a laptop. Cap burst or document it.
• Close the CSV with a context manager; guard h / b when b == 0.

---

Automated review by Grok (xAI) · grok-4.7 · custom prompt

[github-actions]

🤖 Kimi Code Review

Findings

1. Medium — --head does not select the driver revision (benchmarks/hotpath.py:31, 51-63).
   build() always copies DRIVER from the current working tree. Consequently, commands such as --head origin/my-branch can benchmark an unrelated or uncommitted local driver, contrary to the documentation’s “head’s copy” claim. Extract the driver from the resolved head commit with git show, or explicitly require it to match a clean working-tree HEAD.

2. Medium — the gate can pass without running any benchmarks (benchmarks/hotpath.py:66-69, 102-104).
   A typo in --only, or a driver regression that makes list-gate return an empty list, produces an empty table and a successful verdict. Fail when the selected set is empty, and reject requested names that the driver did not report.

3. Low — performance-relevant dependency/build changes skip CI measurement (.github/workflows/perf-gate.yml:39).
   Changes limited to Cargo.lock, crate/workspace Cargo.toml, build.rs, toolchain, or codegen configuration can alter hot-path dependencies or optimization without triggering the gate. Consider including those inputs, particularly lockfile and manifest changes.

4. Low — malformed callgrind output causes an unhelpful exception (benchmarks/hotpath.py:75-82).
   IR.search(out.stderr).group(1) assumes a match even when Valgrind exits successfully without the expected summary. Check the match and fail with captured stderr; similarly, validating large > small and positive reps would make failures clearer.

Otherwise, the measurement design looks sound: per-side target directories, ABBA pairing, completed-send fencing, deterministic benchmark selection, and worktree cleanup are all handled carefully.

---

Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt

[github-actions]

🤖 DeepSeek Code Review

⚠️ Review failed: DeepSeek returned an empty content field (finish_reason=length, reasoning_content present=yes). Raise max_tokens or lower reasoning_effort ('none' disables thinking).

---

Automated review by DeepSeek

[github-actions]

🤖 DeepSeek Code Review

Overall this is a solid CI-only addition: the worktree isolation, instruction-difference method, and ABBA pairing are well thought out. A few issues worth addressing before merge:

1. benchmarks/hotpath/driver.rs:248 — stdout may not be flushed before process::exit
println! is generally line-buffered, but when stdout is piped by hotpath.py, relying on process::exit(0) to flush is fragile. If the final benchmark line is buffered, wallclock() will parse an empty output and fail. Prefer:

   use std::io::Write;
   std::io::stdout().flush().unwrap();
   std::process::exit(0);

   or simply let main return normally and drop the actor/runtime outside the timed region.

2. benchmarks/hotpath/driver.rs:138-140 — async_burst materializes all n futures
   futures::future::join_all(all) stores every request() future before polling. With the abba default --ops 500_000, this can use hundreds of MB and distort cache/memory behavior compared to the intended burst path. The gate sizes are fine (≤6000), but abba runs all benchmarks including async_burst. Consider using FuturesUnordered/buffer_unordered or lowering the abba default ops for this benchmark.

3. benchmarks/hotpath/driver.rs:132,149,166,194,231 — integer division drops remainders silently
   n / tasks, n / producers, and n / workers all truncate. For the current defaults these all happen to be divisible, but a user-supplied odd --ops or odd gate sizes will make the actual measured operation count less than requested. The instruction-difference denominator in hotpath.py then becomes wrong for the gate. Add an assert/distribute remainder, or clearly document the divisibility requirement.

4. benchmarks/hotpath.py:152 — CSV file is not closed explicitly
   csv = open(args.csv, "w") leaks the file handle until process exit. Use a context manager:

   with open(args.csv, "w") as csv_file:
       ...

5. benchmarks/hotpath.py:200-203 — cleanup ignores failures
   git worktree remove --force ... return codes are discarded. This can hide permissions/locking problems, especially when --work is reused. At minimum capture and print a warning, or check returncode and include it in the exit path.

6. perf-gate.yml scope misses dependency changes
   The hot-path gate only runs when concurrency/src, rt/src, or macros/src change. A Cargo.toml/Cargo.lock update can also affect release-mode instruction counts via dependency versions/features. Consider adding workspace manifests/lockfile to the trigger pattern.

None of these are blockers if the defaults are always used, but the driver is a benchmarking tool that should be robust to custom --ops/--small/--large, and the output-flushing issue could make CI flaky.

---

Automated review by DeepSeek · deepseek-v4-pro · custom prompt