Skip to content

feat(divan): run each benchmark in its own process - #194

Open
not-matthias wants to merge 8 commits into
mainfrom
cod-3799-explore-process-per-benchmark-in-codspeed-rust
Open

not-matthias wants to merge 8 commits into
mainfrom
cod-3799-explore-process-per-benchmark-in-codspeed-rust

Conversation

@not-matthias

@not-matthias not-matthias commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

TLDR: Benchmarks in one bench binary share a process, so one benchmark can change the allocator state for every benchmark after it. With CODSPEED_PROCESS_ISOLATION=1, divan_compat now re-executes the binary once per benchmark (simulation mode only).

  • The parent keeps CLI filtering and URI generation, then spawns current_exe() with __CODSPEED_DIVAN_BENCH_ID=<entry>[:<arg>].
  • The child runs only that entry; its hooks and callgrind dump use the child's PID.
  • No changes to cargo-codspeed or the runner: they rely on the existing --trace-children=yes.
  • New allocator_ripple bench: three identical calloc(4 MiB) benchmarks. The first one frees an mmapped chunk, which raises glibc's mmap threshold, so the later ones get heap memory that must be cleared with memset.

Measured (allocator_ripple, callgrind Ir, same on reruns):

Bench Shared process Isolated
zeroed_buffer[1] 821 543
zeroed_buffer[2] 128,070 543
zeroed_buffer[3] 229,939 543

Variance:

executor set runs benches median CV % p90 CV % max CV % CV > 1% CV > 0
valgrind before (prod, last 10) 10 215 0.256 6.593 14.381 67 158
valgrind isolated (staging, 10) 10 218 0.000 0.000 0.188 0 7
memory before (prod, last 10) 10 215 0.000 0.000 0.000 0 0
memory isolated (staging, 10) 10 218 0.000 0.000 0.000 0 0

Review notes

  • Opt-in via env var for now. Each benchmark pays a process start-up under valgrind (about 0.28 s each on a small binary).
  • Untested: walltime (divan_fork), memory-mode tracking of the child processes, criterion and bencher.
  • Doesn't help with binary layout or code-generation shifts.

Refs COD-3799

@codspeed

codspeed Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 6.69%

⚠️ 59 benchmarks measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. These results are not comparable, so they count as unchanged.

Preventing compiler optimizations

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 31 improved benchmarks
❌ 37 regressed benchmarks
✅ 526 untouched benchmarks
🆕 12 new benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory fib_in_thread[32] 424 B 760 B -44.21%
❌ Memory fib_in_thread[30] 424 B 760 B -44.21%
❌ Memory fib_in_thread[31] 424 B 760 B -44.21%
❌ Memory fib_in_thread_bench_local[31] 424 B 760 B -44.21%
❌ Memory fib_in_thread_bench[32] 424 B 760 B -44.21%
❌ Memory fib_in_thread_bench_local[32] 424 B 760 B -44.21%
❌ Memory fib_in_thread_bench[30] 424 B 760 B -44.21%
❌ Memory fib_in_thread_bench[31] 424 B 760 B -44.21%
❌ WallTime mul 2 ns 3 ns -33.33%
❌ Simulation Iterative 121.5 ns 177.1 ns -31.38%
❌ Simulation Iterative[20] 121.5 ns 177.1 ns -31.38%
❌ Simulation Iterative[21] 122.8 ns 178.4 ns -31.15%
❌ Simulation generate_combinations[7] 21.4 µs 27.6 µs -22.35%
❌ Simulation recursive_memoized[HashMap<u64, u64>, 10] 8.6 µs 10.8 µs -19.74%
❌ Simulation add_two_integers[(42, 13)] 233.8 ns 288 ns -18.81%
❌ Simulation add_two_integers[(255, 255)] 233.8 ns 288 ns -18.81%
❌ Simulation add_two_integers[(1024, -1024)] 295.2 ns 349.4 ns -15.5%
❌ Simulation add_two_integers[(65535, -65535)] 307 ns 361.1 ns -15%
❌ Simulation generate_combinations[8] 28.8 µs 33.9 µs -14.97%
❌ Simulation generate_combinations[9] 35.7 µs 41.8 µs -14.51%
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-3799-explore-process-per-benchmark-in-codspeed-rust (304fa86) with main (832f252)

Open in CodSpeed

@not-matthias
not-matthias force-pushed the cod-3799-explore-process-per-benchmark-in-codspeed-rust branch from c9919e1 to 3a215de Compare October 7, 2026 17:32
@not-matthias
not-matthias marked this pull request as ready for review October 7, 2026 17:33
@avalanche-staging

avalanche-staging Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Congrats! CodSpeed is installed 🎉

🆕 444 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

⚠️ This run is associated to a commit that was force-pushed

It won't be associated with its branch history. Be careful when comparing its performance to other runs.

Detected benchmarks


ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.


Open in CodSpeed

@not-matthias
not-matthias force-pushed the cod-3799-explore-process-per-benchmark-in-codspeed-rust branch from 3a215de to fa0b668 Compare October 7, 2026 17:34
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Adds process isolation option for benchmark runs.

Fix the hang with existing Tokio worker pools and satisfy the logging rule before merging.

Fix All in Claude CodeFindings

  1. P1 Existing async workers disappear ▶
  2. P2 Warning bypasses required logging ▶
  3. P2 Isolated runs are never tested ▶
Fix with agent prompt
### Issue 1
crates/codspeed/src/isolation.rs:78-79
With `CODSPEED_PROCESS_ISOLATION=1`, a Criterion benchmark that creates a multithreaded Tokio `Runtime` before `bench_function` can hang. `fork()` copies the runtime but not its worker threads. A closure using `b.to_async(&runtime)` and awaiting a spawned task then waits for workers that no longer exist. The parent also waits forever in `waitpid`.

Start a fresh executable for each benchmark, or keep this path disabled for benchmarks with existing worker threads.

### Issue 2
crates/codspeed/src/isolation.rs:39-42
The new Windows warning uses `eprintln!`. The repository rule says never to use `println!` or `eprintln!` in Rust and to use tracing macros instead. Replace this warning with `warn!` to satisfy the requirement before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 3
crates/codspeed/src/isolation.rs:75
No automated test or CI run enables `CODSPEED_PROCESS_ISOLATION`, so the new child path is never exercised. Adding `allocator_ripple` only tests the shared-process default. Changes that lose child results or hide child failures can therefore pass CI.

Add an isolated analysis run that checks all selected cases report results and that a failing child makes the parent fail.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds opt-in, fork-per-benchmark execution to the Divan and Criterion analysis adapters. The child emits measurement metadata, runs one benchmark, and returns its exit status to the parent.

  • Adds allocator_ripple to demonstrate allocator effects between benchmarks.
  • Sends CI uploads to staging.
  • Existing Tokio worker pools can leave isolated benchmarks hanging.
  • The new warning violates the logging rule, and CI never enables isolation.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Parent selects benchmark and builds URI] --> B{Isolation enabled?}
  B -->|No| C[Run benchmark in parent]
  B -->|Yes| D[Fork child]
  D --> E[Child emits metadata]
  E --> F[Child runs benchmark]
  F --> G[Child exits with success or failure]
  D --> H[Parent waits]
  G --> H
  H --> I{Child succeeded?}
  I -->|Yes| A
  I -->|No| J[Parent exits with child status]
Loading

Reviews (1) · Last reviewed commit: 3a215de · Reviewed by Greptile

Comment thread crates/codspeed/src/isolation.rs Outdated
Comment thread crates/codspeed/src/isolation.rs Outdated
Comment thread crates/codspeed/src/isolation.rs Outdated
The codspeed crate declared libc since its first commit but never used it.
Benchmarks of one bench binary share a process, so the allocator state one
benchmark leaves behind (raised mmap threshold, fragmented bins, retained
caches) changes the measurement of every benchmark after it. Adding or
changing one benchmark can then shift unrelated results.

Add `isolation::Isolation` for the harnesses. It is on by default and turned
off with `CODSPEED_PROCESS_ISOLATION=false`. `Isolation::run` re-executes the
bench binary with the same arguments and `CODSPEED_ISOLATED_BENCHMARK` set to
one benchmark URI, so each benchmark gets a fresh process with its own heap and
threads. The child only runs that benchmark (`only_benchmark`, which overrides
the CLI filters), and a failing child's exit code is propagated (128 + N for
signal N).

Re-exec instead of fork(): fork only copies the calling thread, so threads the
bench binary started before the harness (e.g. a Tokio runtime in `main`) would
be missing in the child and work sent to them would hang.
Route every benchmark of the analysis harness through `Isolation::run`. A
child process only matches its own benchmark URI and doesn't repeat the
"Skipped" lines the parent already printed.
All criterion compat benchmarks go through `BenchmarkGroup::run_bench`; wrap
the measurement in `Isolation::run` like divan. A child process replaces the
CLI filter with an exact match on its benchmark URI.
@not-matthias
not-matthias force-pushed the cod-3799-explore-process-per-benchmark-in-codspeed-rust branch 2 times, most recently from 4a49dfa to 9e80c3b Compare October 8, 2026 12:01
Build and run a divan fixture in simulation mode and check that:
- every benchmark runs in a fresh process by default, including one that uses
  a multi-threaded Tokio runtime started in `main`;
- `CODSPEED_PROCESS_ISOLATION=false` runs them all in one process;
- a panicking benchmark fails the run with the panic exit code.
Three identical benchmarks that allocate a zeroed 4 MiB buffer. In one
process the first one gets fresh mmap pages from glibc; freeing them raises
the mmap threshold, so the next ones get heap memory that calloc must clear.
Under callgrind they report 642, 127,593 and 229,862 instructions without
isolation, and 642 each with it.

`spawn_on_parent_runtime` awaits a task on a Tokio runtime started in `main`,
which only works when each isolated process starts its own runtime.
Every benchmark now runs in its own process. A harness regression that leaves
a child stuck must fail the job instead of blocking it until the GitHub
Actions default of 6 hours.
@not-matthias
not-matthias force-pushed the cod-3799-explore-process-per-benchmark-in-codspeed-rust branch from 9e80c3b to 304fa86 Compare October 8, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant