Skip to content

fix(divan): support threaded walltime benchmarks - #189

Open
not-matthias wants to merge 3 commits into
mainfrom
cod-3324-codspeed-runner-hangs-when-using-divans-threads-feature
Open

fix(divan): support threaded walltime benchmarks#189
not-matthias wants to merge 3 commits into
mainfrom
cod-3324-codspeed-runner-hangs-when-using-divans-threads-feature

Conversation

@not-matthias

@not-matthias not-matthias commented Sep 1, 2026

Copy link
Copy Markdown
Member

Changes in this PR:

  • Prevent threaded Divan walltime benchmarks from concurrently writing to the process-wide instrument-hooks FIFOs (this alone doesn't fix the issue)
  • Divan workers record monotonic bounds locally, then the coordinator emits one marker pair for the complete parallel sampling round. Benchmark bodies remain parallel.

As discussed, depends on CodSpeedHQ/instrument-hooks#30

Exercise Divan native threads with four workers in the walltime benchmark suite. Keep the simulation compatibility path single-threaded because its benchmark options do not expose the native threads setting.
@codspeed-hq

codspeed-hq Bot commented Sep 1, 2026

Copy link
Copy Markdown

Merging this PR will regress 15 benchmarks

⚠️ 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

⚡ 24 improved benchmarks
❌ 15 regressed benchmarks
✅ 552 untouched benchmarks
🆕 3 new benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime bench_array1[1] 1 ns 35 ns -97.14%
WallTime iter_batched_large_input 5 ns 7 ns -28.57%
WallTime find_highest_set_bit[255] 5 ns 6 ns -16.67%
WallTime iter_batched_ref_large_input 5 ns 6 ns -16.67%
WallTime graph_coloring[5] 1 µs 1.2 µs -12.64%
WallTime recursive_memoized[HashMap<u64, u64>, 0] 16 ns 18 ns -11.11%
WallTime iter_batched_per_iteration 43 ns 47 ns -8.51%
WallTime n_queens_solver[4] 2.1 µs 2.3 µs -8.47%
WallTime iter_with_setup 45 ns 48 ns -6.25%
Simulation process_items_with_counter 1.8 µs 1.9 µs -5.84%
WallTime init_array[42] 36 ns 38 ns -5.26%
WallTime graph_coloring[6] 1.1 µs 1.2 µs -5.11%
Simulation b 4.1 µs 4.3 µs -3.8%
Simulation b 4.8 µs 4.9 µs -3.34%
Simulation fibonacci_custom_measurement 1.6 µs 1.7 µs -3.28%
WallTime add_two_integers[(255, 255)] 20 ns 7 ns ×2.9
WallTime add_two_integers[(42, 13)] 20 ns 7 ns ×2.9
Simulation Iterative 177.1 ns 121.5 ns +45.72%
Simulation Iterative[20] 177.1 ns 121.5 ns +45.72%
Simulation Iterative[21] 178.4 ns 122.8 ns +45.24%
... ... ... ... ... ...

ℹ️ 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-3324-codspeed-runner-hangs-when-using-divans-threads-feature (5a9838f) with main (69f74b6)

Open in CodSpeed

@codspeed-hq

codspeed-hq Bot commented Sep 1, 2026

Copy link
Copy Markdown

Unable to generate the flame graphs

The performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists.

@avalanche-staging

avalanche-staging Bot commented Sep 2, 2026

Copy link
Copy Markdown

Congrats! CodSpeed is installed 🎉

🆕 622 new benchmarks were detected.

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

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-3324-codspeed-runner-hangs-when-using-divans-threads-feature branch from f6b60a3 to 224cf47 Compare September 2, 2026 13:42
@not-matthias
not-matthias marked this pull request as ready for review September 2, 2026 13:42
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

The PR enables threaded Divan walltime benchmarks without concurrent writes to the process-wide instrument-hooks transport.

  • Serializes all Linux instrument-hooks FFI transport operations across handles.
  • Collects per-worker monotonic bounds and emits one aggregate marker envelope per parallel sampling round.
  • Adds a four-worker native Divan benchmark to the walltime integration fixture.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable correctness or security issues identified.

The transport is serialized without enclosing benchmark execution, worker samples are fully collected before aggregation, and the runner accepts timestamped markers independently of their delivery time.

Important Files Changed

Filename Overview
crates/codspeed/src/instrument_hooks/mod.rs Adds process-wide serialization around Linux instrument-hooks FFI operations and keeps each timestamp pair adjacent.
crates/divan_compat/divan_fork/src/bench/mod.rs Moves marker transport out of worker threads and emits aggregate bounds after each parallel sample round.
crates/divan_compat/divan_fork/src/stats/sample.rs Extends internal raw samples with the monotonic benchmark bounds required for coordinator aggregation.
crates/divan_compat/benches/thread_example.rs Adds a four-worker benchmark on the native Divan path while retaining single-threaded simulation compatibility.

Sequence Diagram

sequenceDiagram
  participant C as Divan coordinator
  participant W as Worker threads
  participant H as InstrumentHooks
  participant R as Runner FIFO
  C->>W: Start parallel sampling round
  par Worker samples
    W->>W: Capture monotonic start
    W->>W: Run benchmark iterations
    W->>W: Capture monotonic end
  end
  W-->>C: Return samples and timestamp bounds
  C->>C: Select minimum start and maximum end
  C->>H: Add aggregate benchmark timestamps
  H->>H: Acquire process-wide transport lock
  H->>R: BenchmarkStart(minimum start)
  H->>R: BenchmarkEnd(maximum end)
  H->>H: Release transport lock
Loading

Reviews (1): Last reviewed commit: "fix(divan): support threaded walltime be..." | Re-trigger Greptile

@GuillaumeLagrange GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

olgtm, let me know your thoughts about the thread safety being insturment-hooks' responsibility, if you have compelling arguments I'll approve.

Although closing thoughts: how does the simulation instrument behave with the threads option?

Comment thread crates/codspeed/src/instrument_hooks/mod.rs Outdated
The runner FIFO transactions and the environment maps are now serialized inside instrument-hooks itself, so callers can invoke the C API from several threads without an external lock.
Aggregate worker timestamps into one benchmark marker envelope per Divan sampling round so benchmark bodies stay parallel without interleaving FIFO requests.
@not-matthias
not-matthias force-pushed the cod-3324-codspeed-runner-hangs-when-using-divans-threads-feature branch from 224cf47 to 5a9838f Compare September 4, 2026 14:22
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.

2 participants