diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ec0b066..b5ee2127 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,10 @@ on: pull_request: workflow_dispatch: +permissions: + contents: read + id-token: write + jobs: lint: runs-on: ubuntu-latest @@ -127,16 +131,22 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: cargo nextest run -p cargo-codspeed --partition hash:${{ matrix.partition }}/5 - compat-integration-test-instrumentation: - runs-on: ubuntu-latest + analysis-integration-test: strategy: matrix: - build-args: - - "-p codspeed" - - "-p codspeed-bencher-compat" - - "--features async_futures -p codspeed-criterion-compat" - - "-p codspeed-divan-compat" - - "-p codspeed-divan-compat-examples" + package: + - codspeed + - codspeed-bencher-compat + - codspeed-divan-compat + - codspeed-divan-compat-examples + - codspeed-criterion-compat + mode: + - memory + - simulation + include: + - package: codspeed-criterion-compat + features: async_futures + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 with: @@ -149,7 +159,11 @@ jobs: - run: cargo install --path crates/cargo-codspeed --locked - - run: cargo codspeed build ${{ matrix.build-args }} + - name: Build the benchmarks + run: | + # Remove the cargo config else it forces instrumentation mode + rm -f .cargo/config.toml + cargo codspeed build -m ${{matrix.mode}} -p ${{ matrix.package }} ${{ matrix.features && format('--features {0}', matrix.features) || '' }} - name: Run the benchmarks uses: CodSpeedHQ/action@main @@ -157,10 +171,9 @@ jobs: MY_ENV_VAR: "YES" with: run: cargo codspeed run - mode: instrumentation - token: ${{ secrets.CODSPEED_TOKEN }} + mode: ${{ matrix.mode }} - compat-integration-test-walltime: + walltime-integration-test: runs-on: codspeed-macro strategy: matrix: @@ -180,7 +193,8 @@ jobs: - run: cargo install --path crates/cargo-codspeed --locked - - run: | + - name: Build the benchmarks + run: | # Remove the cargo config else it forces instrumentation mode rm -f .cargo/config.toml cargo codspeed build -p ${{ matrix.package }} @@ -192,7 +206,6 @@ jobs: with: run: cargo codspeed run mode: walltime - token: ${{ secrets.CODSPEED_TOKEN }} musl-build-check: strategy: @@ -228,8 +241,8 @@ jobs: - tests-without-cargo-codspeed - test-cargo-codspeed - msrv-check - - compat-integration-test-instrumentation - - compat-integration-test-walltime + - analysis-integration-test + - walltime-integration-test - musl-build-check steps: - uses: re-actors/alls-green@release/v1 diff --git a/crates/cargo-codspeed/src/build.rs b/crates/cargo-codspeed/src/build.rs index 4cc28b83..f0e6f26e 100644 --- a/crates/cargo-codspeed/src/build.rs +++ b/crates/cargo-codspeed/src/build.rs @@ -225,8 +225,10 @@ See `cargo codspeed build --help` for more information."); "-Cstrip=none".to_owned(), ]; - // Add the codspeed cfg flag if simulation mode is enabled - if measurement_mode == MeasurementMode::Simulation { + // Add the codspeed cfg flag if the benchmark should only run once + if measurement_mode == MeasurementMode::Simulation + || measurement_mode == MeasurementMode::Analysis + { flags.push("--cfg=codspeed".to_owned()); } diff --git a/crates/cargo-codspeed/src/measurement_mode.rs b/crates/cargo-codspeed/src/measurement_mode.rs index 171ac611..f9f64dae 100644 --- a/crates/cargo-codspeed/src/measurement_mode.rs +++ b/crates/cargo-codspeed/src/measurement_mode.rs @@ -9,6 +9,8 @@ pub enum MeasurementMode { #[value(alias = "instrumentation")] Simulation, Walltime, + #[value(alias = "memory")] + Analysis, } impl fmt::Display for MeasurementMode { @@ -19,6 +21,7 @@ impl fmt::Display for MeasurementMode { match self { MeasurementMode::Simulation => "simulation", MeasurementMode::Walltime => "walltime", + MeasurementMode::Analysis => "analysis", } ) } diff --git a/crates/codspeed/instrument-hooks b/crates/codspeed/instrument-hooks index 1752e9e4..504dc86b 160000 --- a/crates/codspeed/instrument-hooks +++ b/crates/codspeed/instrument-hooks @@ -1 +1 @@ -Subproject commit 1752e9e4eae585e26703932d0055a1473dd77048 +Subproject commit 504dc86bacb8e0ae44e08b247c2eee09d8d4f6df diff --git a/crates/codspeed/src/codspeed.rs b/crates/codspeed/src/codspeed.rs index 937036be..4a9db8a7 100644 --- a/crates/codspeed/src/codspeed.rs +++ b/crates/codspeed/src/codspeed.rs @@ -10,28 +10,62 @@ pub fn display_native_harness() { println!("Harness: codspeed v{}", env!("CARGO_PKG_VERSION"),); } +#[derive(PartialEq)] +pub enum InstrumentationStatus { + /// Instrumentation detected via inline assembly directly + Valgrind, + /// Instrumentation detected via InstrumentHooks + InstrumentHooks(&'static crate::instrument_hooks::InstrumentHooks), + NotInstrumented, +} + +impl InstrumentationStatus { + pub fn is_instrumented(&self) -> bool { + *self != InstrumentationStatus::NotInstrumented + } +} + pub struct CodSpeed { benchmarked: Vec, current_benchmark: CString, group_stack: Vec, - is_instrumented: bool, + instrumentation_status: InstrumentationStatus, } impl CodSpeed { pub fn new() -> Self { - let is_instrumented = measurement::is_instrumented(); - if !is_instrumented { + use crate::instrument_hooks::InstrumentHooks; + let instrumentation_status = { + // We completely bypass InstrumentHooks if we detect Valgrind via inline assembly + // Until we can reliably get rid of the inline assembly without causing breaking + // changes in CPU simulation measurements by switching to InstrumentHooks only, we need + // to keep this separation. + if measurement::is_instrumented() { + InstrumentationStatus::Valgrind + } else { + let hooks_instance = InstrumentHooks::instance(); + if hooks_instance.is_instrumented() { + InstrumentationStatus::InstrumentHooks(hooks_instance) + } else { + InstrumentationStatus::NotInstrumented + } + } + }; + + if instrumentation_status.is_instrumented() { println!( "{} codspeed is enabled, but no performance measurement will be made since it's running in an unknown environment.", "NOTICE:".to_string().bold() ); - } + }; + measurement::set_metadata(); + Self { benchmarked: Vec::new(), current_benchmark: CString::new("").expect("CString::new failed"), group_stack: Vec::new(), - is_instrumented, + instrumentation_status, } } @@ -46,19 +80,39 @@ impl CodSpeed { #[inline(always)] pub fn start_benchmark(&mut self, name: &str) { self.current_benchmark = CString::new(name).expect("CString::new failed"); + + if let InstrumentationStatus::InstrumentHooks(hooks_instance) = &self.instrumentation_status + { + let _ = hooks_instance.start_benchmark(); + } + + // We intentionally do this no matter the instrumentation status. + // The overhead in case we are not running valgrind is extremely low, and we do not want to + // add a conditionnal branch to valgrind measurements. measurement::start(); } #[inline(always)] pub fn end_benchmark(&mut self) { + // We intentionally do this no matter the instrumentation status. + // The overhead in case we are not running valgrind is extremely low, and we do not want to + // add a conditionnal branch to valgrind measurements. measurement::stop(&self.current_benchmark); + if let InstrumentationStatus::InstrumentHooks(hooks_instance) = &self.instrumentation_status + { + let _ = hooks_instance.stop_benchmark(); + let _ = + hooks_instance.set_executed_benchmark(&self.current_benchmark.to_string_lossy()); + } self.benchmarked .push(self.current_benchmark.to_str().unwrap().to_string()); - let action_str = if self.is_instrumented { + + let action_str = if self.instrumentation_status.is_instrumented() { "Measured" } else { "Checked" }; + let group_str = if self.group_stack.is_empty() { "".to_string() } else { diff --git a/crates/codspeed/src/instrument_hooks/mod.rs b/crates/codspeed/src/instrument_hooks/mod.rs index 2c47ebb4..ff2edd93 100644 --- a/crates/codspeed/src/instrument_hooks/mod.rs +++ b/crates/codspeed/src/instrument_hooks/mod.rs @@ -8,6 +8,7 @@ mod linux_impl { use std::ffi::CString; use std::sync::OnceLock; + #[derive(PartialEq)] pub struct InstrumentHooks(*mut ffi::InstrumentHooks); unsafe impl Send for InstrumentHooks {} @@ -129,6 +130,15 @@ mod linux_impl { ffi::instrument_hooks_current_timestamp() } } + + pub fn disable_callgrind_markers() { + unsafe { + ffi::instrument_hooks_set_feature( + ffi::instrument_hooks_feature_t_FEATURE_DISABLE_CALLGRIND_MARKERS, + true, + ) + }; + } } impl Drop for InstrumentHooks { @@ -142,6 +152,7 @@ mod linux_impl { #[cfg(not(use_instrument_hooks))] mod other_impl { + #[derive(PartialEq)] pub struct InstrumentHooks; impl InstrumentHooks { @@ -175,6 +186,8 @@ mod other_impl { pub fn current_timestamp() -> u64 { 0 } + + pub fn disable_callgrind_markers() {} } } diff --git a/crates/divan_compat/examples/Cargo.toml b/crates/divan_compat/examples/Cargo.toml index 1e9ea793..40c57f3c 100644 --- a/crates/divan_compat/examples/Cargo.toml +++ b/crates/divan_compat/examples/Cargo.toml @@ -44,3 +44,7 @@ harness = false [[bench]] name = "counters" harness = false + +[[bench]] +name = "alloc" +harness = false diff --git a/crates/divan_compat/examples/benches/alloc.rs b/crates/divan_compat/examples/benches/alloc.rs new file mode 100644 index 00000000..e32d380f --- /dev/null +++ b/crates/divan_compat/examples/benches/alloc.rs @@ -0,0 +1,40 @@ +use std::{ + alloc::Layout, + collections::{HashMap, HashSet}, +}; + +#[divan::bench] +fn allocate() { + println!("Hello, world!"); + + let vec = vec![1, 2, 3]; + println!("{vec:?}"); + + let mut map = HashMap::new(); + map.insert("key", "value"); + println!("{map:?}"); + + let mut set = HashSet::new(); + set.insert("apple"); + set.insert("banana"); + println!("{set:?}"); + + std::thread::sleep(std::time::Duration::from_secs(1)); + + let mut bytes_vec = vec![0u8; 0x100]; + println!("{:?}", bytes_vec.len()); + + bytes_vec.extend(&vec![0u8; 0x1000]); + + // Alloc 42 bytes of memory per iteration (4200 bytes total) + for _ in 0..100 { + let layout = Layout::new::<[u8; 42]>(); + let memory = unsafe { std::alloc::alloc(layout) }; + core::hint::black_box(memory); + unsafe { std::alloc::dealloc(memory, layout) }; + } +} + +fn main() { + divan::main(); +}