Skip to content

Commit 5e04897

Browse files
committed
fix: create perf.pipedata in temporary file to avoid issues with golang
1 parent 2359e53 commit 5e04897

3 files changed

Lines changed: 67 additions & 5 deletions

File tree

src/run/runner/tests.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ mod valgrind {
186186
}
187187

188188
mod walltime {
189+
use std::io::Read;
190+
189191
use super::*;
190192

191193
async fn get_walltime_executor() -> (SemaphorePermit<'static>, WallTimeExecutor) {
@@ -297,4 +299,47 @@ fi
297299
let result = executor.run(&config, &system_info, &run_data, &None).await;
298300
assert!(result.is_err(), "Command should fail");
299301
}
302+
303+
#[tokio::test]
304+
async fn test_walltime_executor_works_with_go() {
305+
let system_info = SystemInfo::new().unwrap();
306+
let profile_dir = TempDir::new().unwrap().into_path();
307+
let run_data = RunData {
308+
profile_folder: profile_dir.clone(),
309+
};
310+
311+
let (_permit, executor) = get_walltime_executor().await;
312+
313+
// NOTE: Even though `go test` doesn't work because we don't have benchmarks it should still
314+
// create a few perf events that are written to perf.pipedata.
315+
//```
316+
// [DEBUG go.sh] Called with arguments: test -bench=.
317+
// [DEBUG go.sh] Number of arguments: 2
318+
// [DEBUG go.sh] Detected 'test' command, routing to go-runner
319+
// [DEBUG go.sh] Using go-runner at: /home/not-matthias/.cargo/bin/codspeed-go-runner
320+
// [DEBUG go.sh] Full command: RUST_LOG=info /home/not-matthias/.cargo/bin/codspeed-go-runner test -bench=.
321+
// Error: Failed to execute 'go list': [DEBUG go.sh] Called with arguments: list -test -compiled -json ./...
322+
// [DEBUG go.sh] Number of arguments: 5
323+
// [DEBUG go.sh] Detected non-test command ('list'), routing to standard go binary
324+
// [DEBUG go.sh] Full command: /nix/store/k1kn1c59ss7nq6agdppzq3krwmi3xqy4-go-1.25.2/bin/go list -test -compiled -json ./...
325+
// pattern ./...: directory prefix . does not contain main module or its selected dependencies
326+
//
327+
// [ perf record: Woken up 4 times to write data ]
328+
// [ perf record: Captured and wrote 0.200 MB - ]
329+
// ```
330+
let config = walltime_config("go test -bench=.", true);
331+
332+
let _result = executor.run(&config, &system_info, &run_data, &None).await;
333+
334+
let perf_runner = executor.perf_runner();
335+
let perf_data_path = perf_runner.perf_file().path();
336+
assert!(perf_data_path.exists(), "perf.pipedata should exist");
337+
338+
// Assert it starts with PERFPIPEPERFILE2
339+
let mut file = std::fs::File::open(perf_data_path).unwrap();
340+
let expected = b"PERFILE2";
341+
let mut actual = [0u8; 8];
342+
file.read_exact(&mut actual).unwrap();
343+
assert_eq!(actual, *expected);
344+
}
300345
}

src/run/runner/wall_time/executor.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ impl WallTimeExecutor {
9898
}
9999
}
100100

101+
#[cfg(test)]
102+
pub fn perf_runner(&self) -> &PerfRunner {
103+
self.perf.as_ref().expect("PerfRunner is not available")
104+
}
105+
101106
fn walltime_bench_cmd(
102107
config: &Config,
103108
run_data: &RunData,
@@ -191,8 +196,7 @@ impl Executor for WallTimeExecutor {
191196
if let Some(perf) = &self.perf
192197
&& config.enable_perf
193198
{
194-
perf.run(cmd_builder, config, &run_data.profile_folder)
195-
.await
199+
perf.run(cmd_builder, config).await
196200
} else {
197201
let cmd = wrap_with_sudo(cmd_builder)?.build();
198202
debug!("cmd: {cmd:?}");

src/run/runner/wall_time/perf/mod.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use std::collections::HashSet;
2929
use std::path::Path;
3030
use std::time::Duration;
3131
use std::{cell::OnceCell, collections::HashMap, process::ExitStatus};
32+
use tempfile::NamedTempFile;
3233

3334
mod jit_dump;
3435
mod setup;
@@ -45,9 +46,15 @@ const PERF_DATA_FILE_NAME: &str = "perf.pipedata";
4546

4647
pub struct PerfRunner {
4748
benchmark_data: OnceCell<BenchmarkData>,
49+
perf_file: NamedTempFile,
4850
}
4951

5052
impl PerfRunner {
53+
#[cfg(test)]
54+
pub fn perf_file(&self) -> &NamedTempFile {
55+
&self.perf_file
56+
}
57+
5158
pub async fn setup_environment(
5259
system_info: &crate::run::check_system::SystemInfo,
5360
setup_cache_dir: Option<&Path>,
@@ -82,14 +89,14 @@ impl PerfRunner {
8289
pub fn new() -> Self {
8390
Self {
8491
benchmark_data: OnceCell::new(),
92+
perf_file: NamedTempFile::new().unwrap(),
8593
}
8694
}
8795

8896
pub async fn run(
8997
&self,
9098
mut cmd_builder: CommandBuilder,
9199
config: &Config,
92-
profile_folder: &Path,
93100
) -> anyhow::Result<ExitStatus> {
94101
let perf_fifo = PerfFifo::new()?;
95102
let runner_fifo = RunnerFifo::new()?;
@@ -146,8 +153,8 @@ impl PerfRunner {
146153
]);
147154
cmd_builder.wrap_with(perf_wrapper_builder);
148155

149-
// Copy the perf data to the profile folder
150-
let perf_data_file_path = profile_folder.join(PERF_DATA_FILE_NAME);
156+
// Get the perf data file path from the stored tempfile
157+
let perf_data_file_path = self.perf_file.path();
151158

152159
let raw_command = format!(
153160
"set -o pipefail && {} | cat > {}",
@@ -178,6 +185,12 @@ impl PerfRunner {
178185
pub async fn save_files_to(&self, profile_folder: &Path) -> anyhow::Result<()> {
179186
let start = std::time::Instant::now();
180187

188+
// Copy perf data from tempfile to profile folder
189+
let dest_path = profile_folder.join(PERF_DATA_FILE_NAME);
190+
std::fs::copy(&self.perf_file, &dest_path)
191+
.context("Failed to copy perf.pipedata from tempfile to profile folder")?;
192+
debug!("Copied perf.pipedata to {}", dest_path.display());
193+
181194
let bench_data = self
182195
.benchmark_data
183196
.get()

0 commit comments

Comments
 (0)