diff --git a/crates/codspeed/src/instrument_hooks/mod.rs b/crates/codspeed/src/instrument_hooks/mod.rs index b77ef426..4651013d 100644 --- a/crates/codspeed/src/instrument_hooks/mod.rs +++ b/crates/codspeed/src/instrument_hooks/mod.rs @@ -6,18 +6,30 @@ mod linux_impl { use super::ffi; use std::ffi::CString; - use std::sync::OnceLock; + use std::sync::{Mutex, OnceLock}; + + static INSTRUMENT_HOOKS_LOCK: Mutex<()> = Mutex::new(()); + + fn with_transport_lock(operation: impl FnOnce() -> T) -> T { + let _guard = INSTRUMENT_HOOKS_LOCK + .lock() + .expect("InstrumentHooks mutex poisoned"); + operation() + } #[derive(PartialEq)] pub struct InstrumentHooks(*mut ffi::InstrumentHooks); + // SAFETY: The pointer is immutable after construction, and every access to + // any instance handle or process-global instrument-hooks state is serialized + // by `INSTRUMENT_HOOKS_LOCK`. unsafe impl Send for InstrumentHooks {} unsafe impl Sync for InstrumentHooks {} impl InstrumentHooks { #[inline(always)] pub fn new() -> Option { - let ptr = unsafe { ffi::instrument_hooks_init() }; + let ptr = with_transport_lock(|| unsafe { ffi::instrument_hooks_init() }); if ptr.is_null() { None } else { @@ -76,12 +88,13 @@ mod linux_impl { #[inline(always)] pub fn is_instrumented(&self) -> bool { - unsafe { ffi::instrument_hooks_is_instrumented(self.0) } + with_transport_lock(|| unsafe { ffi::instrument_hooks_is_instrumented(self.0) }) } #[inline(always)] pub fn start_benchmark(&self) -> Result<(), u8> { - let result = unsafe { ffi::instrument_hooks_start_benchmark(self.0) }; + let result = + with_transport_lock(|| unsafe { ffi::instrument_hooks_start_benchmark(self.0) }); if result == 0 { Ok(()) } else { @@ -91,7 +104,8 @@ mod linux_impl { #[inline(always)] pub fn stop_benchmark(&self) -> Result<(), u8> { - let result = unsafe { ffi::instrument_hooks_stop_benchmark(self.0) }; + let result = + with_transport_lock(|| unsafe { ffi::instrument_hooks_stop_benchmark(self.0) }); if result == 0 { Ok(()) } else { @@ -103,9 +117,9 @@ mod linux_impl { pub fn set_executed_benchmark(&self, uri: &str) -> Result<(), u8> { let pid = std::process::id() as i32; let c_uri = CString::new(uri).map_err(|_| 1u8)?; - let result = unsafe { + let result = with_transport_lock(|| unsafe { ffi::instrument_hooks_set_executed_benchmark(self.0, pid, c_uri.as_ptr()) - }; + }); if result == 0 { Ok(()) } else { @@ -117,9 +131,9 @@ mod linux_impl { pub fn set_integration(&self, name: &str, version: &str) -> Result<(), u8> { let c_name = CString::new(name).map_err(|_| 1u8)?; let c_version = CString::new(version).map_err(|_| 1u8)?; - let result = unsafe { + let result = with_transport_lock(|| unsafe { ffi::instrument_hooks_set_integration(self.0, c_name.as_ptr(), c_version.as_ptr()) - }; + }); if result == 0 { Ok(()) } else { @@ -131,22 +145,24 @@ mod linux_impl { pub fn add_benchmark_timestamps(&self, start: u64, end: u64) { let pid = std::process::id() as i32; - unsafe { - ffi::instrument_hooks_add_marker( - self.0, - pid, - ffi::MARKER_TYPE_BENCHMARK_START as u8, - start, - ) - }; - unsafe { - ffi::instrument_hooks_add_marker( - self.0, - pid, - ffi::MARKER_TYPE_BENCHMARK_END as u8, - end, - ) - }; + with_transport_lock(|| { + unsafe { + ffi::instrument_hooks_add_marker( + self.0, + pid, + ffi::MARKER_TYPE_BENCHMARK_START as u8, + start, + ) + }; + unsafe { + ffi::instrument_hooks_add_marker( + self.0, + pid, + ffi::MARKER_TYPE_BENCHMARK_END as u8, + end, + ) + }; + }); } #[inline(always)] @@ -175,14 +191,14 @@ mod linux_impl { let c_section = CString::new(section_name).map_err(|_| 1u8)?; let c_key = CString::new(key).map_err(|_| 1u8)?; let c_value = CString::new(value).map_err(|_| 1u8)?; - let result = unsafe { + let result = with_transport_lock(|| unsafe { ffi::instrument_hooks_set_environment( self.0, c_section.as_ptr(), c_key.as_ptr(), c_value.as_ptr(), ) - }; + }); if result == 0 { Ok(()) } else { @@ -192,7 +208,9 @@ mod linux_impl { pub fn write_environment(&self) -> Result<(), u8> { let pid = std::process::id() as i32; - let result = unsafe { ffi::instrument_hooks_write_environment(self.0, pid) }; + let result = with_transport_lock(|| unsafe { + ffi::instrument_hooks_write_environment(self.0, pid) + }); if result == 0 { Ok(()) } else { @@ -201,20 +219,22 @@ mod linux_impl { } pub fn disable_callgrind_markers() { - unsafe { + with_transport_lock(|| unsafe { ffi::instrument_hooks_set_feature( ffi::instrument_hooks_feature_t_FEATURE_DISABLE_CALLGRIND_MARKERS.into(), true, ) - }; + }); } } impl Drop for InstrumentHooks { fn drop(&mut self) { - if !self.0.is_null() { - unsafe { ffi::instrument_hooks_deinit(self.0) }; - } + with_transport_lock(|| { + if !self.0.is_null() { + unsafe { ffi::instrument_hooks_deinit(self.0) }; + } + }); } } } diff --git a/crates/divan_compat/benches/thread_example.rs b/crates/divan_compat/benches/thread_example.rs index 1695033b..dc0fbe28 100644 --- a/crates/divan_compat/benches/thread_example.rs +++ b/crates/divan_compat/benches/thread_example.rs @@ -28,6 +28,15 @@ fn fib_in_thread_bench_local(bencher: codspeed_divan_compat::Bencher, n: usize) }) } +#[cfg_attr( + not(codspeed), + codspeed_divan_compat::bench(threads = 4, sample_count = 1) +)] +#[cfg_attr(codspeed, codspeed_divan_compat::bench(sample_count = 1))] +fn divan_threads() { + codspeed_divan_compat::black_box(fibo(20)); +} + fn main() { codspeed_divan_compat::main(); } diff --git a/crates/divan_compat/divan_fork/src/bench/mod.rs b/crates/divan_compat/divan_fork/src/bench/mod.rs index 560fe5f8..5303ae25 100644 --- a/crates/divan_compat/divan_fork/src/bench/mod.rs +++ b/crates/divan_compat/divan_fork/src/bench/mod.rs @@ -698,10 +698,18 @@ impl<'a> BenchContext<'a> { }; // Sample loop: - let ([start, end], alloc_info) = + let ([start, end], alloc_info, [benchmark_start, benchmark_end]) = record_sample(sample_size as usize, barrier.as_ref(), &mut count_input); - RawSample { start, end, timer, alloc_info, counter_totals } + RawSample { + start, + end, + benchmark_start, + benchmark_end, + timer, + alloc_info, + counter_totals, + } }; // Sample loop: @@ -723,6 +731,11 @@ impl<'a> BenchContext<'a> { std::slice::from_raw_parts(raw_samples.as_ptr().cast(), raw_samples.len()) } }; + let benchmark_start = + raw_samples.iter().map(|sample| sample.benchmark_start).min().unwrap(); + let benchmark_end = + raw_samples.iter().map(|sample| sample.benchmark_end).max().unwrap(); + InstrumentHooks::instance().add_benchmark_timestamps(benchmark_start, benchmark_end); // If testing, exit the benchmarking loop immediately after timing a // single run. @@ -825,8 +838,11 @@ impl<'a> BenchContext<'a> { gen_input: impl Fn() -> I, benched: impl Fn(&UnsafeCell>) -> O, drop_input: impl Fn(&UnsafeCell>), - ) -> impl Fn(usize, Option<&Barrier>, &mut dyn FnMut(&I)) -> ([Timestamp; 2], ThreadAllocInfo) - { + ) -> impl Fn( + usize, + Option<&Barrier>, + &mut dyn FnMut(&I), + ) -> ([Timestamp; 2], ThreadAllocInfo, [u64; 2]) { // We defer: // - Usage of `gen_input` values. // - Drop destructor for `O`, preventing it from affecting sample @@ -897,8 +913,9 @@ impl<'a> BenchContext<'a> { // benchmarking. let sample_start: UntaggedTimestamp; let sample_end: UntaggedTimestamp; + let benchmark_start: u64; + let benchmark_end: u64; - let instrument_hooks = InstrumentHooks::instance(); if size_of::() == 0 && (size_of::() == 0 || !mem::needs_drop::()) { // Use a range instead of `defer_store` to make the benchmarking // loop cheaper. @@ -915,7 +932,7 @@ impl<'a> BenchContext<'a> { sync_threads(true); - let start_time = InstrumentHooks::current_timestamp(); + benchmark_start = InstrumentHooks::current_timestamp(); sample_start = UntaggedTimestamp::start(timer_kind); // Sample loop: @@ -928,8 +945,7 @@ impl<'a> BenchContext<'a> { } sample_end = UntaggedTimestamp::end(timer_kind); - let end_time = InstrumentHooks::current_timestamp(); - instrument_hooks.add_benchmark_timestamps(start_time, end_time); + benchmark_end = InstrumentHooks::current_timestamp(); sync_threads(false); save_alloc_info(); @@ -972,7 +988,7 @@ impl<'a> BenchContext<'a> { let defer_slots_iter = defer_slots_slice.iter(); sync_threads(true); - let start_time = InstrumentHooks::current_timestamp(); + benchmark_start = InstrumentHooks::current_timestamp(); sample_start = UntaggedTimestamp::start(timer_kind); // Sample loop: @@ -987,8 +1003,7 @@ impl<'a> BenchContext<'a> { } sample_end = UntaggedTimestamp::end(timer_kind); - let end_time = InstrumentHooks::current_timestamp(); - instrument_hooks.add_benchmark_timestamps(start_time, end_time); + benchmark_end = InstrumentHooks::current_timestamp(); sync_threads(false); save_alloc_info(); @@ -1028,7 +1043,7 @@ impl<'a> BenchContext<'a> { let defer_inputs_iter = defer_inputs_slice.iter(); sync_threads(true); - let start_time = InstrumentHooks::current_timestamp(); + benchmark_start = InstrumentHooks::current_timestamp(); sample_start = UntaggedTimestamp::start(timer_kind); // Sample loop: @@ -1039,8 +1054,7 @@ impl<'a> BenchContext<'a> { } sample_end = UntaggedTimestamp::end(timer_kind); - let end_time = InstrumentHooks::current_timestamp(); - instrument_hooks.add_benchmark_timestamps(start_time, end_time); + benchmark_end = InstrumentHooks::current_timestamp(); sync_threads(false); save_alloc_info(); @@ -1065,7 +1079,7 @@ impl<'a> BenchContext<'a> { [sample_start.into_timestamp(timer_kind), sample_end.into_timestamp(timer_kind)] }; - (interval, saved_alloc_info) + (interval, saved_alloc_info, [benchmark_start, benchmark_end]) } } diff --git a/crates/divan_compat/divan_fork/src/stats/sample.rs b/crates/divan_compat/divan_fork/src/stats/sample.rs index b1e1727d..f491a3c4 100644 --- a/crates/divan_compat/divan_fork/src/stats/sample.rs +++ b/crates/divan_compat/divan_fork/src/stats/sample.rs @@ -22,6 +22,8 @@ pub(crate) struct TimeSample { pub(crate) struct RawSample { pub start: Timestamp, pub end: Timestamp, + pub benchmark_start: u64, + pub benchmark_end: u64, pub timer: Timer, pub alloc_info: ThreadAllocInfo, pub counter_totals: [u128; KnownCounterKind::COUNT],