diff --git a/src/run/runner/tests.rs b/src/run/runner/tests.rs index 5dd895bf..d2a9b64e 100644 --- a/src/run/runner/tests.rs +++ b/src/run/runner/tests.rs @@ -186,6 +186,8 @@ mod valgrind { } mod walltime { + use std::io::Read; + use super::*; async fn get_walltime_executor() -> (SemaphorePermit<'static>, WallTimeExecutor) { @@ -297,4 +299,47 @@ fi let result = executor.run(&config, &system_info, &run_data, &None).await; assert!(result.is_err(), "Command should fail"); } + + #[tokio::test] + async fn test_walltime_executor_works_with_go() { + let system_info = SystemInfo::new().unwrap(); + let profile_dir = TempDir::new().unwrap().into_path(); + let run_data = RunData { + profile_folder: profile_dir.clone(), + }; + + let (_permit, executor) = get_walltime_executor().await; + + // NOTE: Even though `go test` doesn't work because we don't have benchmarks it should still + // create a few perf events that are written to perf.pipedata. + //``` + // [DEBUG go.sh] Called with arguments: test -bench=. + // [DEBUG go.sh] Number of arguments: 2 + // [DEBUG go.sh] Detected 'test' command, routing to go-runner + // [DEBUG go.sh] Using go-runner at: /home/not-matthias/.cargo/bin/codspeed-go-runner + // [DEBUG go.sh] Full command: RUST_LOG=info /home/not-matthias/.cargo/bin/codspeed-go-runner test -bench=. + // Error: Failed to execute 'go list': [DEBUG go.sh] Called with arguments: list -test -compiled -json ./... + // [DEBUG go.sh] Number of arguments: 5 + // [DEBUG go.sh] Detected non-test command ('list'), routing to standard go binary + // [DEBUG go.sh] Full command: /nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/bin/go list -test -compiled -json ./... + // pattern ./...: directory prefix . does not contain main module or its selected dependencies + // + // [ perf record: Woken up 4 times to write data ] + // [ perf record: Captured and wrote 0.200 MB - ] + // ``` + let config = walltime_config("go test -bench=.", true); + + let _result = executor.run(&config, &system_info, &run_data, &None).await; + + let perf_runner = executor.perf_runner(); + let perf_data_path = perf_runner.perf_file().path(); + assert!(perf_data_path.exists(), "perf.pipedata should exist"); + + // Assert it starts with PERFILE2 + let mut file = std::fs::File::open(perf_data_path).unwrap(); + let expected = b"PERFILE2"; + let mut actual = [0u8; 8]; + file.read_exact(&mut actual).unwrap(); + assert_eq!(actual, *expected); + } } diff --git a/src/run/runner/wall_time/executor.rs b/src/run/runner/wall_time/executor.rs index f26334d0..20bb9c31 100644 --- a/src/run/runner/wall_time/executor.rs +++ b/src/run/runner/wall_time/executor.rs @@ -98,6 +98,11 @@ impl WallTimeExecutor { } } + #[cfg(test)] + pub fn perf_runner(&self) -> &PerfRunner { + self.perf.as_ref().expect("PerfRunner is not available") + } + fn walltime_bench_cmd( config: &Config, run_data: &RunData, @@ -191,8 +196,7 @@ impl Executor for WallTimeExecutor { if let Some(perf) = &self.perf && config.enable_perf { - perf.run(cmd_builder, config, &run_data.profile_folder) - .await + perf.run(cmd_builder, config).await } else { let cmd = wrap_with_sudo(cmd_builder)?.build(); debug!("cmd: {cmd:?}"); diff --git a/src/run/runner/wall_time/perf/mod.rs b/src/run/runner/wall_time/perf/mod.rs index 7e4c8220..69ccad1b 100644 --- a/src/run/runner/wall_time/perf/mod.rs +++ b/src/run/runner/wall_time/perf/mod.rs @@ -29,6 +29,7 @@ use std::collections::HashSet; use std::path::Path; use std::time::Duration; use std::{cell::OnceCell, collections::HashMap, process::ExitStatus}; +use tempfile::NamedTempFile; mod jit_dump; mod setup; @@ -45,9 +46,15 @@ const PERF_DATA_FILE_NAME: &str = "perf.pipedata"; pub struct PerfRunner { benchmark_data: OnceCell, + perf_file: NamedTempFile, } impl PerfRunner { + #[cfg(test)] + pub fn perf_file(&self) -> &NamedTempFile { + &self.perf_file + } + pub async fn setup_environment( system_info: &crate::run::check_system::SystemInfo, setup_cache_dir: Option<&Path>, @@ -82,6 +89,7 @@ impl PerfRunner { pub fn new() -> Self { Self { benchmark_data: OnceCell::new(), + perf_file: NamedTempFile::new().unwrap(), } } @@ -89,7 +97,6 @@ impl PerfRunner { &self, mut cmd_builder: CommandBuilder, config: &Config, - profile_folder: &Path, ) -> anyhow::Result { let perf_fifo = PerfFifo::new()?; let runner_fifo = RunnerFifo::new()?; @@ -146,8 +153,21 @@ impl PerfRunner { ]); cmd_builder.wrap_with(perf_wrapper_builder); - // Copy the perf data to the profile folder - let perf_data_file_path = profile_folder.join(PERF_DATA_FILE_NAME); + // Get the perf data file path from the stored tempfile + let perf_data_file_path = self.perf_file.path(); + + // Ensure we have the permissions to write to the tempfile when running with sudo + run_with_sudo( + "chown", + [ + &format!( + "{}:{}", + nix::unistd::Uid::current(), + nix::unistd::Gid::current() + ), + &perf_data_file_path.to_string_lossy().to_string(), + ], + )?; let raw_command = format!( "set -o pipefail && {} | cat > {}", @@ -178,6 +198,26 @@ impl PerfRunner { pub async fn save_files_to(&self, profile_folder: &Path) -> anyhow::Result<()> { let start = std::time::Instant::now(); + // We ran perf with sudo, so we have to change the ownership of the perf.data + run_with_sudo( + "chown", + [ + "-R", + &format!( + "{}:{}", + nix::unistd::Uid::current(), + nix::unistd::Gid::current() + ), + &self.perf_file.path().to_string_lossy(), + ], + )?; + + // Copy perf data from tempfile to profile folder + let dest_path = profile_folder.join(PERF_DATA_FILE_NAME); + std::fs::copy(&self.perf_file, &dest_path) + .context("Failed to copy perf.pipedata from tempfile to profile folder")?; + debug!("Copied perf.pipedata to {}", dest_path.display()); + let bench_data = self .benchmark_data .get()