From 04ee01d1922e44f8dcb4cd199f262089e55bfe92 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:10:12 +0100 Subject: [PATCH 1/7] build(memtrack): support building against musl musl has no `close_range` wrapper, so call the syscall directly. libbpf-sys vendors elfutils, whose `configure` requires `argp`, `obstack` and `fts`, none of which musl provides even though a libelf-only build never calls them: `.cargo/config.toml` seeds the autoconf cache to skip those checks, adds an `argp.h` stub through `CPATH`, and points Debian's `musl-gcc` back at the kernel UAPI headers. aarch64 also links libgcc for the outline-atomic helpers. The libc symbol resolution test now reads the Ubuntu libc path and only runs in CI, since a static musl test binary maps no libc of its own. Refs COD-3440 Co-Authored-By: Claude --- .cargo/config.toml | 29 ++++++++++ crates/memtrack/musl/argp.h | 56 +++++++++++++++++++ .../memtrack/src/ebpf/memtrack/fd_holder.rs | 4 +- crates/memtrack/src/ebpf/memtrack/mod.rs | 13 ++--- 4 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 crates/memtrack/musl/argp.h diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..95d67cab3 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,29 @@ +# Nothing here is `force = true`, so a shell exporting `CFLAGS` or `CPATH` wins +# and breaks the musl build. +# +# `libbpf-sys` vendors elfutils, whose `configure` looks for `argp`, `obstack` +# and `fts`; musl has none of them, so it stops before libelf even though a +# libelf-only build never calls them. Pre-seeding the cache skips the checks. +# Only autoconf reads these names, so a gnu build is unaffected. +# Upstream tracks this at https://github.com/libbpf/libbpf-sys/issues/153. +[env] +ac_cv_search_argp_parse = "none required" +ac_cv_search__obstack_free = "none required" +ac_cv_search_fts_close = "none required" + +# The `argp.h` stub. `CPATH` and not a `-I` in the flags below, because +# `relative = true` only works on a bare path. It is therefore global to the +# build, so the stub defers to the real header wherever one exists. +CPATH = { value = "crates/memtrack/musl", relative = true } + +# Debian's musl-gcc runs with -nostdinc, so the kernel UAPI headers libbpf needs +# have to be added back, last so they never shadow the toolchain's own. Scoped +# per target: cc-rs looks up `CFLAGS_` before `CFLAGS`, and libbpf-sys +# builds zlib, elfutils and libbpf from `cc::Tool::cflags_env()`. +CFLAGS_x86_64_unknown_linux_musl = "-idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include" +CFLAGS_aarch64_unknown_linux_musl = "-idirafter /usr/include/aarch64-linux-gnu -idirafter /usr/include" + +# rustc links with `-nodefaultlibs`; libbpf's C code needs the outline-atomic +# helpers from libgcc on aarch64. +[target.aarch64-unknown-linux-musl] +rustflags = ["-C", "link-arg=-lgcc"] diff --git a/crates/memtrack/musl/argp.h b/crates/memtrack/musl/argp.h new file mode 100644 index 000000000..7fa6f51ab --- /dev/null +++ b/crates/memtrack/musl/argp.h @@ -0,0 +1,56 @@ +/* crates/memtrack/musl/argp.h — stub for musl builds of libbpf-sys' vendored elfutils. + Declarations only: a libelf-only build never calls into argp, but the + elfutils sources still `#include `, which musl does not ship. + If compilation complains about a missing type or macro, add it here. + + This directory is on `CPATH` for every build, gnu included, so the header + defers to a real whenever one exists. The test is `__GLIBC__` and not + `__has_include_next`: a musl build also gets `-idirafter /usr/include`, which + puts glibc's argp.h in reach, and including it would die on `__THROW`. + is only there to pull in . */ +#include +#if defined(__GLIBC__) +#include_next +#else + +#ifndef CODSPEED_STUB_ARGP_H +#define CODSPEED_STUB_ARGP_H + +#include + +typedef int error_t; + +struct argp_option { + const char *name; + int key; + const char *arg; + int flags; + const char *doc; + int group; +}; + +struct argp_state { + const char *name; +}; + +typedef error_t (*argp_parser_t)(int key, char *arg, struct argp_state *state); + +struct argp { + const struct argp_option *options; + argp_parser_t parser; + const char *args_doc; + const char *doc; + const void *children; + void *help_filter; + const char *argp_domain; +}; + +#define OPTION_ARG_OPTIONAL 0x1 +#define ARGP_HELP_SEE 0x40 +#define ARGP_ERR_UNKNOWN 1 + +int argp_help(const struct argp *argp, FILE *stream, unsigned int flags, char *name); + +#endif /* CODSPEED_STUB_ARGP_H */ + +#endif /* __GLIBC__ */ diff --git a/crates/memtrack/src/ebpf/memtrack/fd_holder.rs b/crates/memtrack/src/ebpf/memtrack/fd_holder.rs index cd594a581..77d0cdea0 100644 --- a/crates/memtrack/src/ebpf/memtrack/fd_holder.rs +++ b/crates/memtrack/src/ebpf/memtrack/fd_holder.rs @@ -32,14 +32,14 @@ unsafe fn close_fds_except(keep: &[RawFd]) { if fd > lo { // SAFETY: caller upholds the fork-child, no-allocation contract. unsafe { - libc::close_range(lo, fd - 1, 0); + libc::syscall(libc::SYS_close_range, lo, fd - 1, 0); } } lo = fd.saturating_add(1); } // SAFETY: same as above. unsafe { - libc::close_range(lo, u32::MAX, 0); + libc::syscall(libc::SYS_close_range, lo, u32::MAX, 0); } } diff --git a/crates/memtrack/src/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index dbe4680ad..df7b19460 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -348,16 +348,13 @@ mod tests { /// Allocator entry points must resolve to file offsets; a symbol that /// silently fails to resolve attaches nothing and loses all events. + /// + /// CI-only: the path is the Ubuntu one, and a static musl build of this + /// binary has no libc of its own to look at. + #[test_with::env(GITHUB_ACTIONS)] #[test] fn libc_allocator_symbols_resolve_to_offsets() { - let maps = std::fs::read_to_string("/proc/self/maps").unwrap(); - let libc_path = maps - .lines() - .find_map(|line| { - let path = line.split_whitespace().last()?; - path.contains("libc.so.6").then(|| path.to_owned()) - }) - .expect("test process has no mapped libc.so.6"); + let libc_path = format!("/lib/{}-linux-gnu/libc.so.6", std::env::consts::ARCH); let symbols = resolve_symbol_offsets(Path::new(&libc_path)).unwrap(); for symbol in ["malloc", "calloc", "realloc", "free"] { From 81cedb9b8b5501d09eac814ba163663cbbda4545 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:10:31 +0100 Subject: [PATCH 2/7] refactor(memtrack, exec-harness): move the CLI into the library Move each binary's logger setup, argument parsing and dispatch into a `cli::run_cli` library function that takes its argv, leaving `main` a thin wrapper. This lets the runner call either CLI in-process with the same logging. No behavior change. Refs COD-3440 Co-Authored-By: Claude --- crates/exec-harness/src/cli.rs | 59 +++++++++ crates/exec-harness/src/lib.rs | 1 + crates/exec-harness/src/main.rs | 58 +-------- crates/memtrack/src/cli.rs | 216 ++++++++++++++++++++++++++++++++ crates/memtrack/src/lib.rs | 2 + crates/memtrack/src/main.rs | 210 +------------------------------ 6 files changed, 283 insertions(+), 263 deletions(-) create mode 100644 crates/exec-harness/src/cli.rs create mode 100644 crates/memtrack/src/cli.rs diff --git a/crates/exec-harness/src/cli.rs b/crates/exec-harness/src/cli.rs new file mode 100644 index 000000000..869ec5364 --- /dev/null +++ b/crates/exec-harness/src/cli.rs @@ -0,0 +1,59 @@ +use crate::prelude::*; +use crate::walltime::WalltimeExecutionArgs; +use crate::{BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin}; +use clap::Parser; +use std::ffi::OsString; + +#[derive(Parser, Debug)] +#[command(name = "exec-harness")] +#[command(about = "CodSpeed exec harness - wraps commands with performance instrumentation")] +struct Args { + /// Optional benchmark name, else the command will be used as the name + #[arg(long)] + name: Option, + + /// Set by the runner, should be coherent with the executor being used + #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] + measurement_mode: Option, + + #[command(flatten)] + walltime_args: WalltimeExecutionArgs, + + /// The command and arguments to execute. + /// Use "-" as the only argument to read a JSON payload from stdin. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, +} + +pub fn run_cli(argv: I) -> Result<()> +where + I: IntoIterator, + T: Into + Clone, +{ + env_logger::builder() + .parse_env(env_logger::Env::new().filter_or("CODSPEED_LOG", "info")) + .format(|buf, record| { + use std::io::Write; + writeln!(buf, "{}", record.args()) + }) + .init(); + + debug!("Starting exec-harness with pid {}", std::process::id()); + + let args = Args::parse_from(argv); + let measurement_mode = args.measurement_mode; + + let commands = match args.command.as_slice() { + [single] if single == "-" => read_commands_from_stdin()?, + [] => bail!("No command provided"), + _ => vec![BenchmarkCommand { + command: args.command, + name: args.name, + walltime_args: args.walltime_args, + }], + }; + + execute_benchmarks(commands, measurement_mode)?; + + Ok(()) +} diff --git a/crates/exec-harness/src/lib.rs b/crates/exec-harness/src/lib.rs index 48410a54b..608219583 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize}; use std::io::{self, BufRead}; pub mod analysis; +pub mod cli; pub mod constants; pub mod prelude; pub mod runtime_env; diff --git a/crates/exec-harness/src/main.rs b/crates/exec-harness/src/main.rs index 99cbf7cd2..4f027540f 100644 --- a/crates/exec-harness/src/main.rs +++ b/crates/exec-harness/src/main.rs @@ -1,60 +1,6 @@ -use clap::Parser; +use exec_harness::cli::run_cli; use exec_harness::prelude::*; -use exec_harness::walltime::WalltimeExecutionArgs; -use exec_harness::{ - BenchmarkCommand, MeasurementMode, execute_benchmarks, read_commands_from_stdin, -}; - -#[derive(Parser, Debug)] -#[command(name = "exec-harness")] -#[command( - version, - about = "CodSpeed exec harness - wraps commands with performance instrumentation" -)] -struct Args { - /// Optional benchmark name, else the command will be used as the name - #[arg(long)] - name: Option, - - /// Set by the runner, should be coherent with the executor being used - #[arg(short, long, global = true, env = "CODSPEED_RUNNER_MODE", hide = true)] - measurement_mode: Option, - - #[command(flatten)] - walltime_args: WalltimeExecutionArgs, - - /// The command and arguments to execute. - /// Use "-" as the only argument to read a JSON payload from stdin. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - command: Vec, -} fn main() -> Result<()> { - env_logger::builder() - .parse_env(env_logger::Env::new().filter_or("CODSPEED_LOG", "info")) - .format(|buf, record| { - use std::io::Write; - writeln!(buf, "{}", record.args()) - }) - .init(); - - debug!("Starting exec-harness with pid {}", std::process::id()); - - let args = Args::parse(); - let measurement_mode = args.measurement_mode; - - // Determine if we're in stdin mode or CLI mode - let commands = match args.command.as_slice() { - [single] if single == "-" => read_commands_from_stdin()?, - [] => bail!("No command provided"), - _ => vec![BenchmarkCommand { - command: args.command, - name: args.name, - walltime_args: args.walltime_args, - }], - }; - - execute_benchmarks(commands, measurement_mode)?; - - Ok(()) + run_cli(std::env::args_os()) } diff --git a/crates/memtrack/src/cli.rs b/crates/memtrack/src/cli.rs new file mode 100644 index 000000000..7963efec9 --- /dev/null +++ b/crates/memtrack/src/cli.rs @@ -0,0 +1,216 @@ +use crate::prelude::*; +use crate::{MemtrackIpcMessage, Tracker, handle_ipc_message}; +use clap::Parser; +use ipc_channel::ipc; +use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Arc; +use std::thread; + +#[derive(Parser)] +#[command(name = "memtrack")] +#[command(about = "Track memory allocations using eBPF", long_about = None)] +struct Cli { + #[command(subcommand)] + command: Commands, +} + +#[derive(Parser)] +enum Commands { + /// Track memory allocations for a command + Track { + /// Command to execute and track + command: String, + + /// Output folder for the allocations data + #[arg(short, long, default_value = ".")] + output: PathBuf, + + /// Optional IPC server name for receiving control commands + #[arg(long)] + ipc_server: Option, + }, +} + +pub fn run_cli(argv: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + env_logger::builder() + .parse_env(env_logger::Env::new().filter_or("CODSPEED_LOG", "info")) + .format_timestamp(None) + .init(); + + let cli = Cli::parse_from(argv); + + match cli.command { + Commands::Track { + command, + output: out_dir, + ipc_server, + } => { + debug!("Starting memtrack for command: {command}"); + + let status = + track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; + + Ok(status.code().unwrap_or(1)) + } + } +} + +/// Get the original user's UID and GID when running under sudo. +/// Returns None if not running under sudo or if the environment variables are not set. +fn get_user_uid_gid() -> Option<(u32, u32)> { + let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; + let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; + Some((uid, gid)) +} + +fn track_command( + cmd_string: &str, + ipc_server_name: Option, + out_dir: &Path, +) -> anyhow::Result { + // First, establish IPC connection if needed to avoid timeouts on the runner because + // creating the Tracker instance takes some time. + let ipc_channel = if let Some(server_name) = ipc_server_name { + debug!("Connecting to IPC server: {server_name}"); + + let (tx, rx) = ipc::channel::()?; + let sender = ipc::IpcSender::connect(server_name)?; + sender.send(tx)?; + + Some(rx) + } else { + None + }; + + let tracker = Arc::new(Tracker::new()?); + + // Spawn IPC handler thread with the now-available tracker + let ipc_handle = if let Some(rx) = ipc_channel { + let tracker = tracker.clone(); + Some(thread::spawn(move || { + while let Ok(msg) = rx.recv() { + handle_ipc_message(msg, &tracker); + } + })) + } else { + // Without IPC, nothing toggles the tracking_enabled map, so allocator + // events would be dropped by the eBPF is_enabled() check. Enable it up + // front. + tracker.enable_tracking()?; + None + }; + + // Run the target command through bash to handle shell syntax. Drop + // privileges if running under sudo to avoid permission issues when the + // target accesses files owned by the original user. + let mut cmd = Command::new("bash"); + cmd.arg("-c").arg(cmd_string); + let uid_gid = get_user_uid_gid(); + if let Some((uid, gid)) = uid_gid { + debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); + } + + let mut session = tracker + .spawn(&cmd, uid_gid) + .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; + let root_pid = session.pid(); + let event_rx = session.take_events()?; + debug!("Spawned child with pid {root_pid}"); + + // Generate output file name and create file for streaming events + let file_name = MemtrackArtifact::file_name(Some(root_pid)); + let out_file = std::fs::File::create(out_dir.join(file_name))?; + + // Leave headroom for the ring buffer poll thread and the tracked + // command: encode workers on every core starve the poller during + // allocation bursts, which overflows the kernel ring buffer. + let n_workers = thread::available_parallelism() + .map(|n| n.get().saturating_sub(2).max(1)) + .unwrap_or(4); + + let pipeline_thread = + thread::spawn(move || encode_events(event_rx.into_iter().flatten(), out_file, n_workers)); + + // A worker failure must not skip disabling tracking, draining, joining the + // encoder, or detaching probes. Keep the wait result until teardown is done. + let status = session.wait().context("Failed to wait for command"); + if let Ok(status) = &status { + debug!("Command exited with status: {status}"); + } + + // Stop allocator-event production before draining: the child has exited, + // so anything still arriving is already in the ring buffer. + if let Err(e) = tracker.disable_tracking() { + warn!("Failed to disable tracking: {e:#}"); + } + + // Dropping the session drops the event poller, which does a final drain of + // the ring buffer and then closes the event channel. Without this the + // encode pipeline join below would block forever. + debug!("Stopping the ring buffer poller"); + drop(session); + + debug!("Waiting for the encode pipeline to finish"); + let total = pipeline_thread + .join() + .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline")) + .and_then(|result| result); + + if let Ok(total) = &total { + info!("Wrote {total} memtrack events to disk"); + } + + // Stop background workers after the ring pipeline has drained. Fatal + // worker errors mean the capture is incomplete. + let finish = tracker.finish(); + let stack_stats = if tracker.stack_capture_enabled() { + Some( + tracker + .stack_capture_stats() + .context("Failed to read stack capture stats"), + ) + } else { + None + }; + if let Some(Ok(stats)) = &stack_stats { + debug!("stack capture stats: {stats:?}"); + } + + // Detach probes explicitly: the IPC thread still holds an Arc clone, so the + // tracker would otherwise never be dropped before process::exit and the + // kernel would close every link fd serially during exit. + tracker.detach(); + + let status = status?; + total?; + finish?; + if let Some(stats) = stack_stats { + stats?; + } + + // Read the eBPF dropped-event counter after the run is complete. + // A non-zero value means the ring buffer overflowed and the trace is + // incomplete. + let dropped_events = tracker + .dropped_events_count() + .context("Failed to read memtrack dropped-event counter")?; + if dropped_events > 0 { + bail!( + "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ + Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ + or report it at https://github.com/CodSpeedHQ/codspeed/issues." + ); + } + + // IPC thread will exit when channel closes + drop(ipc_handle); + + Ok(status) +} diff --git a/crates/memtrack/src/lib.rs b/crates/memtrack/src/lib.rs index 30cfcc99c..17cd9aff9 100644 --- a/crates/memtrack/src/lib.rs +++ b/crates/memtrack/src/lib.rs @@ -1,6 +1,8 @@ mod allocators; mod bpf_token; #[cfg(feature = "ebpf")] +pub mod cli; +#[cfg(feature = "ebpf")] mod ebpf; mod ipc; mod kernel; diff --git a/crates/memtrack/src/main.rs b/crates/memtrack/src/main.rs index 5ff58cb41..020e5047c 100644 --- a/crates/memtrack/src/main.rs +++ b/crates/memtrack/src/main.rs @@ -1,214 +1,10 @@ #[global_allocator] static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; -use clap::Parser; -use ipc_channel::ipc; +use memtrack::cli::run_cli; use memtrack::prelude::*; -use memtrack::{MemtrackIpcMessage, Tracker, handle_ipc_message}; -use runner_shared::artifacts::{ArtifactExt, MemtrackArtifact, encode_events}; -use std::path::{Path, PathBuf}; -use std::process::Command; -use std::sync::Arc; -use std::thread; - -#[derive(Parser)] -#[command(name = "memtrack")] -#[command(version, about = "Track memory allocations using eBPF", long_about = None)] -struct Cli { - #[command(subcommand)] - command: Commands, -} - -#[derive(Parser)] -enum Commands { - /// Track memory allocations for a command - Track { - /// Command to execute and track - command: String, - - /// Output folder for the allocations data - #[arg(short, long, default_value = ".")] - output: PathBuf, - - /// Optional IPC server name for receiving control commands - #[arg(long)] - ipc_server: Option, - }, -} - -/// Get the original user's UID and GID when running under sudo. -/// Returns None if not running under sudo or if the environment variables are not set. -fn get_user_uid_gid() -> Option<(u32, u32)> { - let uid = std::env::var("SUDO_UID").ok()?.parse().ok()?; - let gid = std::env::var("SUDO_GID").ok()?.parse().ok()?; - Some((uid, gid)) -} fn main() -> Result<()> { - env_logger::builder() - .parse_env(env_logger::Env::new().filter_or("CODSPEED_LOG", "info")) - .format_timestamp(None) - .init(); - - let cli = Cli::parse(); - - match cli.command { - Commands::Track { - command, - output: out_dir, - ipc_server, - } => { - debug!("Starting memtrack for command: {command}"); - - let status = - track_command(&command, ipc_server, &out_dir).context("Failed to track command")?; - - std::process::exit(status.code().unwrap_or(1)); - } - } -} - -fn track_command( - cmd_string: &str, - ipc_server_name: Option, - out_dir: &Path, -) -> anyhow::Result { - // First, establish IPC connection if needed to avoid timeouts on the runner because - // creating the Tracker instance takes some time. - let ipc_channel = if let Some(server_name) = ipc_server_name { - debug!("Connecting to IPC server: {server_name}"); - - let (tx, rx) = ipc::channel::()?; - let sender = ipc::IpcSender::connect(server_name)?; - sender.send(tx)?; - - Some(rx) - } else { - None - }; - - let tracker = Arc::new(Tracker::new()?); - - // Spawn IPC handler thread with the now-available tracker - let ipc_handle = if let Some(rx) = ipc_channel { - let tracker = tracker.clone(); - Some(thread::spawn(move || { - while let Ok(msg) = rx.recv() { - handle_ipc_message(msg, &tracker); - } - })) - } else { - // Without IPC, nothing toggles the tracking_enabled map, so allocator - // events would be dropped by the eBPF is_enabled() check. Enable it up - // front. - tracker.enable_tracking()?; - None - }; - - // Run the target command through bash to handle shell syntax. Drop - // privileges if running under sudo to avoid permission issues when the - // target accesses files owned by the original user. - let mut cmd = Command::new("bash"); - cmd.arg("-c").arg(cmd_string); - let uid_gid = get_user_uid_gid(); - if let Some((uid, gid)) = uid_gid { - debug!("Running under sudo, dropping privileges to uid={uid}, gid={gid}"); - } - - let mut session = tracker - .spawn(&cmd, uid_gid) - .map_err(|e| anyhow!("Failed to spawn child process: {e}"))?; - let root_pid = session.pid(); - let event_rx = session.take_events()?; - debug!("Spawned child with pid {root_pid}"); - - // Generate output file name and create file for streaming events - let file_name = MemtrackArtifact::file_name(Some(root_pid)); - let out_file = std::fs::File::create(out_dir.join(file_name))?; - - // Leave headroom for the ring buffer poll thread and the tracked - // command: encode workers on every core starve the poller during - // allocation bursts, which overflows the kernel ring buffer. - let n_workers = thread::available_parallelism() - .map(|n| n.get().saturating_sub(2).max(1)) - .unwrap_or(4); - - let pipeline_thread = - thread::spawn(move || encode_events(event_rx.into_iter().flatten(), out_file, n_workers)); - - // A worker failure must not skip disabling tracking, draining, joining the - // encoder, or detaching probes. Keep the wait result until teardown is done. - let status = session.wait().context("Failed to wait for command"); - if let Ok(status) = &status { - debug!("Command exited with status: {status}"); - } - - // Stop allocator-event production before draining: the child has exited, - // so anything still arriving is already in the ring buffer. - if let Err(e) = tracker.disable_tracking() { - warn!("Failed to disable tracking: {e:#}"); - } - - // Dropping the session drops the event poller, which does a final drain of - // the ring buffer and then closes the event channel. Without this the - // encode pipeline join below would block forever. - debug!("Stopping the ring buffer poller"); - drop(session); - - debug!("Waiting for the encode pipeline to finish"); - let total = pipeline_thread - .join() - .map_err(|_| anyhow::anyhow!("Failed to join memtrack encode pipeline")) - .and_then(|result| result); - - if let Ok(total) = &total { - info!("Wrote {total} memtrack events to disk"); - } - - // Stop background workers after the ring pipeline has drained. Fatal - // worker errors mean the capture is incomplete. - let finish = tracker.finish(); - let stack_stats = if tracker.stack_capture_enabled() { - Some( - tracker - .stack_capture_stats() - .context("Failed to read stack capture stats"), - ) - } else { - None - }; - if let Some(Ok(stats)) = &stack_stats { - debug!("stack capture stats: {stats:?}"); - } - - // Detach probes explicitly: the IPC thread still holds an Arc clone, so the - // tracker would otherwise never be dropped before process::exit and the - // kernel would close every link fd serially during exit. - tracker.detach(); - - let status = status?; - total?; - finish?; - if let Some(stats) = stack_stats { - stats?; - } - - // Read the eBPF dropped-event counter after the run is complete. - // A non-zero value means the ring buffer overflowed and the trace is - // incomplete. - let dropped_events = tracker - .dropped_events_count() - .context("Failed to read memtrack dropped-event counter")?; - if dropped_events > 0 { - bail!( - "Memtrack ring buffer overflowed: {dropped_events} events lost, aborting since the trace is incomplete.\n\ - Try reducing the benchmark's allocation rate (fewer iterations or smaller inputs), \ - or report it at https://github.com/CodSpeedHQ/codspeed/issues." - ); - } - - // IPC thread will exit when channel closes - drop(ipc_handle); - - Ok(status) + let code = run_cli(std::env::args_os())?; + std::process::exit(code); } From 882866035af8fc9fdb3fe43af9bead1310977400 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:10:56 +0100 Subject: [PATCH 3/7] feat(exec-harness): track subprocesses instead of preloading a library In simulation mode, exec-harness ran each target with an `LD_PRELOAD` library that toggled instrumentation from inside it. That needed a C build step and failed for executables that ignore `LD_PRELOAD`, such as static binaries. Simulation now runs targets like memory mode does, and the runner enables subprocess tracking for exec-harness runs so valgrind follows into the target. Refs COD-3440 Co-Authored-By: Claude --- crates/exec-harness/build.rs | 170 ------------------ .../exec-harness/preload/codspeed_preload.c | 87 --------- .../src/analysis/ld_preload_check.rs | 120 ------------- crates/exec-harness/src/analysis/mod.rs | 38 +--- .../src/analysis/preload_lib_file.rs | 46 ----- crates/exec-harness/src/constants.rs | 14 +- crates/exec-harness/src/lib.rs | 3 +- src/executor/config.rs | 11 +- src/executor/orchestrator.rs | 2 +- 9 files changed, 9 insertions(+), 482 deletions(-) delete mode 100644 crates/exec-harness/build.rs delete mode 100644 crates/exec-harness/preload/codspeed_preload.c delete mode 100644 crates/exec-harness/src/analysis/ld_preload_check.rs delete mode 100644 crates/exec-harness/src/analysis/preload_lib_file.rs diff --git a/crates/exec-harness/build.rs b/crates/exec-harness/build.rs deleted file mode 100644 index bf65ef7e5..000000000 --- a/crates/exec-harness/build.rs +++ /dev/null @@ -1,170 +0,0 @@ -//! Build script for exec-harness -//! -//! This script compiles the `libcodspeed_preload.so` shared library that is used -//! to inject instrumentation into child processes via LD_PRELOAD. -//! -//! The library is built using the `core.c` and headers from the `instrument-hooks-bindings` -//! crate's `instrument-hooks` directory. - -use std::env; -use std::path::PathBuf; - -/// Shared constants for the preload library. -/// These are passed as C defines during compilation and exported as environment -/// variables for the Rust code to use via `env!()`. -struct PreloadConstants { - /// Environment variable name for the benchmark URI. - uri_env: &'static str, - /// Integration name reported to CodSpeed. - integration_name: &'static str, - /// Integration version reported to CodSpeed. - integration_version: &'static str, - /// Filename for the preload shared library. - preload_lib_filename: &'static str, -} - -fn main() { - println!("cargo:rerun-if-changed=preload/codspeed_preload.c"); - println!("cargo:rerun-if-env-changed=CODSPEED_INSTRUMENT_HOOKS_DIR"); - - let preload_constants: PreloadConstants = PreloadConstants::default(); - - // Export constants as environment variables for the Rust code - println!( - "cargo:rustc-env=CODSPEED_URI_ENV={}", - preload_constants.uri_env - ); - println!( - "cargo:rustc-env=CODSPEED_INTEGRATION_NAME={}", - preload_constants.integration_name - ); - println!( - "cargo:rustc-env=CODSPEED_INTEGRATION_VERSION={}", - preload_constants.integration_version - ); - println!( - "cargo:rustc-env=CODSPEED_PRELOAD_LIB_FILENAME={}", - preload_constants.preload_lib_filename - ); - - let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()); - - // Try to get the instrument-hooks directory from the environment variable first, - // otherwise use the one from the instrument-hooks-bindings crate - let instrument_hooks_dir = manifest_dir - .parent() - .unwrap() - .join("instrument-hooks-bindings/instrument-hooks"); - - // Build the preload shared library - let paths = PreloadBuildPaths { - preload_c: manifest_dir.join("preload/codspeed_preload.c"), - core_c: instrument_hooks_dir.join("dist/core.c"), - includes_dir: instrument_hooks_dir.join("includes"), - }; - println!("cargo:rerun-if-changed={}", paths.core_c.display()); - paths.check_sources_exist(); - build_shared_library(&paths, &preload_constants); -} - -/// Build the shared library using the cc crate -fn build_shared_library(paths: &PreloadBuildPaths, constants: &PreloadConstants) { - let uri_env_val = format!("\"{}\"", constants.uri_env); - let integration_name_val = format!("\"{}\"", constants.integration_name); - let integration_version_val = format!("\"{}\"", constants.integration_version); - let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap()); - let out_file = out_dir.join(constants.preload_lib_filename); - - let mut build = cc::Build::new(); - build - .file(&paths.preload_c) - .file(&paths.core_c) - .include(&paths.includes_dir) - .pic(true) - .opt_level(3) - // There's no need to output cargo metadata as we are just building a shared library - // that will be copied to disk and loaded through LD_PRELOAD at runtime - .cargo_metadata(false) - // Pass constants as C defines - .define("CODSPEED_URI_ENV", uri_env_val.as_str()) - .define("CODSPEED_INTEGRATION_NAME", integration_name_val.as_str()) - .define( - "CODSPEED_INTEGRATION_VERSION", - integration_version_val.as_str(), - ) - .std("gnu11") // need gnu11 instead of just c11 for setenv - // Suppress warnings from generated Zig code - .flag("-Wno-format") - .flag("-Wno-format-security") - .flag("-Wno-unused-but-set-variable") - .flag("-Wno-unused-const-variable") - .flag("-Wno-type-limits") - .flag("-Wno-uninitialized") - .flag("-Wno-overflow") - .flag("-Wno-unused-function") - .flag("-Wno-unterminated-string-initialization"); - - // Compile source files to object files - let objects = build.compile_intermediates(); - - // Link object files into shared library - let compiler = build.get_compiler(); - let mut link_cmd = compiler.to_command(); - link_cmd - .arg("-shared") - .arg("-o") - .arg(&out_file) - .args(&objects) - .arg("-lpthread"); - - let status = link_cmd.status().expect("Failed to run linker"); - if !status.success() { - panic!("Failed to link libcodspeed_preload.so"); - } -} - -impl Default for PreloadConstants { - fn default() -> Self { - Self { - uri_env: "CODSPEED_BENCH_URI", - integration_name: "exec-harness", - integration_version: env!("CARGO_PKG_VERSION"), - preload_lib_filename: "libcodspeed_preload.so", - } - } -} - -/// Paths required to build the preload shared library. -struct PreloadBuildPaths { - /// Path to the preload C source file (codspeed_preload.c). - preload_c: PathBuf, - /// Path to the core C source file from instrument-hooks. - core_c: PathBuf, - /// Path to the includes directory from instrument-hooks. - includes_dir: PathBuf, -} - -impl PreloadBuildPaths { - /// Verify that all required source files and directories exist. - /// Panics with a descriptive message if any path is missing. - fn check_sources_exist(&self) { - if !self.core_c.exists() { - panic!( - "core.c not found at {}. Make sure the instrument hooks submodule is available.", - self.core_c.display() - ); - } - if !self.includes_dir.exists() { - panic!( - "includes directory not found at {}. instrument hooks submodule is available.", - self.includes_dir.display() - ); - } - if !self.preload_c.exists() { - panic!( - "codspeed_preload.c not found at {}", - self.preload_c.display() - ); - } - } -} diff --git a/crates/exec-harness/preload/codspeed_preload.c b/crates/exec-harness/preload/codspeed_preload.c deleted file mode 100644 index 418af1430..000000000 --- a/crates/exec-harness/preload/codspeed_preload.c +++ /dev/null @@ -1,87 +0,0 @@ -// LD_PRELOAD library for enabling Valgrind instrumentation in child processes -// -// This library is loaded via LD_PRELOAD into benchmark processes spawned by -// exec-harness. It enables callgrind instrumentation on load and disables it on -// exit, allowing exec-harness to measure arbitrary commands without requiring -// them to link against instrument-hooks. -// -// Environment variables: -// CODSPEED_BENCH_URI - The benchmark URI to report (required) - -#include -#include - -#include "core.h" - -#ifndef RUNNING_ON_VALGRIND -// If somehow the core.h did not include the valgrind header, something is -// wrong, but still have a fallback -#warning "RUNNING_ON_VALGRIND not defined, headers may be missing" -#define RUNNING_ON_VALGRIND 0 -#endif - -// These constants are defined by the build script (build.rs) via -D flags -#ifndef CODSPEED_URI_ENV -#error "CODSPEED_URI_ENV must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_NAME -#error "CODSPEED_INTEGRATION_NAME must be defined by the build system" -#endif -#ifndef CODSPEED_INTEGRATION_VERSION -#error "CODSPEED_INTEGRATION_VERSION must be defined by the build system" -#endif - -static const char *URI_ENV = CODSPEED_URI_ENV; -static const char *INTEGRATION_NAME = CODSPEED_INTEGRATION_NAME; -static const char *INTEGRATION_VERSION = CODSPEED_INTEGRATION_VERSION; - -static InstrumentHooks *g_hooks = NULL; -static const char *g_bench_uri = NULL; - -__attribute__((constructor)) static void codspeed_preload_init(void) { - // Skip initialization if not running under Valgrind yet. - // When using LD_PRELOAD with Valgrind, the constructor runs twice: - // once before Valgrind takes over, and once after. We only want to - // initialize when Valgrind is active. - // - // This is purely empirical, and is not (yet) backed up by documented - // behavior. - if (!RUNNING_ON_VALGRIND) { - return; - } - - g_bench_uri = getenv(URI_ENV); - if (!g_bench_uri) { - return; - } - - g_hooks = instrument_hooks_init(); - if (!g_hooks) { - return; - } - - instrument_hooks_set_integration(g_hooks, INTEGRATION_NAME, - INTEGRATION_VERSION); - - if (instrument_hooks_start_benchmark_inline(g_hooks) != 0) { - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; - return; - } -} - -__attribute__((destructor)) static void codspeed_preload_fini(void) { - // If the process is not the owner of the lock, this means g_hooks was not - // initialized - if (!g_hooks) { - return; - } - - instrument_hooks_stop_benchmark_inline(g_hooks); - - int32_t pid = getpid(); - instrument_hooks_set_executed_benchmark(g_hooks, pid, g_bench_uri); - - instrument_hooks_deinit(g_hooks); - g_hooks = NULL; -} diff --git a/crates/exec-harness/src/analysis/ld_preload_check.rs b/crates/exec-harness/src/analysis/ld_preload_check.rs deleted file mode 100644 index 702f18d2a..000000000 --- a/crates/exec-harness/src/analysis/ld_preload_check.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::prelude::*; -use std::fs; -use std::path::Path; - -/// Checks if the given executable will honor LD_PRELOAD. -/// -/// Returns `Ok(())` if LD_PRELOAD will work, or an error with a descriptive message if not. -/// -/// LD_PRELOAD works for: -/// - Dynamically linked ELF binaries -/// - Scripts (the interpreter is typically dynamically linked) -/// -/// LD_PRELOAD does NOT work for: -/// - Statically linked ELF binaries (no dynamic linker involved) -pub fn check_ld_preload_compatible(executable: &str) -> Result<()> { - let path = resolve_executable(executable)?; - let data = fs::read(&path) - .with_context(|| format!("Failed to read executable: {}", path.display()))?; - - // Check ELF magic bytes - if data.len() >= 4 && &data[0..4] == b"\x7FELF" { - check_elf_is_dynamic(&data, &path) - } else { - // Not an ELF file - likely a script with a shebang. - // Scripts use an interpreter which is typically dynamically linked. - Ok(()) - } -} - -/// Resolve executable name to its full path using PATH lookup. -fn resolve_executable(executable: &str) -> Result { - let path = Path::new(executable); - - // If it's already an absolute or relative path, use it directly - if path.is_absolute() || executable.contains('/') { - return Ok(path.to_path_buf()); - } - - // Search in PATH - if let Ok(path_env) = std::env::var("PATH") { - for dir in path_env.split(':') { - let candidate = Path::new(dir).join(executable); - if candidate.is_file() { - return Ok(candidate); - } - } - } - - bail!("Executable not found in PATH: {executable}") -} - -/// Check if an ELF binary is dynamically linked. -fn check_elf_is_dynamic(data: &[u8], path: &Path) -> Result<()> { - use object::Endianness; - use object::read::elf::ElfFile; - - // Try parsing as 64-bit ELF first, then 32-bit - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - if let Ok(elf) = ElfFile::>::parse(data) { - return check_elf_has_interp(elf, path); - } - - bail!("Failed to parse ELF file: {}", path.display()) -} - -/// Check if an ELF file has a PT_INTERP or PT_DYNAMIC segment, indicating dynamic linking. -fn check_elf_has_interp<'data, Elf>( - elf: object::read::elf::ElfFile<'data, Elf>, - path: &Path, -) -> Result<()> -where - Elf: object::read::elf::FileHeader, -{ - use object::read::elf::ProgramHeader; - - let endian = elf.endian(); - - for segment in elf.elf_program_headers() { - let p_type = segment.p_type(endian); - // Either PT_INTERP or PT_DYNAMIC indicates a dynamically linked binary - if p_type == object::elf::PT_INTERP || p_type == object::elf::PT_DYNAMIC { - return Ok(()); - } - } - - // No PT_INTERP found - this is a statically linked binary - bail!( - "The codspeed CLI in CPU Simulation mode does not support statically linked binaries.\n\n\ - Executable '{}' is statically linked.\n\n\ - Please either:\n\ - - Use a dynamically linked executable, or\n\ - - Use a different measurement mode, or\n\ - - Use one of the CodSpeed framework benchmark integrations", - path.display() - ) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_dynamic_binary() { - // /bin/sh or similar should be dynamically linked on most systems - let result = check_ld_preload_compatible("sh"); - assert!( - result.is_ok(), - "sh should be dynamically linked: {result:?}" - ); - } - - #[test] - fn test_nonexistent_binary() { - let result = check_ld_preload_compatible("nonexistent_binary_12345"); - assert!(result.is_err()); - } -} diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 414e44998..ec6132ca6 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -3,14 +3,10 @@ use crate::constants::INTEGRATION_VERSION; use crate::prelude::*; use crate::BenchmarkCommand; -use crate::constants; use crate::uri; use instrument_hooks_bindings::InstrumentHooks; use std::process::Command; -mod ld_preload_check; -mod preload_lib_file; - pub fn perform(commands: Vec) -> Result<()> { let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION); @@ -20,6 +16,7 @@ pub fn perform(commands: Vec) -> Result<()> { let mut cmd = Command::new(&benchmark_cmd.command[0]); cmd.args(&benchmark_cmd.command[1..]); + hooks.start_benchmark().unwrap(); let status = cmd.status(); hooks.stop_benchmark().unwrap(); @@ -34,36 +31,3 @@ pub fn perform(commands: Vec) -> Result<()> { Ok(()) } - -/// Executes the given benchmark commands using a preload based trick to handle valgrind control. -/// -/// This function is only supported on Unix-like platforms, as it relies on the -/// `LD_PRELOAD` environment variable and Unix file permissions for shared libraries. -/// It will not work on non-Unix platforms or with statically linked binaries. -pub fn perform_with_valgrind(commands: Vec) -> Result<()> { - let preload_lib_path = preload_lib_file::get_preload_lib_path()?; - - for benchmark_cmd in commands { - // Check if the executable will honor LD_PRELOAD before running - ld_preload_check::check_ld_preload_compatible(&benchmark_cmd.command[0])?; - - let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command); - name_and_uri.print_executing(); - - let mut cmd = Command::new(&benchmark_cmd.command[0]); - cmd.args(&benchmark_cmd.command[1..]); - // Use LD_PRELOAD to inject instrumentation into the child process - cmd.env("LD_PRELOAD", preload_lib_path); - cmd.env(constants::URI_ENV, &name_and_uri.uri); - - let mut child = cmd.spawn().context("Failed to spawn command")?; - - let status = child.wait().context("Failed to execute command")?; - - if !status.success() { - bail!("Command exited with non-zero status: {status}"); - } - } - - Ok(()) -} diff --git a/crates/exec-harness/src/analysis/preload_lib_file.rs b/crates/exec-harness/src/analysis/preload_lib_file.rs deleted file mode 100644 index 2d53804cb..000000000 --- a/crates/exec-harness/src/analysis/preload_lib_file.rs +++ /dev/null @@ -1,46 +0,0 @@ -use crate::prelude::*; - -use std::io::Write; -use std::sync::OnceLock; - -/// Filename for the preload shared library. -const PRELOAD_LIB_FILENAME: &str = env!("CODSPEED_PRELOAD_LIB_FILENAME"); - -/// The preload library binary embedded at compile time. -const PRELOAD_LIB_BYTES: &[u8] = include_bytes!(concat!( - env!("OUT_DIR"), - "/", - env!("CODSPEED_PRELOAD_LIB_FILENAME") -)); - -/// Lazily initialized temp file containing the extracted preload library. -/// Kept in a static to prevent cleanup until process exit. -static PRELOAD_LIB_FILE: OnceLock = OnceLock::new(); - -/// Extracts the preload library to a temp file. -fn extract_preload_lib() -> Result { - let mut file = tempfile::Builder::new() - .suffix(PRELOAD_LIB_FILENAME) - .tempfile() - .context("Failed to create temp file for preload library")?; - - file.write_all(PRELOAD_LIB_BYTES) - .context("Failed to write preload library to temp file")?; - - debug!( - "Extracted preload library to temp file: {}", - file.path().display() - ); - - Ok(file) -} - -/// Returns the path to the preload library, extracting it to a temp file if needed. -pub(super) fn get_preload_lib_path() -> Result<&'static std::path::Path> { - if let Some(file) = PRELOAD_LIB_FILE.get() { - return Ok(file.path()); - } - - let file = extract_preload_lib()?; - Ok(PRELOAD_LIB_FILE.get_or_init(|| file).path()) -} diff --git a/crates/exec-harness/src/constants.rs b/crates/exec-harness/src/constants.rs index 9a47591ce..d3f76b899 100644 --- a/crates/exec-harness/src/constants.rs +++ b/crates/exec-harness/src/constants.rs @@ -1,15 +1,5 @@ -//! Shared constants for the exec-harness crate. -//! -//! These constants are defined in the build script (build.rs) and exported as -//! environment variables. The same values are passed to the C preload library -//! as compiler defines, ensuring both Rust and C code use the same source of truth. - -/// Environment variable name for the benchmark URI. -pub const URI_ENV: &str = env!("CODSPEED_URI_ENV"); - /// Integration name reported to CodSpeed. -pub const INTEGRATION_NAME: &str = env!("CODSPEED_INTEGRATION_NAME"); +pub const INTEGRATION_NAME: &str = "exec-harness"; /// Integration version reported to CodSpeed. -/// This should match the version of the `codspeed` crate dependency. -pub const INTEGRATION_VERSION: &str = env!("CODSPEED_INTEGRATION_VERSION"); +pub const INTEGRATION_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/crates/exec-harness/src/lib.rs b/crates/exec-harness/src/lib.rs index 608219583..6eb293d3a 100644 --- a/crates/exec-harness/src/lib.rs +++ b/crates/exec-harness/src/lib.rs @@ -70,7 +70,6 @@ pub fn execute_benchmarks( match measurement_mode { MeasurementMode::Walltime => walltime::perform(commands), - MeasurementMode::Memory => analysis::perform(commands), - MeasurementMode::Simulation => analysis::perform_with_valgrind(commands), + MeasurementMode::Memory | MeasurementMode::Simulation => analysis::perform(commands), } } diff --git a/src/executor/config.rs b/src/executor/config.rs index f3ec640c8..9cb54b7b7 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -205,13 +205,10 @@ impl OrchestratorConfig { } /// Produce a per-execution [`ExecutorConfig`] for the given command and mode. - /// - /// `enable_introspection` controls whether language-level wrappers (Node.js, Go) - /// are injected into `PATH`. This should be `false` for exec-harness targets. pub fn executor_config_for_command( &self, command: String, - enable_introspection: bool, + uses_exec_harness: bool, ) -> ExecutorConfig { ExecutorConfig { working_directory: self.working_directory.clone(), @@ -225,11 +222,11 @@ impl OrchestratorConfig { allow_empty: self.allow_empty, go_runner_version: self.go_runner_version.clone(), extra_env: self.extra_env.clone(), - enable_introspection, + enable_introspection: !uses_exec_harness, fair_sched: self.fair_sched, cycle_estimation: self.cycle_estimation, exclude_allocations: self.exclude_allocations, - simulation_track_subprocess: self.simulation_track_subprocess, + simulation_track_subprocess: self.simulation_track_subprocess || uses_exec_harness, memory_track_physical: self.memory_track_physical, disable_pythonmalloc_override: self.disable_pythonmalloc_override, memory_capture_stack: self.memory_capture_stack, @@ -279,7 +276,7 @@ impl OrchestratorConfig { impl ExecutorConfig { /// Constructs a new `ExecutorConfig` with default values for testing purposes pub fn test() -> Self { - OrchestratorConfig::test().executor_config_for_command("".into(), true) + OrchestratorConfig::test().executor_config_for_command("".into(), false) } } diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index ad2c9c5f7..29d725729 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -143,7 +143,7 @@ impl Orchestrator { for (run_part_index, part) in run_parts.into_iter().enumerate() { let config = self .config - .executor_config_for_command(part.command, !part.uses_exec_harness); + .executor_config_for_command(part.command, part.uses_exec_harness); let mut executor = get_executor_from_mode(part.mode, self.config.walltime_profiler); let profile_folder = self.resolve_profile_folder(&executor.name(), run_part_index, total_parts)?; From 032d5bd98af26b14e795d4189aba57e0f0aa5855 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:12:41 +0100 Subject: [PATCH 4/7] feat: bundle exec-harness into the runner binary exec-harness was released on its own and downloaded at a pinned version the first time a run had exec targets. Run it as a hidden `codspeed exec-harness` subcommand instead, so it always matches the runner and needs no download. The exec-harness crate is no longer released. Internal subcommands now dispatch before any runner setup: they run in the benchmark's working directory, where loading a stray `codspeed.yaml` could abort the measurement. `codspeed samply` is hidden from the help like the others. Refs COD-3440 Co-Authored-By: Claude --- .github/workflows/ci.yml | 4 ---- crates/exec-harness/Cargo.toml | 3 ++- src/binary_pins.rs | 16 +-------------- src/cli/exec/multi_targets.rs | 15 ++++++++++---- src/cli/exec_harness.rs | 14 +++++++++++++ src/cli/mod.rs | 36 +++++++++++++++++++++++++++++++--- src/executor/orchestrator.rs | 12 ------------ 7 files changed, 61 insertions(+), 39 deletions(-) create mode 100644 src/cli/exec_harness.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41d917150..639711b15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,10 +112,6 @@ jobs: with: submodules: true - uses: ./.github/actions/install-rust - - name: Install exec-harness - run: | - cargo install --path crates/exec-harness --locked - - name: Run tests env: # Profiling system commands (e.g. `ls`) with samply is not yet supported on MacOS diff --git a/crates/exec-harness/Cargo.toml b/crates/exec-harness/Cargo.toml index 47234e168..b7c7a26c0 100644 --- a/crates/exec-harness/Cargo.toml +++ b/crates/exec-harness/Cargo.toml @@ -28,5 +28,6 @@ rstest = { workspace = true } [build-dependencies] cc = "1" +# Bundled in the runner binary, not released on its own. [package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] +dist = false diff --git a/src/binary_pins.rs b/src/binary_pins.rs index 85a3a9756..77cb793c6 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -124,13 +124,6 @@ const MEMTRACK_INSTALLER: BinaryPin = BinaryPin { #[cfg(target_os = "linux")] pub const MEMTRACK_VERSION: &str = MEMTRACK_INSTALLER.version; -const EXEC_HARNESS_INSTALLER: BinaryPin = BinaryPin { - version: "1.4.1", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/exec-harness-v{version}/exec-harness-installer.sh", - sha256: "e1891561b054e5ef20448a8eb634366d4e4c8bd2508e462d4771da32f182b7c7", -}; -pub const EXEC_HARNESS_VERSION: &str = EXEC_HARNESS_INSTALLER.version; - const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { version: "cs-mongo-tracer-v0.2.0", url_template: "https://codspeed-public-assets.s3.eu-west-1.amazonaws.com/mongo-tracer/{version}/cs-mongo-tracer-installer.sh", @@ -146,7 +139,6 @@ pub enum PinnedBinary { // Only installed by the Linux-only memory executor. #[cfg_attr(not(target_os = "linux"), allow(dead_code))] MemtrackInstaller, - ExecHarnessInstaller, MongoTracerInstaller, } @@ -155,7 +147,6 @@ impl PinnedBinary { match self { PinnedBinary::ValgrindDeb(target) => target.url(), PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -164,7 +155,6 @@ impl PinnedBinary { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, - PinnedBinary::ExecHarnessInstaller => EXEC_HARNESS_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -178,7 +168,6 @@ mod tests { const INSTALLER_BINARIES: &[PinnedBinary] = &[ PinnedBinary::MemtrackInstaller, - PinnedBinary::ExecHarnessInstaller, PinnedBinary::MongoTracerInstaller, ]; @@ -212,9 +201,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller - | PinnedBinary::ExecHarnessInstaller - | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MemtrackInstaller | PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -231,7 +218,6 @@ mod tests { #[test] fn installer_variant_list_is_exhaustive() { assert_installer_variant_is_listed(PinnedBinary::MemtrackInstaller); - assert_installer_variant_is_listed(PinnedBinary::ExecHarnessInstaller); assert_installer_variant_is_listed(PinnedBinary::MongoTracerInstaller); } diff --git a/src/cli/exec/multi_targets.rs b/src/cli/exec/multi_targets.rs index d24c16b93..0586497e6 100644 --- a/src/cli/exec/multi_targets.rs +++ b/src/cli/exec/multi_targets.rs @@ -1,5 +1,6 @@ +use crate::cli::InternalCommands; +use crate::cli::exec_harness::ExecHarnessArgs; use crate::executor::config::BenchmarkTarget; -use crate::executor::orchestrator::EXEC_HARNESS_COMMAND; use crate::prelude::*; use crate::project_config::{Target, TargetCommand, WalltimeOptions}; use exec_harness::BenchmarkCommand; @@ -92,9 +93,15 @@ pub fn build_exec_targets_pipe_command( .collect::>>()?; let json = serde_json::to_string(&inputs).context("Failed to serialize targets to JSON")?; - Ok(build_pipe_command_from_json(&json)) + build_pipe_command_from_json(&json) } -fn build_pipe_command_from_json(json: &str) -> String { - format!("{EXEC_HARNESS_COMMAND} - <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF") +fn build_pipe_command_from_json(json: &str) -> Result { + let exec_harness = InternalCommands::ExecHarness(ExecHarnessArgs { + args: vec!["-".into()], + }) + .get_shell_command()?; + Ok(format!( + "{exec_harness} <<'CODSPEED_EOF'\n{json}\nCODSPEED_EOF" + )) } diff --git a/src/cli/exec_harness.rs b/src/cli/exec_harness.rs new file mode 100644 index 000000000..ebe30e55f --- /dev/null +++ b/src/cli/exec_harness.rs @@ -0,0 +1,14 @@ +use crate::prelude::*; + +#[derive(Debug, clap::Args)] +pub struct ExecHarnessArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: ExecHarnessArgs) -> Result<()> { + // exec-harness's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("exec-harness")).chain(args.args); + + ::exec_harness::cli::run_cli(argv) +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 2a9218ddc..b589410c3 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,5 +1,6 @@ mod auth; pub(crate) mod exec; +pub(crate) mod exec_harness; pub(crate) mod experimental; mod profile; pub(crate) mod run; @@ -108,8 +109,11 @@ enum Commands { #[derive(Subcommand, Debug)] pub(crate) enum InternalCommands { /// Run the bundled samply profiler. Args are forwarded to samply. - #[command(disable_help_flag = true, disable_help_subcommand = true)] + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] Samply(samply::SamplyArgs), + /// Run the bundled exec-harness. Args are forwarded to exec-harness. + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] + ExecHarness(exec_harness::ExecHarnessArgs), } /// Overrides the executable used to re-invoke internal subcommands. @@ -134,13 +138,36 @@ impl InternalCommands { builder.arg("samply"); builder.args(args.args.iter().cloned()); } + InternalCommands::ExecHarness(args) => { + builder.arg("exec-harness"); + builder.args(args.args.iter().cloned()); + } } Ok(builder) } + + pub fn get_shell_command(&self) -> Result { + Ok(self.get_command_builder()?.as_command_line()) + } +} + +/// Dispatch a bundled subcommand, before any runner setup: these run in the +/// benchmark's working directory, where a stray `codspeed.yaml` would otherwise +/// abort the measurement. +fn run_internal(command: InternalCommands) -> Result<()> { + match command { + InternalCommands::Samply(args) => samply::run(args), + InternalCommands::ExecHarness(args) => exec_harness::run(args), + } } pub async fn run() -> Result<()> { let cli = Cli::parse(); + + if let Commands::Internal(command) = cli.command { + return run_internal(command); + } + let codspeed_config = load_config(&cli)?; let mut api_client = build_api_client(&cli, &codspeed_config); @@ -158,7 +185,8 @@ pub async fn run() -> Result<()> { let setup_cache_dir = setup_cache_dir.as_deref(); match cli.command { - Commands::Run(_) | Commands::Exec(_) | Commands::Internal(InternalCommands::Samply(_)) => {} // these are responsible for their own logger initialization + // These initialize their own logging. + Commands::Run(_) | Commands::Exec(_) => {} _ => { init_local_logger()?; } @@ -210,7 +238,9 @@ pub async fn run() -> Result<()> { Commands::Use(args) => use_mode::run(args)?, Commands::Show => show::run()?, Commands::Update => update::run().await?, - Commands::Internal(InternalCommands::Samply(args)) => samply::run(args)?, + Commands::Internal(_) => { + unreachable!("internal subcommands are dispatched before runner setup") + } } Ok(()) } diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index 29d725729..7c9645b3c 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -1,7 +1,5 @@ use super::{ExecutionContext, ExecutorName, get_executor_from_mode, run_executor}; use crate::api_client::CodSpeedAPIClient; -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; use crate::cli::exec::multi_targets; use crate::cli::run::logger::Logger; use crate::executor::config::BenchmarkTarget; @@ -17,9 +15,6 @@ use serde_json::Value; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -pub const EXEC_HARNESS_COMMAND: &str = "exec-harness"; -pub const EXEC_HARNESS_VERSION: &str = binary_pins::EXEC_HARNESS_VERSION; - /// Shared orchestration state created once per CLI invocation. /// /// Holds the run-level configuration, environment provider, system info, and logger. @@ -82,13 +77,6 @@ impl Orchestrator { .collect(); if !exec_targets.is_empty() { - ensure_binary_installed( - EXEC_HARNESS_COMMAND, - EXEC_HARNESS_VERSION, - PinnedBinary::ExecHarnessInstaller, - ) - .await?; - let pipe_cmd = multi_targets::build_exec_targets_pipe_command(&exec_targets)?; let label = match exec_targets.as_slice() { [BenchmarkTarget::Exec { command, .. }] => { From 58403694344c1b449155f18d8a98eaffe4f120ab Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:13:00 +0100 Subject: [PATCH 5/7] feat(memory): bundle memtrack into the runner binary memtrack was released on its own as `codspeed-memtrack` and installed at a pinned version by the memory executor's setup. Link it into the runner with its eBPF feature and run it as a hidden `codspeed memtrack` subcommand, so it always matches the runner and needs no download. With exec-harness already bundled, the binary installer has no users left and is removed. memtrack is no longer released. The eBPF capabilities are granted to a copy of the runner executable rather than the runner itself, so the runner never runs in glibc's secure-execution mode, which would strip `LD_*` variables from the benchmark environment. The copy lives under `~/.cache/codspeed/memtrack`, keyed by the executable's hash, and is created before any sudo run can leave a root-owned `~/.cache`. Copies of other builds unused for a day are removed. `codspeed setup memtrack-path` prints the copy's path. Because that copy gets `setcap`, the `CODSPEED_SELF_EXE` override of the re-executed binary is now test-only. `codspeed setup status` reports no separate memtrack tool any more, and shows privilege status for executors without one. Building the runner now builds memtrack's vendored libbpf, so CI installs the bpf dependencies for it, the release gets memtrack's build dependencies, and a new CI job builds the musl targets the runner ships. Closes COD-3440 Co-Authored-By: Claude --- .github/workflows/ci.yml | 53 ++++++--- CONTRIBUTING.md | 49 ++------ Cargo.toml | 16 ++- crates/memtrack/Cargo.toml | 15 +-- crates/memtrack/benchmarks/dd.yml | 4 +- crates/memtrack/benchmarks/tar.yml | 4 +- src/binary_installer/mod.rs | 97 ---------------- src/binary_installer/versions.rs | 134 --------------------- src/binary_pins.rs | 21 +--- src/cli/memtrack.rs | 16 +++ src/cli/mod.rs | 45 ++++++-- src/cli/setup.rs | 36 ++++-- src/executor/memory/executor.rs | 24 ++-- src/executor/memory/setup.rs | 179 +++++++++++++++-------------- src/executor/mod.rs | 2 + src/executor/orchestrator.rs | 9 ++ src/lib.rs | 1 - 17 files changed, 256 insertions(+), 449 deletions(-) delete mode 100644 src/binary_installer/mod.rs delete mode 100644 src/binary_installer/versions.rs create mode 100644 src/cli/memtrack.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 639711b15..844a5c528 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,9 @@ jobs: - uses: ./.github/actions/install-rust with: components: rustfmt, clippy + # Building the runner builds memtrack's vendored libbpf-sys with it. + - uses: ./.github/actions/install-bpf-deps + if: matrix.os == 'ubuntu-latest' - uses: j178/prek-action@bdca6f102f98e2b4c7029491a53dfd366469e33d # v2.0.4 with: extra-args: --all-files @@ -102,6 +105,7 @@ jobs: - uses: ./.github/actions/install-rust with: cache-key: ${{ matrix.os }} + - uses: ./.github/actions/install-bpf-deps - name: Run tests run: cargo run -- exec ${{ matrix.extra-args }} -m simulation,walltime,memory --warmup-time 0s --max-rounds 5 -- sleep 1 @@ -230,13 +234,6 @@ jobs: - uses: ./.github/actions/install-rust - uses: ./.github/actions/install-bpf-deps - - name: Install memtrack - uses: baptiste0928/cargo-install@8195d4f734a149db85385bb4102b42efcd373759 # v3.5.0 - with: - crate: memtrack - git: https://github.com/CodSpeedHQ/codspeed - commit: ${{ env.CODSPEED_REV }} - - name: Install codspeed-runner uses: baptiste0928/cargo-install@8195d4f734a149db85385bb4102b42efcd373759 # v3.5.0 with: @@ -250,16 +247,19 @@ jobs: - name: Reuse the cached runner install run: echo "CARGO_INSTALL_ROOT=$HOME/.cargo-install/codspeed-runner" >> "$GITHUB_ENV" - - name: Verify installed binaries + - name: Verify the installed binary run: | - which codspeed codspeed-memtrack + which codspeed codspeed --version - codspeed-memtrack --version - # The benchmarked command is memtrack itself, which needs file capabilities - # to load its eBPF programs even though the runner measures walltime. + # The benchmarked command is memtrack, a subcommand of the runner binary, + # which needs file capabilities to load its eBPF programs even though the + # runner measures walltime. The capabilities land on a copy of the runner, + # which the benchmarks call as `codspeed-memtrack`. - name: Grant memtrack file capabilities - run: codspeed setup --mode memory + run: | + codspeed setup --mode memory + sudo ln -s "$(codspeed setup memtrack-path)" /usr/local/bin/codspeed-memtrack # The write benchmarks overwrite these files every round; creating them # inside a measured round would time one-off disk allocation. @@ -277,6 +277,32 @@ jobs: runner-version: rev:${{ env.CODSPEED_REV }} skip-hash-check-warning: true + musl-build: + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + include: + - runner: ubuntu-latest + target: x86_64-unknown-linux-musl + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-musl + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + submodules: true + - uses: ./.github/actions/install-rust + with: + cache-key: ${{ matrix.target }} + - uses: ./.github/actions/install-bpf-deps + - name: Install the musl toolchain + run: | + sudo apt-get install -y musl-tools linux-libc-dev + rustup target add "${{ matrix.target }}" + + - name: Build + run: cargo build --bin codspeed --target "${{ matrix.target }}" + check: runs-on: ubuntu-latest if: always() @@ -288,6 +314,7 @@ jobs: - basic-run-test - macos-basic-run-test - bpf-tests + - musl-build - benchmarks - memtrack-benchmarks steps: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1eb1419d2..c06d39e02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,50 +10,21 @@ prek install ## Release Process -This repository is a Cargo workspace containing multiple crates. The release process differs depending on which crate you're releasing. +This repository is a Cargo workspace, but only the main runner is released. The other crates +are linked into its binary. ### Workspace Structure - **`codspeed-runner`**: The main CLI binary (`codspeed`) -- **`memtrack`**: Memory tracking binary (`codspeed-memtrack`) -- **`exec-harness`**: Execution harness binary +- **`memtrack`**: Memory tracker, reached as `codspeed memtrack` +- **`exec-harness`**: Execution harness, reached as `codspeed exec-harness` - **`runner-shared`**: Shared library used by other crates -### Releasing Support Crates (memtrack, exec-harness, runner-shared) - -For any crate other than the main runner: - -```bash -cargo release -p --execute -``` - -Where `` is one of: `alpha`, `beta`, `patch`, `minor`, or `major`. - -**Examples:** - -```bash -# Release a new patch version of memtrack -cargo release -p memtrack --execute patch - -# Release a beta version of exec-harness -cargo release -p exec-harness --execute beta -``` - -#### Post-Release: Update Version References - -After releasing `memtrack` or `exec-harness`, you **must** update the version references in the runner code: - -1. **For memtrack**: Update the `MEMTRACK_INSTALLER` pin record in `src/binary_pins.rs` (see [Pinned binary hashes](#pinned-binary-hashes) below). - -2. **For exec-harness**: Update the `EXEC_HARNESS_INSTALLER` pin record in `src/binary_pins.rs`. - -These constants are used by the runner to download and install the correct versions of the binaries from GitHub releases. - ### Pinned binary hashes Every binary the runner downloads at install time is SHA-256-pinned. The pins live in two places: -- **`src/binary_pins.rs`** — the patched valgrind `.deb`, the memtrack installer, the exec-harness installer, and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. +- **`src/binary_pins.rs`** — the patched valgrind `.deb` and the mongo-tracer installer. Each artifact keeps its version, URL template, and hash together in a pin record. - **`src/executor/helpers/introspected_golang/go.sh`** — the go-runner installer published by [CodSpeedHQ/codspeed-go](https://github.com/CodSpeedHQ/codspeed-go), one ` ` row per release in the `GO_RUNNER_INSTALLER_SHA256S` table. `DEFAULT_GO_RUNNER_VERSION` (just below the table) selects the row used by default. When you bump a pinned version (or add a new go-runner row), update the matching pin record / table row with the new version and its SHA-256. @@ -84,16 +55,12 @@ These tests also run in CI, but running them locally before opening the PR avoid ### Releasing the Main Runner -The main runner (`codspeed-runner`) should be released after ensuring all dependency versions are correct. +The main runner (`codspeed-runner`) is the only crate that is released. #### Pre-Release Check -**Verify binary version references**: Check that version constants in the runner code match the released versions: - -- `MEMTRACK_VERSION` in `src/binary_pins.rs` -- `EXEC_HARNESS_VERSION` in `src/binary_pins.rs` - -Also confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released artifacts. +Confirm the SHA-256 entries in the pin records in `src/binary_pins.rs` match the released +artifacts they point at. #### Release Command diff --git a/Cargo.toml b/Cargo.toml index 80fa28da9..a481d4932 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,7 +84,7 @@ bytes = "1" [target.'cfg(target_os = "linux")'.dependencies] procfs = "0.18" caps = "0.5" -memtrack = { path = "crates/memtrack", default-features = false } +memtrack = { path = "crates/memtrack" } ipc-channel = { workspace = true } [dev-dependencies] @@ -148,3 +148,17 @@ targets = ["aarch64-apple-darwin", "aarch64-unknown-linux-musl", "x86_64-unknown binaries.aarch64-apple-darwin = ["codspeed"] binaries.aarch64-unknown-linux-musl = ["codspeed"] binaries.x86_64-unknown-linux-musl = ["codspeed"] + +# memtrack's vendored libbpf/elfutils build runs as part of this package. +[package.metadata.dist.dependencies.apt] +build-essential = "*" +pkgconf = "*" +zlib1g-dev = "*" +libbpf-dev = "*" +musl-tools = "*" +linux-libc-dev = "*" + +# Required for the vendored feature +autopoint = "*" +bison = "*" +flex = "*" diff --git a/crates/memtrack/Cargo.toml b/crates/memtrack/Cargo.toml index 6e9279b1f..9d3ddcde8 100644 --- a/crates/memtrack/Cargo.toml +++ b/crates/memtrack/Cargo.toml @@ -55,17 +55,6 @@ test-log = { workspace = true } insta = { workspace = true, features = ["json", "redactions"] } test-with = { workspace = true } +# Bundled in the runner binary, not released on its own. [package.metadata.dist] -targets = ["aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu"] -features = ["libbpf-rs/static"] - -[package.metadata.dist.dependencies.apt] -build-essential = "*" -pkgconf = "*" -zlib1g-dev = "*" -libbpf-dev = "*" - -# Required for the vendored feature -autopoint = "*" -bison = "*" -flex = "*" +dist = false diff --git a/crates/memtrack/benchmarks/dd.yml b/crates/memtrack/benchmarks/dd.yml index 60e00753b..969c790c9 100644 --- a/crates/memtrack/benchmarks/dd.yml +++ b/crates/memtrack/benchmarks/dd.yml @@ -12,7 +12,7 @@ options: benchmarks: # I/O-heavy with minimal allocation. - name: "memtrack track dd" - exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=0 codspeed-memtrack track "dd if=/dev/zero of=/tmp/memtrack-bench-dd.bin bs=1M count=64 2> /dev/null" --output /tmp/codspeed-memtrack-bench + exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=0 codspeed-memtrack memtrack track "dd if=/dev/zero of=/tmp/memtrack-bench-dd.bin bs=1M count=64 2> /dev/null" --output /tmp/codspeed-memtrack-bench - name: "memtrack track dd (with physical)" - exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=1 codspeed-memtrack track "dd if=/dev/zero of=/tmp/memtrack-bench-dd.bin bs=1M count=64 2> /dev/null" --output /tmp/codspeed-memtrack-bench + exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=1 codspeed-memtrack memtrack track "dd if=/dev/zero of=/tmp/memtrack-bench-dd.bin bs=1M count=64 2> /dev/null" --output /tmp/codspeed-memtrack-bench diff --git a/crates/memtrack/benchmarks/tar.yml b/crates/memtrack/benchmarks/tar.yml index d96a5bdfa..14daa8215 100644 --- a/crates/memtrack/benchmarks/tar.yml +++ b/crates/memtrack/benchmarks/tar.yml @@ -12,7 +12,7 @@ options: benchmarks: # Allocation- and I/O-heavy: many small file reads. - name: "memtrack track tar" - exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=0 codspeed-memtrack track "tar -cf /tmp/memtrack-bench.tar /usr/bin" --output /tmp/codspeed-memtrack-bench + exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=0 codspeed-memtrack memtrack track "tar -cf /tmp/memtrack-bench.tar /usr/bin" --output /tmp/codspeed-memtrack-bench - name: "memtrack track tar (with physical)" - exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=1 codspeed-memtrack track "tar -cf /tmp/memtrack-bench.tar /usr/bin" --output /tmp/codspeed-memtrack-bench + exec: env CODSPEED_MEMTRACK_TRACK_PHYSICAL=1 codspeed-memtrack memtrack track "tar -cf /tmp/memtrack-bench.tar /usr/bin" --output /tmp/codspeed-memtrack-bench diff --git a/src/binary_installer/mod.rs b/src/binary_installer/mod.rs deleted file mode 100644 index d8bdb75bf..000000000 --- a/src/binary_installer/mod.rs +++ /dev/null @@ -1,97 +0,0 @@ -use crate::binary_pins::PinnedBinary; -use crate::cli::run::helpers::download_pinned_file; -use crate::prelude::*; -use semver::Version; -use std::process::Command; -use tempfile::NamedTempFile; - -mod versions; - -/// Ensure a binary is installed, or install it from a `PinnedBinary` installer script. -/// -/// This function checks if the binary is already installed with the correct version. -/// If not, it downloads and executes the pinned installer script. -/// -/// # Arguments -/// * `binary_name` - The binary command name (e.g., "codspeed-memtrack", "codspeed-exec-harness") -/// * `version` - The version to install (e.g., "4.4.2-alpha.2") -/// * `installer` - The `PinnedBinary` installer to download. -pub async fn ensure_binary_installed( - binary_name: &str, - version: &str, - installer: PinnedBinary, -) -> Result<()> { - if is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - debug!("{binary_name} version {version} is already installed"); - return Ok(()); - } - - debug!("Downloading installer for {binary_name}"); - - // Download the installer script to a temporary file (with sha256 verification) - let temp_file = NamedTempFile::new().context("Failed to create temporary file")?; - download_pinned_file(installer, temp_file.path()).await?; - - // Execute the installer script - let output = Command::new("sh") - .arg(temp_file.path()) - .output() - .context("Failed to execute installer command")?; - - if !output.status.success() { - bail!( - "Failed to install {binary_name} version {version}. Installer exited with output: {output:?}", - ); - } - - if !is_command_installed( - binary_name, - Version::parse(version).context("Invalid version format")?, - ) { - bail!( - "Could not veryfy installation of {binary_name} version {version} after running installer" - ); - } - - info!("Successfully installed {binary_name} version {version}"); - Ok(()) -} - -/// Check if the given command is installed and its version matches the expected version. -/// -/// Expects the command to support the `--version` flag and return a version string. -fn is_command_installed(command: &str, expected_version: Version) -> bool { - let is_command_installed = Command::new("which") - .arg(command) - .output() - .is_ok_and(|output| output.status.success()); - - if !is_command_installed { - debug!("{command} is not installed"); - return false; - } - - let Ok(version_output) = Command::new(command).arg("--version").output() else { - return false; - }; - - if !version_output.status.success() { - debug!( - "Failed to get command version. stderr: {}", - String::from_utf8_lossy(&version_output.stderr) - ); - return false; - } - - let version_string = String::from_utf8_lossy(&version_output.stdout); - let Ok(version) = versions::parse_from_output(&version_string) else { - return false; - }; - - debug!("Found {command} version: {version}"); - - versions::is_compatible(command, &version, &expected_version) -} diff --git a/src/binary_installer/versions.rs b/src/binary_installer/versions.rs deleted file mode 100644 index 4d12e7de7..000000000 --- a/src/binary_installer/versions.rs +++ /dev/null @@ -1,134 +0,0 @@ -use crate::prelude::*; -use semver::Version; - -/// Parse a version string from command output. -/// -/// Expects the output format to be: "command_name version_string" -/// Example: "codspeed-memtrack 4.4.2" -pub(super) fn parse_from_output(output: &str) -> Result { - let version_str = output - .split_once(" ") - .context("Unexpected version output format: missing space separator")? - .1 - .trim(); - - Version::parse(version_str) - .with_context(|| format!("Failed to parse version from: {version_str}")) -} - -/// Check if an installed version is compatible with the expected version. -/// -/// Returns true if the installed version is greater than or equal to the expected version. -/// Logs warnings for outdated or experimental versions. -pub(super) fn is_compatible(command: &str, installed: &Version, expected: &Version) -> bool { - match installed.cmp(expected) { - std::cmp::Ordering::Less => { - warn!( - "{command} is installed but the version is too old. expecting {expected} or higher but found installed: {installed}", - ); - false - } - std::cmp::Ordering::Greater => { - warn!( - "Using experimental {command} version {installed}. The recommended version is {expected}", - ); - true - } - std::cmp::Ordering::Equal => true, - } -} -#[cfg(test)] -mod tests { - use super::*; - - mod parse_version_from_output { - use super::*; - - #[test] - fn parses_valid_version() { - let output = "codspeed-memtrack 4.4.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version, Version::new(4, 4, 2)); - } - - #[test] - fn parses_version_with_prerelease() { - let output = "codspeed-exec-harness 4.4.2-alpha.2"; - let version = parse_from_output(output).unwrap(); - assert_eq!(version.major, 4); - assert_eq!(version.minor, 4); - assert_eq!(version.patch, 2); - assert_eq!(version.pre.as_str(), "alpha.2"); - } - } - - mod is_version_compatible { - use super::*; - - #[test] - fn returns_true_for_equal_versions() { - let installed = Version::new(4, 4, 2); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_true_for_newer_version() { - let installed = Version::new(4, 5, 0); - let expected = Version::new(4, 4, 2); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn returns_false_for_older_version() { - let installed = Version::new(4, 3, 0); - let expected = Version::new(4, 4, 2); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn handles_prerelease_versions() { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 1); - // 4.4.2-alpha.2 > 4.4.1 because 4.4.2 > 4.4.1 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - #[test] - fn prerelease_different_stage() { - { - let installed = Version::parse("4.4.2-alpha.2").unwrap(); - let expected = Version::new(4, 4, 2); - // 4.4.2-alpha.2 < 4.4.2 - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-beta.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.1").unwrap(); - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::new(4, 4, 2); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - // 4.4.2 > 4.4.2-alpha.2 - assert!(is_compatible("test-cmd", &installed, &expected)); - } - - { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-beta.1").unwrap(); - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } - - #[test] - fn prerelease_same_stage() { - let installed = Version::parse("4.4.2-alpha.1").unwrap(); - let expected = Version::parse("4.4.2-alpha.2").unwrap(); - - assert!(!is_compatible("test-cmd", &installed, &expected)); - } - } -} diff --git a/src/binary_pins.rs b/src/binary_pins.rs index 77cb793c6..2315c8729 100644 --- a/src/binary_pins.rs +++ b/src/binary_pins.rs @@ -116,14 +116,6 @@ impl ValgrindTarget { } } -const MEMTRACK_INSTALLER: BinaryPin = BinaryPin { - version: "1.6.1", - url_template: "https://github.com/CodSpeedHQ/codspeed/releases/download/memtrack-v{version}/memtrack-installer.sh", - sha256: "3912e92bed8fe5d4ad85bb4208edec71e5b707a6fd8fb73dd9625b890b04f438", -}; -#[cfg(target_os = "linux")] -pub const MEMTRACK_VERSION: &str = MEMTRACK_INSTALLER.version; - const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { version: "cs-mongo-tracer-v0.2.0", url_template: "https://codspeed-public-assets.s3.eu-west-1.amazonaws.com/mongo-tracer/{version}/cs-mongo-tracer-installer.sh", @@ -136,9 +128,6 @@ const MONGO_TRACER_INSTALLER: BinaryPin = BinaryPin { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PinnedBinary { ValgrindDeb(ValgrindTarget), - // Only installed by the Linux-only memory executor. - #[cfg_attr(not(target_os = "linux"), allow(dead_code))] - MemtrackInstaller, MongoTracerInstaller, } @@ -146,7 +135,6 @@ impl PinnedBinary { pub fn url(&self) -> String { match self { PinnedBinary::ValgrindDeb(target) => target.url(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.url(), PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.url(), } } @@ -154,7 +142,6 @@ impl PinnedBinary { pub fn sha256(&self) -> &'static str { match self { PinnedBinary::ValgrindDeb(target) => target.sha256(), - PinnedBinary::MemtrackInstaller => MEMTRACK_INSTALLER.sha256, PinnedBinary::MongoTracerInstaller => MONGO_TRACER_INSTALLER.sha256, } } @@ -166,10 +153,7 @@ mod tests { use crate::cli::run::helpers::download_pinned_file; use tempfile::NamedTempFile; - const INSTALLER_BINARIES: &[PinnedBinary] = &[ - PinnedBinary::MemtrackInstaller, - PinnedBinary::MongoTracerInstaller, - ]; + const INSTALLER_BINARIES: &[PinnedBinary] = &[PinnedBinary::MongoTracerInstaller]; const ALL_VALGRIND_TARGETS: &[ValgrindTarget] = &[ ValgrindTarget { @@ -201,7 +185,7 @@ mod tests { fn assert_installer_variant_is_listed(binary: PinnedBinary) { match binary { PinnedBinary::ValgrindDeb(_) => {} - PinnedBinary::MemtrackInstaller | PinnedBinary::MongoTracerInstaller => { + PinnedBinary::MongoTracerInstaller => { assert!(INSTALLER_BINARIES.contains(&binary)); } } @@ -217,7 +201,6 @@ mod tests { #[test] fn installer_variant_list_is_exhaustive() { - assert_installer_variant_is_listed(PinnedBinary::MemtrackInstaller); assert_installer_variant_is_listed(PinnedBinary::MongoTracerInstaller); } diff --git a/src/cli/memtrack.rs b/src/cli/memtrack.rs new file mode 100644 index 000000000..ba7e121d1 --- /dev/null +++ b/src/cli/memtrack.rs @@ -0,0 +1,16 @@ +use crate::prelude::*; + +#[derive(Debug, clap::Args)] +pub struct MemtrackArgs { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub args: Vec, +} + +pub fn run(args: MemtrackArgs) -> Result<()> { + // memtrack's own clap parser expects its name as `argv[0]`, not ours. + let argv = std::iter::once(std::ffi::OsString::from("memtrack")).chain(args.args); + + // The runner reads this exit code to decide whether the benchmark failed. + let code = ::memtrack::cli::run_cli(argv)?; + std::process::exit(code); +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index b589410c3..ba70a0bfd 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2,6 +2,8 @@ mod auth; pub(crate) mod exec; pub(crate) mod exec_harness; pub(crate) mod experimental; +#[cfg(target_os = "linux")] +pub(crate) mod memtrack; mod profile; pub(crate) mod run; pub(crate) mod samply; @@ -114,25 +116,39 @@ pub(crate) enum InternalCommands { /// Run the bundled exec-harness. Args are forwarded to exec-harness. #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] ExecHarness(exec_harness::ExecHarnessArgs), + /// Run the bundled memtrack. Args are forwarded to memtrack. + #[cfg(target_os = "linux")] + #[command(hide = true, disable_help_flag = true, disable_help_subcommand = true)] + Memtrack(memtrack::MemtrackArgs), } -/// Overrides the executable used to re-invoke internal subcommands. +/// Test-only override for the executable internal subcommands re-exec: under +/// `cargo test` [`std::env::current_exe`] is the test harness. /// -/// [`std::env::current_exe`] is not always a binary that can dispatch them: it -/// resolves to the host executable when this crate is linked into one, and to -/// a wrapper when the CLI is invoked through a launcher script. +/// `cfg(test)` because a copy of this path gets `sudo setcap +ep`. +#[cfg(test)] pub(crate) const SELF_EXE_ENV_VAR: &str = "CODSPEED_SELF_EXE"; +/// The executable that internal subcommands are re-invoked through. +pub(crate) fn self_exe() -> Result { + #[cfg(test)] + if let Some(path) = std::env::var_os(SELF_EXE_ENV_VAR) { + return Ok(PathBuf::from(path)); + } + + std::env::current_exe().context("failed to resolve current executable for internal subcommand") +} + impl InternalCommands { /// Build a [`CommandBuilder`] that re-execs the current binary into this /// internal subcommand. Each variant owns its own arg layout. pub fn get_command_builder(&self) -> Result { - let self_exe = match std::env::var_os(SELF_EXE_ENV_VAR) { - Some(path) => PathBuf::from(path), - None => std::env::current_exe() - .context("failed to resolve current executable for internal subcommand")?, - }; - let mut builder = CommandBuilder::new(self_exe); + Ok(self.command_builder_for(self_exe()?)) + } + + /// Same as [`Self::get_command_builder`], re-executing `program` instead. + pub fn command_builder_for(&self, program: PathBuf) -> CommandBuilder { + let mut builder = CommandBuilder::new(program); match self { InternalCommands::Samply(args) => { builder.arg("samply"); @@ -142,8 +158,13 @@ impl InternalCommands { builder.arg("exec-harness"); builder.args(args.args.iter().cloned()); } + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => { + builder.arg("memtrack"); + builder.args(args.args.iter().cloned()); + } } - Ok(builder) + builder } pub fn get_shell_command(&self) -> Result { @@ -158,6 +179,8 @@ fn run_internal(command: InternalCommands) -> Result<()> { match command { InternalCommands::Samply(args) => samply::run(args), InternalCommands::ExecHarness(args) => exec_harness::run(args), + #[cfg(target_os = "linux")] + InternalCommands::Memtrack(args) => memtrack::run(args), } } diff --git a/src/cli/setup.rs b/src/cli/setup.rs index e1f63f6dd..afe2f1a25 100644 --- a/src/cli/setup.rs +++ b/src/cli/setup.rs @@ -31,11 +31,20 @@ pub struct SetupArgs { enum SetupCommands { /// Show the installation status of CodSpeed tools Status, + /// Print the path of the binary that holds memtrack's capabilities + #[cfg(target_os = "linux")] + #[command(hide = true)] + MemtrackPath, } pub async fn run(args: SetupArgs, setup_cache_dir: Option<&Path>) -> Result<()> { match args.command { Some(SetupCommands::Status) => status(&args.mode), + #[cfg(target_os = "linux")] + Some(SetupCommands::MemtrackPath) => { + println!("{}", crate::executor::memtrack_path()?.display()); + Ok(()) + } None => setup(&args.mode, setup_cache_dir).await, } } @@ -120,7 +129,7 @@ pub fn status(modes: &[RunnerMode]) -> Result<()> { if executor.support_level(&system_info) == ExecutorSupport::Unsupported { continue; } - match executor.tool_status() { + let installed = match executor.tool_status() { Some(tool_status) => match &tool_status.status { ToolInstallStatus::Installed { version } => { info!( @@ -130,15 +139,7 @@ pub fn status(modes: &[RunnerMode]) -> Result<()> { tool_status.tool_name, version ); - match executor.privilege_status() { - Some(PrivilegeStatus::Satisfied { detail }) => { - info!(" {} privileges: {}", check_mark(), detail); - } - Some(PrivilegeStatus::Missing { message }) => { - info!(" {} privileges: {}", cross_mark(), message); - } - None => {} - } + true } ToolInstallStatus::IncorrectVersion { version, message } => { info!( @@ -149,6 +150,7 @@ pub fn status(modes: &[RunnerMode]) -> Result<()> { version, message ); + false } ToolInstallStatus::NotInstalled => { info!( @@ -157,6 +159,7 @@ pub fn status(modes: &[RunnerMode]) -> Result<()> { executor.name(), tool_status.tool_name ); + false } }, None => { @@ -165,6 +168,19 @@ pub fn status(modes: &[RunnerMode]) -> Result<()> { check_mark(), executor.name() ); + true + } + }; + + if installed { + match executor.privilege_status() { + Some(PrivilegeStatus::Satisfied { detail }) => { + info!(" {} privileges: {}", check_mark(), detail); + } + Some(PrivilegeStatus::Missing { message }) => { + info!(" {} privileges: {}", cross_mark(), message); + } + None => {} } } } diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index 1cfbe4775..d8b9ad573 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -1,3 +1,5 @@ +use crate::cli::InternalCommands; +use crate::cli::memtrack::MemtrackArgs; use crate::executor::ExecutorName; use crate::executor::ExecutorSupport; use crate::executor::PrivilegeStatus; @@ -27,15 +29,11 @@ use runner_shared::fifo::IntegrationMode; use semver::Version; use std::cell::RefCell; use std::fs::canonicalize; -use std::path::Path; use std::rc::Rc; use tempfile::NamedTempFile; use tokio::time::{Duration, timeout}; -use super::setup::{ - MEMTRACK_COMMAND, ensure_memtrack_capabilities, get_memtrack_status, has_memtrack_capabilities, - install_memtrack, -}; +use super::setup::{ensure_memtrack_capabilities, has_memtrack_capabilities, memtrack_path}; pub struct MemoryExecutor; @@ -63,8 +61,8 @@ impl MemoryExecutor { let bench_command = get_bench_command(&execution_context.config)?; let (bench_command, env_file) = prefix_command_with_env(&bench_command, &extra_env)?; - // Build the memtrack command - let mut cmd_builder = CommandBuilder::new(MEMTRACK_COMMAND); + let mut cmd_builder = InternalCommands::Memtrack(MemtrackArgs { args: vec![] }) + .command_builder_for(memtrack_path()?); if execution_context.config.memory_track_physical { cmd_builder.env("CODSPEED_MEMTRACK_TRACK_PHYSICAL", "1"); } @@ -102,7 +100,7 @@ impl MemoryExecutor { } bail!( - "{MEMTRACK_COMMAND} needs elevated privileges to load its eBPF programs, but the \ + "codspeed needs elevated privileges to load its eBPF programs, but the \ required capabilities could not be granted." ); } @@ -115,7 +113,7 @@ impl Executor for MemoryExecutor { } fn tool_status(&self) -> Option { - Some(get_memtrack_status()) + None } fn privilege_status(&self) -> Option { @@ -146,14 +144,6 @@ impl Executor for MemoryExecutor { } } - async fn setup( - &self, - _system_info: &SystemInfo, - _setup_cache_dir: Option<&Path>, - ) -> Result<()> { - install_memtrack().await - } - fn grant_privileges(&self) -> Result<()> { // A delegated BPF token already provides the privilege memtrack needs; // there is nothing to grant (and no sudo to prompt for) in that case. diff --git a/src/executor/memory/setup.rs b/src/executor/memory/setup.rs index e393e8b1b..97ac64d55 100644 --- a/src/executor/memory/setup.rs +++ b/src/executor/memory/setup.rs @@ -1,15 +1,11 @@ -use crate::binary_installer::ensure_binary_installed; -use crate::binary_pins::{self, PinnedBinary}; +use crate::cli::self_exe; use crate::executor::helpers::capabilities::binary_has_capabilities; use crate::executor::helpers::run_with_sudo::{is_root_user, run_with_sudo}; -use crate::executor::{ToolInstallStatus, ToolStatus}; use crate::prelude::*; use caps::Capability; -use std::path::PathBuf; -use std::process::Command; - -pub const MEMTRACK_COMMAND: &str = "codspeed-memtrack"; -pub const MEMTRACK_CODSPEED_VERSION: &str = binary_pins::MEMTRACK_VERSION; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; +use std::time::{Duration, SystemTime}; const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ Capability::CAP_DAC_READ_SEARCH, @@ -19,6 +15,9 @@ const MEMTRACK_REQUIRED_CAPS: &[Capability] = &[ Capability::CAP_SYS_RESOURCE, ]; +/// Copies of other builds unused for this long are removed. +const STALE_COPY_AGE: Duration = Duration::from_secs(24 * 60 * 60); + fn memtrack_required_caps_mask() -> u64 { MEMTRACK_REQUIRED_CAPS .iter() @@ -28,7 +27,7 @@ fn memtrack_required_caps_mask() -> u64 { /// `setcap` grammar form of [`MEMTRACK_REQUIRED_CAPS`]: the lowercase cap names /// (libcap renders them lowercase) joined with commas and the `+ep` /// effective+permitted flag. Derived from the enum so the two never drift. -fn memtrack_setcap_spec() -> String { +pub(crate) fn memtrack_setcap_spec() -> String { let caps = MEMTRACK_REQUIRED_CAPS .iter() .map(|c| c.to_string().to_lowercase()) @@ -37,14 +36,85 @@ fn memtrack_setcap_spec() -> String { format!("{caps}+ep") } -fn memtrack_path() -> Option { - which::which(MEMTRACK_COMMAND).ok() +/// The binary that carries the eBPF capabilities: a copy of this executable, so +/// the runner itself never runs in glibc's secure-execution mode, which strips +/// `LD_*` and similar variables from the environment benchmarks inherit. +/// Granted `+ep`, not inheritable, so a benchmark spawned from it does not +/// receive them. +/// +/// Keyed by a hash of this executable, and copied there on first use. The copy's +/// directory mtime records its last use. +pub fn memtrack_path() -> Result { + static PATH: OnceLock = OnceLock::new(); + if let Some(path) = PATH.get() { + return Ok(path.clone()); + } + + let exe = self_exe()?; + let hash = + sha256::try_digest(&exe).with_context(|| format!("failed to hash {}", exe.display()))?; + let cache_dir = memtrack_cache_dir()?; + let dir = cache_dir.join(hash); + let path = dir.join("codspeed"); + if path.exists() { + let _ = std::fs::File::open(&dir).and_then(|dir| dir.set_modified(SystemTime::now())); + } else { + std::fs::create_dir_all(&dir) + .with_context(|| format!("failed to create {}", dir.display()))?; + // Linked into place: `path.exists()` must never accept an interrupted copy, + // and a concurrent install must not replace a copy that may already hold + // its capabilities. + let tmp = dir.join(format!("codspeed.{}.tmp", std::process::id())); + std::fs::copy(&exe, &tmp) + .with_context(|| format!("failed to copy {} to {}", exe.display(), tmp.display()))?; + let linked = std::fs::hard_link(&tmp, &path); + let _ = std::fs::remove_file(&tmp); + match linked { + Err(e) if e.kind() != std::io::ErrorKind::AlreadyExists => { + return Err(e).with_context(|| { + format!("failed to move {} to {}", tmp.display(), path.display()) + }); + } + _ => {} + } + prune_stale_copies(&cache_dir, &dir); + } + + Ok(PATH.get_or_init(|| path).clone()) +} + +fn prune_stale_copies(cache_dir: &Path, current: &Path) { + let Ok(entries) = std::fs::read_dir(cache_dir) else { + return; + }; + for entry in entries.flatten() { + let dir = entry.path(); + let is_stale = entry + .metadata() + .and_then(|metadata| metadata.modified()) + .is_ok_and(|modified| modified.elapsed().is_ok_and(|age| age > STALE_COPY_AGE)); + if dir != current && is_stale { + if let Err(e) = std::fs::remove_dir_all(&dir) { + debug!( + "Failed to remove stale memtrack copy {}: {e}", + dir.display() + ); + } + } + } +} + +fn memtrack_cache_dir() -> Result { + let cache = std::env::var_os("XDG_CACHE_HOME") + .map(PathBuf::from) + .or_else(|| std::env::var_os("HOME").map(|home| PathBuf::from(home).join(".cache"))) + .context("neither XDG_CACHE_HOME nor HOME is set")?; + Ok(cache.join("codspeed").join("memtrack")) } /// Whether the installed memtrack binary already carries the required capabilities. pub fn has_memtrack_capabilities() -> bool { - memtrack_path() - .is_some_and(|path| binary_has_capabilities(&path, memtrack_required_caps_mask())) + memtrack_path().is_ok_and(|path| binary_has_capabilities(&path, memtrack_required_caps_mask())) } /// Grant memtrack the capabilities it needs to run without sudo. @@ -54,14 +124,19 @@ pub fn has_memtrack_capabilities() -> bool { /// Failures are surfaced as warnings rather than aborting, since the run-time /// privilege guard enforces the requirement and reports it clearly. pub fn ensure_memtrack_capabilities() -> Result<()> { + const MEMTRACK_COMMAND: &str = "codspeed"; + if is_root_user() { debug!("Running as root, memtrack does not need file capabilities"); return Ok(()); } - let Some(path) = memtrack_path() else { - warn!("Could not locate {MEMTRACK_COMMAND} to grant capabilities"); - return Ok(()); + let path = match memtrack_path() { + Ok(path) => path, + Err(e) => { + warn!("Could not install {MEMTRACK_COMMAND} to grant capabilities ({e:#})"); + return Ok(()); + } }; if binary_has_capabilities(&path, memtrack_required_caps_mask()) { @@ -92,75 +167,3 @@ pub fn ensure_memtrack_capabilities() -> Result<()> { Ok(()) } - -pub fn get_memtrack_status() -> ToolStatus { - let tool_name = MEMTRACK_COMMAND.to_string(); - - let is_available = Command::new("which") - .arg(MEMTRACK_COMMAND) - .output() - .is_ok_and(|output| output.status.success()); - if !is_available { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let Ok(version_output) = Command::new(MEMTRACK_COMMAND).arg("--version").output() else { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - }; - - if !version_output.status.success() { - return ToolStatus { - tool_name, - status: ToolInstallStatus::NotInstalled, - }; - } - - let version = String::from_utf8_lossy(&version_output.stdout) - .trim() - .to_string(); - - // Parse the version number from output like "memtrack 1.2.2" - let expected = semver::Version::parse(MEMTRACK_CODSPEED_VERSION).unwrap(); - if let Some(version_str) = version.split_once(' ').map(|(_, v)| v.trim()) { - if let Ok(installed) = semver::Version::parse(version_str) { - if installed < expected { - return ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: format!( - "version too old, expecting {MEMTRACK_CODSPEED_VERSION} or higher", - ), - }, - }; - } - return ToolStatus { - tool_name, - status: ToolInstallStatus::Installed { version }, - }; - } - } - - ToolStatus { - tool_name, - status: ToolInstallStatus::IncorrectVersion { - version, - message: "could not parse version".to_string(), - }, - } -} - -pub async fn install_memtrack() -> Result<()> { - ensure_binary_installed( - MEMTRACK_COMMAND, - MEMTRACK_CODSPEED_VERSION, - PinnedBinary::MemtrackInstaller, - ) - .await -} diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 74885c429..0d88d9bb0 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -24,6 +24,8 @@ pub use orchestrator::Orchestrator; #[cfg(target_os = "linux")] use memory::executor::MemoryExecutor; +#[cfg(target_os = "linux")] +pub(crate) use memory::setup::memtrack_path; use std::path::Path; use valgrind::executor::ValgrindExecutor; use wall_time::executor::WallTimeExecutor; diff --git a/src/executor/orchestrator.rs b/src/executor/orchestrator.rs index 7c9645b3c..4dc8b8bdb 100644 --- a/src/executor/orchestrator.rs +++ b/src/executor/orchestrator.rs @@ -125,6 +125,15 @@ impl Orchestrator { let mut all_completed_runs = vec![]; if !self.config.skip_run { + // Install memtrack's copy before any run under sudo, which may create + // a root-owned `~/.cache` (e.g. samply's symbol cache in walltime). + #[cfg(target_os = "linux")] + if modes.contains(&RunnerMode::Memory) { + if let Err(e) = super::memtrack_path() { + debug!("Failed to install memtrack: {e:#}"); + } + } + start_opened_group!("Running the benchmarks"); } diff --git a/src/lib.rs b/src/lib.rs index fe86e3e32..c2926fae1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,6 @@ //! CodSpeed Runner library mod api_client; -mod binary_installer; mod binary_pins; pub mod cli; mod config; From fd51df2ef7b9a96980ec9e53472aa18a43a91239 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 2 Nov 2026 18:43:42 +0100 Subject: [PATCH 6/7] test: run the executor tests in release With memtrack linked into the runner, every memory run hashes the runner binary to locate its capability-holding copy and runs memtrack from it. In the debug build both are unoptimized: hashing the 376 MB binary alone takes about 5.6s per run, which took the memory shards from ~10s to ~42s. Build and set up the executor tests with the release profile; the memory shards are back to 8-10s. Refs COD-3440 Co-Authored-By: Claude --- tests/docker/run.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/docker/run.sh b/tests/docker/run.sh index a5021d69d..1f88091e5 100755 --- a/tests/docker/run.sh +++ b/tests/docker/run.sh @@ -85,7 +85,9 @@ docker_flags=( in_container() { docker run --rm "${tty[@]}" "${docker_flags[@]}" "$run_image" "$@" } -cargo_test=(cargo test -p codspeed-runner --features executor-tests --test executors) +# Release: every memory run hashes the runner binary and runs the memtrack linked +# into it, both slow unoptimized. +cargo_test=(cargo test --release -p codspeed-runner --features executor-tests --test executors) tty=() run_image=$image:$flavor @@ -107,7 +109,7 @@ if ! docker image inspect "$run_image" >/dev/null 2>&1; then setup_container=$image-setup-$flavor docker rm -f "$setup_container" >/dev/null 2>&1 || true docker run --name "$setup_container" "${docker_flags[@]}" "${setup_flags[@]}" "$image:$flavor" \ - tests/docker/setup.sh /home/tester/target/debug/codspeed "${valgrind_source[@]}" + tests/docker/setup.sh /home/tester/target/release/codspeed "${valgrind_source[@]}" docker commit "$setup_container" "$run_image" >/dev/null docker rm "$setup_container" >/dev/null docker images --filter "reference=$image:$flavor-setup-*" --format '{{.Repository}}:{{.Tag}}' \ From fbb67ac856257ec595a8a2f283289d62307c05b1 Mon Sep 17 00:00:00 2001 From: Luc Varoqui Date: Wed, 14 Oct 2026 15:43:03 +0200 Subject: [PATCH 7/7] feat(instrument-hooks): declare benchmarks run in another process Add set_executed_benchmark_for_pid, which passes an explicit pid to instrument_hooks_set_executed_benchmark instead of the calling process' own. set_executed_benchmark keeps its behavior and delegates to it. Bump instrument-hooks, whose valgrind instrument now writes a "Benchmark pid: " desc line in the dump part when that pid is not the calling process'. This also brings thread-safe C API exports and the callgrind_toggle_collect helper. Refs COD-3722 Co-Authored-By: Claude Opus 5.5 --- crates/exec-harness/src/analysis/mod.rs | 11 +++-- .../instrument-hooks | 2 +- crates/instrument-hooks-bindings/src/lib.rs | 18 +++++++- src/executor/valgrind/measure.rs | 7 +++ tests/docker/run.sh | 2 +- tests/executors.rs | 45 +++++++++++++++++++ 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index ec6132ca6..b4e41e6e6 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -18,15 +18,20 @@ pub fn perform(commands: Vec) -> Result<()> { cmd.args(&benchmark_cmd.command[1..]); hooks.start_benchmark().unwrap(); - let status = cmd.status(); + let result = cmd.spawn().and_then(|mut child| { + let pid = child.id(); + child.wait().map(|status| (pid, status)) + }); hooks.stop_benchmark().unwrap(); - let status = status.context("Failed to execute command")?; + let (pid, status) = result.context("Failed to execute command")?; if !status.success() { bail!("Command exited with non-zero status: {status}"); } - hooks.set_executed_benchmark(&name_and_uri.uri).unwrap(); + hooks + .set_executed_benchmark_for_pid(pid, &name_and_uri.uri) + .unwrap(); } Ok(()) diff --git a/crates/instrument-hooks-bindings/instrument-hooks b/crates/instrument-hooks-bindings/instrument-hooks index b9ddb5bc6..d38f44cbc 160000 --- a/crates/instrument-hooks-bindings/instrument-hooks +++ b/crates/instrument-hooks-bindings/instrument-hooks @@ -1 +1 @@ -Subproject commit b9ddb5bc654b2e6fa13eb18efcd3a45e7ecda0bb +Subproject commit d38f44cbc992b58920440d8c80f6c1222d6657e2 diff --git a/crates/instrument-hooks-bindings/src/lib.rs b/crates/instrument-hooks-bindings/src/lib.rs index 8ea620cb8..09a031dd7 100644 --- a/crates/instrument-hooks-bindings/src/lib.rs +++ b/crates/instrument-hooks-bindings/src/lib.rs @@ -58,10 +58,15 @@ mod linux_impl { #[inline(always)] pub fn set_executed_benchmark(&self, uri: &str) -> Result<(), u8> { - let pid = std::process::id() as i32; + self.set_executed_benchmark_for_pid(std::process::id(), uri) + } + + /// Declares a benchmark that ran in the process `pid` rather than in this one. + #[inline(always)] + pub fn set_executed_benchmark_for_pid(&self, pid: u32, uri: &str) -> Result<(), u8> { let c_uri = CString::new(uri).map_err(|_| 1u8)?; let result = unsafe { - ffi::instrument_hooks_set_executed_benchmark(self.0, pid, c_uri.as_ptr()) + ffi::instrument_hooks_set_executed_benchmark(self.0, pid as i32, c_uri.as_ptr()) }; if result == 0 { Ok(()) } else { Err(result) } } @@ -184,6 +189,10 @@ mod other_impl { Ok(()) } + pub fn set_executed_benchmark_for_pid(&self, _pid: u32, _uri: &str) -> Result<(), u8> { + Ok(()) + } + pub fn set_integration(&self, _name: &str, _version: &str) -> Result<(), u8> { Ok(()) } @@ -213,6 +222,11 @@ mod tests { let hooks = InstrumentHooks::instance("test_integration", "1.0.0"); assert!(!hooks.is_instrumented() || hooks.start_benchmark().is_ok()); assert!(hooks.set_executed_benchmark("test_uri").is_ok()); + assert!( + hooks + .set_executed_benchmark_for_pid(std::process::id() + 1, "test_uri") + .is_ok() + ); assert!(hooks.set_integration("test_integration", "1.0.0").is_ok()); let start = InstrumentHooks::current_timestamp(); let end = start + 1_000_000; // Simulate 1ms later diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index fca89e776..44dac04a1 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -64,6 +64,13 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec /dev/null"]) + .assert() + .success(); + + let profiles: Vec<(String, String)> = std::fs::read_dir(profile_folder.path()) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "out")) + .map(|path| { + let pid = path.file_stem().unwrap().to_string_lossy().into_owned(); + (pid, std::fs::read_to_string(&path).unwrap()) + }) + .collect(); + + let declarations: Vec<(&str, &str)> = profiles + .iter() + .flat_map(|(pid, content)| { + content + .lines() + .filter_map(|line| line.strip_prefix("desc: Benchmark pid: ")) + .map(move |benchmark_pid| (pid.as_str(), benchmark_pid)) + }) + .collect(); + let [(harness_pid, benchmark_pid)] = declarations.as_slice() else { + panic!("expected exactly one benchmark pid declaration, got {declarations:?}"); + }; + + assert_ne!(harness_pid, benchmark_pid); + assert!( + profiles.iter().any(|(pid, _)| pid == benchmark_pid), + "benchmark pid {benchmark_pid} has no profile among {:?}", + profiles.iter().map(|(pid, _)| pid).collect::>() + ); +} + /// Prepends `prefix` to `var` and checks the benchmark still sees it. fn memory_forwards_path_like(var: &str, prefix: &str) { let value = match std::env::var(var).unwrap_or_default() {