From 17c9f5ec5b8ddd3586e014c4c13fce99ccebe83a Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 20 Jun 2025 20:17:02 +0200 Subject: [PATCH 1/3] feat: add pre- and post-benchmark scripts --- src/run/runner/wall_time/perf/mod.rs | 131 ++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) diff --git a/src/run/runner/wall_time/perf/mod.rs b/src/run/runner/wall_time/perf/mod.rs index d52dd8bf..d15efeb6 100644 --- a/src/run/runner/wall_time/perf/mod.rs +++ b/src/run/runner/wall_time/perf/mod.rs @@ -31,6 +31,52 @@ pub mod unwind_data; const PERF_DATA_PREFIX: &str = "perf.data."; +struct EnvGuard; + +impl EnvGuard { + fn execute_script_from_env(script_env_var: &str) -> anyhow::Result<()> { + let Ok(script_path) = std::env::var(script_env_var) else { + debug!("Couldn't find {script_env_var}, skipping script execution"); + return Ok(()); + }; + + if script_path.is_empty() { + return Ok(()); + } + + let path = std::path::Path::new(&script_path); + if !path.exists() || !path.is_file() { + warn!("Script not found or not a file: {}", script_path); + return Ok(()); + } + + let output = Command::new("bash").args([&script_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: {}", script_path); + } + + Ok(()) + } + + pub fn setup() -> Self { + if let Err(e) = Self::execute_script_from_env("CODSPEED_PRE_STARTUP_SCRIPT") { + warn!("Failed to execute pre-startup script: {}", e); + println!("asdf: {e}"); + } + Self + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + if let Err(e) = Self::execute_script_from_env("CODSPEED_POST_CLEANUP_SCRIPT") { + warn!("Failed to execute post-cleanup script: {}", e); + } + } +} + pub struct PerfRunner { perf_dir: TempDir, benchmark_data: OnceCell, @@ -126,7 +172,11 @@ impl PerfRunner { Ok(()) }; - run_command_with_log_pipe_and_callback(cmd, on_process_started).await + + { + let _guard = EnvGuard::setup(); + run_command_with_log_pipe_and_callback(cmd, on_process_started).await + } } pub async fn save_files_to(&self, profile_folder: &PathBuf) -> anyhow::Result<()> { @@ -389,3 +439,82 @@ 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, + }; + + fn with_env(vars: &[(&str, &str)], mut f: F) + where + F: FnMut(), + { + let original_vars: Vec<(&str, std::result::Result)> = + vars.iter().map(|(k, _)| (*k, std::env::var(*k))).collect(); + + for (k, v) in vars { + std::env::set_var(k, v); + } + + f(); + + for (k, v) in original_vars { + if let Ok(val) = v { + std::env::set_var(k, val); + } else { + std::env::remove_var(k); + } + } + } + + #[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 env_vars = [ + ( + "CODSPEED_PRE_STARTUP_SCRIPT", + &*pre_script.path().to_string_lossy(), + ), + ( + "CODSPEED_POST_CLEANUP_SCRIPT", + &post_script.path().to_string_lossy(), + ), + ]; + + with_env(&env_vars, || { + let _guard = EnvGuard::setup(); + }); + + let mut result = String::new(); + tmp_dst.read_to_string(&mut result).unwrap(); + assert_eq!(result, "pre\npost\n"); + } +} From 674015e28f2dd3a22612f84689f6ffe616a30373 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Fri, 20 Jun 2025 20:17:56 +0200 Subject: [PATCH 2/3] fix: symlink libpython doesn't work for statically linked python --- Cargo.lock | 36 +++++++++++++++++++ Cargo.toml | 2 +- .../runner/valgrind/helpers/venv_compat.rs | 2 ++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 63e434e6..7f797444 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -312,6 +312,7 @@ dependencies = [ "sysinfo", "temp-env", "tempfile", + "test-with", "tokio", "tokio-tar", "url", @@ -1540,6 +1541,28 @@ dependencies = [ "version_check", ] +[[package]] +name = "proc-macro-error-attr2" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96de42df36bb9bba5542fe9f1a054b8cc87e172759a1868aa05c1f3acc89dfc5" +dependencies = [ + "proc-macro2", + "quote", +] + +[[package]] +name = "proc-macro-error2" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11ec05c52be0a07b08061f7dd003e7d7092e0472bc731b4af7bb1ef876109802" +dependencies = [ + "proc-macro-error-attr2", + "proc-macro2", + "quote", + "syn 2.0.96", +] + [[package]] name = "proc-macro2" version = "1.0.93" @@ -2154,6 +2177,19 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "test-with" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bb2a43a245ab793321d71ff9f2be21785999c9aa5b2ea93dc507e97572be843" +dependencies = [ + "proc-macro-error2", + "proc-macro2", + "quote", + "regex", + "syn 2.0.96", +] + [[package]] name = "thiserror" version = "1.0.69" diff --git a/Cargo.toml b/Cargo.toml index 92ff82a5..ebd17773 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,7 +61,7 @@ procfs = "0.17.0" [dev-dependencies] temp-env = { version = "0.3.6", features = ["async_closure"] } insta = { version = "1.29.0", features = ["json", "redactions"] } - +test-with = { version = "0.15", default-features = false, features = [] } [workspace.metadata.release] sign-tag = true diff --git a/src/run/runner/valgrind/helpers/venv_compat.rs b/src/run/runner/valgrind/helpers/venv_compat.rs index c5d696ee..c0f9ba8d 100644 --- a/src/run/runner/valgrind/helpers/venv_compat.rs +++ b/src/run/runner/valgrind/helpers/venv_compat.rs @@ -51,6 +51,8 @@ pub fn symlink_libpython(cwd: Option<&String>) -> anyhow::Result<()> { mod tests { use super::*; + // Only run in Github Actions, to ensure python is dynamically linked. + #[test_with::env(GITHUB_ACTIONS)] #[test] fn test_venv_compat_no_crash() { assert!(symlink_libpython(None).is_ok()); From fa915805929acd02502ace3c2e10eb9e388e2776 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Mon, 30 Jun 2025 11:23:56 +0200 Subject: [PATCH 3/3] fix: use fixed script path --- src/run/runner/wall_time/perf/mod.rs | 86 +++++++++------------------- 1 file changed, 28 insertions(+), 58 deletions(-) diff --git a/src/run/runner/wall_time/perf/mod.rs b/src/run/runner/wall_time/perf/mod.rs index d15efeb6..ce2f3568 100644 --- a/src/run/runner/wall_time/perf/mod.rs +++ b/src/run/runner/wall_time/perf/mod.rs @@ -12,7 +12,7 @@ use metadata::PerfMetadata; use perf_map::ProcessSymbols; use shared::Command as FifoCommand; use std::collections::HashSet; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; use std::time::Duration; use std::{cell::OnceCell, collections::HashMap, process::ExitStatus}; @@ -31,48 +31,51 @@ pub mod unwind_data; const PERF_DATA_PREFIX: &str = "perf.data."; -struct EnvGuard; +struct EnvGuard { + post_bench_script: PathBuf, +} impl EnvGuard { - fn execute_script_from_env(script_env_var: &str) -> anyhow::Result<()> { - let Ok(script_path) = std::env::var(script_env_var) else { - debug!("Couldn't find {script_env_var}, skipping script execution"); - return Ok(()); - }; - - if script_path.is_empty() { - return Ok(()); - } - - let path = std::path::Path::new(&script_path); + 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: {}", script_path); + warn!("Script not found or not a file: {}", path.display()); return Ok(()); } - let output = Command::new("bash").args([&script_path]).output()?; + 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: {}", script_path); + bail!("Failed to execute script: {}", path.display()); } Ok(()) } - pub fn setup() -> Self { - if let Err(e) = Self::execute_script_from_env("CODSPEED_PRE_STARTUP_SCRIPT") { - warn!("Failed to execute pre-startup script: {}", e); + 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 + + 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_env("CODSPEED_POST_CLEANUP_SCRIPT") { - warn!("Failed to execute post-cleanup script: {}", e); + if let Err(e) = Self::execute_script_from_path(&self.post_bench_script) { + warn!("Failed to execute post-bench script: {}", e); } } } @@ -449,28 +452,6 @@ mod tests { os::unix::fs::PermissionsExt, }; - fn with_env(vars: &[(&str, &str)], mut f: F) - where - F: FnMut(), - { - let original_vars: Vec<(&str, std::result::Result)> = - vars.iter().map(|(k, _)| (*k, std::env::var(*k))).collect(); - - for (k, v) in vars { - std::env::set_var(k, v); - } - - f(); - - for (k, v) in original_vars { - if let Ok(val) = v { - std::env::set_var(k, val); - } else { - std::env::remove_var(k); - } - } - } - #[test] fn test_env_guard_no_crash() { fn create_run_script(content: &str) -> anyhow::Result { @@ -498,20 +479,9 @@ mod tests { )) .unwrap(); - let env_vars = [ - ( - "CODSPEED_PRE_STARTUP_SCRIPT", - &*pre_script.path().to_string_lossy(), - ), - ( - "CODSPEED_POST_CLEANUP_SCRIPT", - &post_script.path().to_string_lossy(), - ), - ]; - - with_env(&env_vars, || { - let _guard = EnvGuard::setup(); - }); + { + 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();