Skip to content

feat: add support for analysis mode #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat: add support for analysis mode #152

wants to merge 8 commits into from

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Nov 19, 2025

No description provided.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 20, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #152 will degrade performances by 12.84%

Comparing cod-1711-codspeed-rust-add-analysis-mode-support (fa4e22b) with main (9181bce)

Summary

⚡ 34 improvements
❌ 6 regressions
✅ 319 untouched
🆕 12 new
⏩ 7 skipped1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
🆕 WallTime iter N/A 1 ns N/A
🆕 WallTime iter_batched_large_input N/A 5 ns N/A
🆕 WallTime iter_batched_per_iteration N/A 45 ns N/A
🆕 WallTime iter_batched_ref_large_input N/A 5 ns N/A
🆕 WallTime iter_batched_ref_per_iteration N/A 45 ns N/A
🆕 WallTime iter_batched_ref_small_input N/A 5 ns N/A
🆕 WallTime iter_batched_small_input N/A 5 ns N/A
🆕 WallTime iter_with_large_drop N/A 5 ns N/A
🆕 WallTime iter_with_large_setup N/A 5 ns N/A
🆕 WallTime iter_with_setup N/A 45 ns N/A
⚡ WallTime large_drop 204 µs 173.6 µs +17.49%
⚡ WallTime large_setup 20 ns 11 ns +81.82%
⚡ WallTime small_setup 2 ns 1 ns ×2
❌ WallTime iter_with_setup 36 ns 39 ns -7.69%
❌ WallTime iter_batched_per_iteration 36 ns 40 ns -10%
❌ WallTime from_elem_decimal[1024] 79 ns 83 ns -4.82%
⚡ WallTime init_array[1000] 2.1 µs 2 µs +7.02%
⚡ WallTime fibo_10 2 ns 1 ns ×2
❌ WallTime fib_10 375 ns 425 ns -11.76%
❌ WallTime fib_20 45.6 µs 52.3 µs -12.84%
... ... ... ... ... ...

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

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@not-matthias not-matthias changed the title chore: wip [skip ci] feat: add support for analysis mode Nov 20, 2025
@not-matthias not-matthias force-pushed the cod-1711-codspeed-rust-add-analysis-mode-support branch 2 times, most recently from 5bab4b1 to f2e74d7 Compare November 21, 2025 15:33
@not-matthias not-matthias requested a review from Copilot November 21, 2025 15:33
Copilot AI reviewed Nov 21, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for a new "analysis" measurement mode to the CodSpeed benchmarking framework. The analysis mode enables instrumentation similar to simulation mode and integrates with instrument hooks for tracking benchmark execution.

Key changes:

  • Introduced a new Analysis variant to the MeasurementMode enum
  • Integrated InstrumentHooks for tracking benchmark lifecycle events
  • Removed the deprecated is_instrumented() function and RunningOnValgrind enum variant

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/divan_compat/examples/benches/alloc.rs New benchmark file for testing memory allocation scenarios
crates/divan_compat/examples/Cargo.toml Added the new alloc benchmark to the build configuration
crates/codspeed/src/request/mod.rs Removed unused RunningOnValgrind client request variant
crates/codspeed/src/measurement.rs Removed deprecated is_instrumented() function
crates/codspeed/src/codspeed.rs Integrated InstrumentHooks for benchmark lifecycle tracking
crates/cargo-codspeed/src/measurement_mode.rs Added Analysis variant to measurement modes
crates/cargo-codspeed/src/build.rs Extended codspeed cfg flag to apply to analysis mode
.github/workflows/ci.yml Added CI workflow for testing memory analysis integration
crates/codspeed/instrument-hooks Updated submodule commit reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@not-matthias not-matthias marked this pull request as ready for review November 24, 2025 09:52
@not-matthias not-matthias force-pushed the cod-1711-codspeed-rust-add-analysis-mode-support branch from f2e74d7 to 435a805 Compare November 24, 2025 10:26
GuillaumeLagrange
GuillaumeLagrange requested changes Nov 25, 2025
View reviewed changes
Copy link
Contributor

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

olgtm

@not-matthias not-matthias force-pushed the cod-1711-codspeed-rust-add-analysis-mode-support branch 2 times, most recently from 433456e to 274a517 Compare November 27, 2025 15:04
@not-matthias not-matthias force-pushed the cod-1711-codspeed-rust-add-analysis-mode-support branch from 274a517 to 7eaae17 Compare November 27, 2025 15:23
@not-matthias not-matthias force-pushed the cod-1711-codspeed-rust-add-analysis-mode-support branch from 7eaae17 to fa4e22b Compare November 27, 2025 15:27
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 27, 2025

Unable to generate the performance report

The benchmarks did not report any results. Please check the documentation to check your benchmarking setup.

GuillaumeLagrange
GuillaumeLagrange approved these changes Nov 27, 2025
View reviewed changes
Copy link
Contributor

@GuillaumeLagrange GuillaumeLagrange left a comment

Choose a reason for hiding this comment

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

olgtm, although CI outputed the No performance analysis stuff, is this expected with memory mapping not being merged all the way ?

crates/codspeed/src/instrument_hooks/mod.rs
instance
.set_integration("codspeed-rust", env!("CARGO_PKG_VERSION"))
.expect("Failed to set integration");
InstrumentHooks::disable_callgrind_markers();
Copy link
Contributor

@GuillaumeLagrange GuillaumeLagrange Nov 27, 2025

Choose a reason for hiding this comment

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

Small comment to explain that we have inline assembly for rust integration therefore we do not use instrument hooks for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@GuillaumeLagrange GuillaumeLagrange GuillaumeLagrange approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@not-matthias @GuillaumeLagrange