From 73c2d7b2e239961ceb873379439e86a8e36d01ae Mon Sep 17 00:00:00 2001 From: not-matthias Date: Wed, 9 Jul 2025 12:24:08 +0200 Subject: [PATCH] fix: execute pre- and post-bench scripts for non-perf walltime runner --- src/run/runner/wall_time/executor.rs | 118 +++++++++++++++++++++++++-- src/run/runner/wall_time/perf/mod.rs | 103 +---------------------- 2 files changed, 111 insertions(+), 110 deletions(-) diff --git a/src/run/runner/wall_time/executor.rs b/src/run/runner/wall_time/executor.rs index eea8ba18..ae43199b 100644 --- a/src/run/runner/wall_time/executor.rs +++ b/src/run/runner/wall_time/executor.rs @@ -11,9 +11,58 @@ use crate::run::{check_system::SystemInfo, config::Config}; use async_trait::async_trait; use std::fs::canonicalize; use std::io::Write; +use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::NamedTempFile; +struct HookScriptsGuard { + post_bench_script: PathBuf, +} + +impl HookScriptsGuard { + fn execute_script_from_path>(path: P) -> anyhow::Result<()> { + let path = path.as_ref(); + if !path.exists() || !path.is_file() { + debug!("Script not found or not a file: {}", path.display()); + return Ok(()); + } + + let output = Command::new("bash").args([&path]).output()?; + if !output.status.success() { + debug!("stdout: {}", String::from_utf8_lossy(&output.stdout)); + debug!("stderr: {}", String::from_utf8_lossy(&output.stderr)); + bail!("Failed to execute script: {}", path.display()); + } + + Ok(()) + } + + pub fn setup_with_scripts>(pre_bench_script: P, post_bench_script: P) -> Self { + if let Err(e) = Self::execute_script_from_path(pre_bench_script.as_ref()) { + debug!("Failed to execute pre-bench script: {e}"); + } + + Self { + post_bench_script: post_bench_script.as_ref().to_path_buf(), + } + } + + pub fn setup() -> Self { + Self::setup_with_scripts( + "/usr/local/bin/codspeed-pre-bench", + "/usr/local/bin/codspeed-post-bench", + ) + } +} + +impl Drop for HookScriptsGuard { + fn drop(&mut self) { + if let Err(e) = Self::execute_script_from_path(&self.post_bench_script) { + debug!("Failed to execute post-bench script: {e}"); + } + } +} + pub struct WallTimeExecutor { perf: Option, } @@ -86,17 +135,20 @@ impl Executor for WallTimeExecutor { cmd.current_dir(abs_cwd); } - let (_env_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?; + let status = { + let _guard = HookScriptsGuard::setup(); - let status = if let Some(perf) = &self.perf - && config.enable_perf - { - perf.run(cmd, &bench_cmd, config).await - } else { - cmd.args(["sh", "-c", &bench_cmd]); - debug!("cmd: {cmd:?}"); + let (_env_file, bench_cmd) = Self::walltime_bench_cmd(config, run_data)?; + if let Some(perf) = &self.perf + && config.enable_perf + { + perf.run(cmd, &bench_cmd, config).await + } else { + cmd.args(["sh", "-c", &bench_cmd]); + debug!("cmd: {cmd:?}"); - run_command_with_log_pipe(cmd).await + run_command_with_log_pipe(cmd).await + } }; let status = @@ -127,3 +179,51 @@ impl Executor for WallTimeExecutor { Ok(()) } } + +#[cfg(test)] +mod tests { + use tempfile::NamedTempFile; + + use super::*; + use std::{ + io::{Read, Write}, + os::unix::fs::PermissionsExt, + }; + + #[test] + fn test_env_guard_no_crash() { + fn create_run_script(content: &str) -> anyhow::Result { + let rwx = std::fs::Permissions::from_mode(0o777); + let mut script_file = tempfile::Builder::new() + .suffix(".sh") + .permissions(rwx) + .keep(true) + .tempfile()?; + script_file.write_all(content.as_bytes())?; + + Ok(script_file) + } + + let mut tmp_dst = tempfile::NamedTempFile::new().unwrap(); + + let pre_script = create_run_script(&format!( + "#!/usr/bin/env bash\necho \"pre\" >> {}", + tmp_dst.path().display() + )) + .unwrap(); + let post_script = create_run_script(&format!( + "#!/usr/bin/env bash\necho \"post\" >> {}", + tmp_dst.path().display() + )) + .unwrap(); + + { + let _guard = + HookScriptsGuard::setup_with_scripts(pre_script.path(), post_script.path()); + } + + let mut result = String::new(); + tmp_dst.read_to_string(&mut result).unwrap(); + assert_eq!(result, "pre\npost\n"); + } +} diff --git a/src/run/runner/wall_time/perf/mod.rs b/src/run/runner/wall_time/perf/mod.rs index 12c9133f..48e2de08 100644 --- a/src/run/runner/wall_time/perf/mod.rs +++ b/src/run/runner/wall_time/perf/mod.rs @@ -15,7 +15,7 @@ use metadata::PerfMetadata; use perf_map::ProcessSymbols; use shared::Command as FifoCommand; use std::collections::HashSet; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::process::Command; use std::time::Duration; use std::{cell::OnceCell, collections::HashMap, process::ExitStatus}; @@ -34,55 +34,6 @@ pub mod unwind_data; const PERF_DATA_PREFIX: &str = "perf.data."; -struct EnvGuard { - post_bench_script: PathBuf, -} - -impl EnvGuard { - fn execute_script_from_path>(path: P) -> anyhow::Result<()> { - let path = path.as_ref(); - if !path.exists() || !path.is_file() { - warn!("Script not found or not a file: {}", path.display()); - return Ok(()); - } - - let output = Command::new("bash").args([&path]).output()?; - if !output.status.success() { - info!("stdout: {}", String::from_utf8_lossy(&output.stdout)); - error!("stderr: {}", String::from_utf8_lossy(&output.stderr)); - bail!("Failed to execute script: {}", path.display()); - } - - Ok(()) - } - - pub fn setup_with_scripts>(pre_bench_script: P, post_bench_script: P) -> Self { - if let Err(e) = Self::execute_script_from_path(pre_bench_script.as_ref()) { - warn!("Failed to execute pre-bench script: {e}"); - println!("asdf: {e}"); - } - - Self { - post_bench_script: post_bench_script.as_ref().to_path_buf(), - } - } - - pub fn setup() -> Self { - Self::setup_with_scripts( - "/usr/local/bin/codspeed-pre-bench", - "/usr/local/bin/codspeed-post-bench", - ) - } -} - -impl Drop for EnvGuard { - fn drop(&mut self) { - if let Err(e) = Self::execute_script_from_path(&self.post_bench_script) { - warn!("Failed to execute post-bench script: {e}"); - } - } -} - pub struct PerfRunner { perf_dir: TempDir, benchmark_data: OnceCell, @@ -182,11 +133,7 @@ impl PerfRunner { Ok(()) }; - - { - let _guard = EnvGuard::setup(); - run_command_with_log_pipe_and_callback(cmd, on_process_started).await - } + run_command_with_log_pipe_and_callback(cmd, on_process_started).await } pub async fn save_files_to(&self, profile_folder: &PathBuf) -> anyhow::Result<()> { @@ -456,49 +403,3 @@ impl BenchmarkData { self.bench_order_by_pid.values().map(|v| v.len()).sum() } } -#[cfg(test)] -mod tests { - use tempfile::NamedTempFile; - - use super::*; - use std::{ - io::{Read, Write}, - os::unix::fs::PermissionsExt, - }; - - #[test] - fn test_env_guard_no_crash() { - fn create_run_script(content: &str) -> anyhow::Result { - let rwx = std::fs::Permissions::from_mode(0o777); - let mut script_file = tempfile::Builder::new() - .suffix(".sh") - .permissions(rwx) - .keep(true) - .tempfile()?; - script_file.write_all(content.as_bytes())?; - - Ok(script_file) - } - - let mut tmp_dst = tempfile::NamedTempFile::new().unwrap(); - - let pre_script = create_run_script(&format!( - "#!/usr/bin/env bash\necho \"pre\" >> {}", - tmp_dst.path().display() - )) - .unwrap(); - let post_script = create_run_script(&format!( - "#!/usr/bin/env bash\necho \"post\" >> {}", - tmp_dst.path().display() - )) - .unwrap(); - - { - let _guard = EnvGuard::setup_with_scripts(pre_script.path(), post_script.path()); - } - - let mut result = String::new(); - tmp_dst.read_to_string(&mut result).unwrap(); - assert_eq!(result, "pre\npost\n"); - } -}