Support python in profiling cli

.github/workflows/ci.yml

@@ -10,6 +10,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
+        with:
+          submodules: true
       - uses: moonrepo/setup-rust@v1
         with:
           components: rustfmt, clippy
@@ -23,6 +25,7 @@ jobs:
       - uses: actions/checkout@v3
         with:
           lfs: true
+          submodules: true
 
       - name: "Install rust-toolchain.toml"
         run: rustup toolchain install
@@ -45,6 +48,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
+        with:
+          submodules: true
       - uses: moonrepo/setup-rust@v1
       - name: Run tests
         run: cargo test -p exec-harness
@@ -55,6 +60,7 @@ jobs:
       - uses: actions/checkout@v3
         with:
           lfs: true
+          submodules: true
       - uses: moonrepo/setup-rust@v1
       - name: Install dependencies required for libbpf-sys (vendored feature)
         run: sudo apt-get update && sudo apt-get install -y autopoint bison flex
@@ -76,6 +82,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
+        with:
+          submodules: true
 
       - name: "Install rust-toolchain.toml"
         run: rustup toolchain install

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "crates/instrument-hooks-bindings/instrument-hooks"]
+	path = crates/instrument-hooks-bindings/instrument-hooks
+	url = https://github.com/CodSpeedHQ/instrument-hooks.git

Cargo.toml

@@ -86,7 +86,12 @@ rstest_reuse = "0.7.0"
 shell-quote = "0.7.2"
 
 [workspace]
-members = ["crates/runner-shared", "crates/memtrack", "crates/exec-harness"]
+members = [
+    "crates/runner-shared",
+    "crates/memtrack",
+    "crates/exec-harness",
+    "crates/instrument-hooks-bindings",
+]
 
 [workspace.dependencies]
 anyhow = "1.0"

crates/exec-harness/Cargo.toml

@@ -10,8 +10,8 @@ name = "exec-harness"
 path = "src/main.rs"
 
 [dependencies]
+instrument-hooks-bindings = { path = "../instrument-hooks-bindings" }
 anyhow = { workspace = true }
-codspeed = "4.2.0"
 log = { workspace = true }
 env_logger = { workspace = true }
 clap = { workspace = true }
@@ -23,7 +23,6 @@ tempfile = { workspace = true }
 object = { workspace = true }
 
 [build-dependencies]
-cargo_metadata = "0.19"
 cc = "1"
 
 [package.metadata.dist]

crates/exec-harness/build.rs

@@ -3,16 +3,9 @@
 //! 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 `codspeed` crate's
-//! `instrument-hooks` directory.
-//!
-//! # Environment Variables
-//!
-//! - `CODSPEED_INSTRUMENT_HOOKS_DIR`: Optional override for the instrument-hooks
-//!   source directory. If not set, the build script will locate it from the
-//!   `codspeed` crate in the cargo registry.
+//! The library is built using the `core.c` and headers from the `instrument-hooks-bindings`
+//! crate's `instrument-hooks` directory.
 
-use cargo_metadata::MetadataCommand;
 use std::env;
 use std::path::PathBuf;
 
@@ -25,7 +18,6 @@ struct PreloadConstants {
     /// Integration name reported to CodSpeed.
     integration_name: &'static str,
     /// Integration version reported to CodSpeed.
-    /// This should match the version of the `codspeed` crate dependency.
     integration_version: &'static str,
     /// Filename for the preload shared library.
     preload_lib_filename: &'static str,
@@ -58,11 +50,11 @@ fn main() {
     let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
 
     // Try to get the instrument-hooks directory from the environment variable first,
-    // otherwise locate it from the codspeed crate
-    let instrument_hooks_dir = match env::var("CODSPEED_INSTRUMENT_HOOKS_DIR") {
-        Ok(dir) => PathBuf::from(dir),
-        Err(_) => find_codspeed_instrument_hooks_dir(),
-    };
+    // 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 {
@@ -130,40 +122,12 @@ fn build_shared_library(paths: &PreloadBuildPaths, constants: &PreloadConstants)
     }
 }
 
-/// Find the instrument-hooks directory from the codspeed crate using cargo_metadata
-fn find_codspeed_instrument_hooks_dir() -> PathBuf {
-    let metadata = MetadataCommand::new()
-        .exec()
-        .expect("Failed to run cargo metadata");
-
-    // Find the codspeed package in the resolved dependencies
-    let codspeed_pkg = metadata
-        .packages
-        .iter()
-        .find(|p| p.name == "codspeed")
-        .expect("codspeed crate not found in dependencies");
-
-    let codspeed_dir = codspeed_pkg
-        .manifest_path
-        .parent()
-        .expect("Failed to get codspeed crate directory");
-
-    let instrument_hooks_dir = codspeed_dir.join("instrument-hooks");
-
-    if !instrument_hooks_dir.exists() {
-        panic!("instrument-hooks directory not found at {instrument_hooks_dir}");
-    }
-
-    instrument_hooks_dir.into_std_path_buf()
-}
-
 impl Default for PreloadConstants {
-    // TODO(COD-1736): Stop impersonating codspeed-rust 🥸
     fn default() -> Self {
         Self {
             uri_env: "CODSPEED_BENCH_URI",
-            integration_name: "codspeed-rust",
-            integration_version: "4.2.0",
+            integration_name: "exec-harness",
+            integration_version: env!("CARGO_PKG_VERSION"),
             preload_lib_filename: "libcodspeed_preload.so",
         }
     }
@@ -185,13 +149,13 @@ impl PreloadBuildPaths {
     fn check_sources_exist(&self) {
         if !self.core_c.exists() {
             panic!(
-                "core.c not found at {}. Make sure the codspeed crate is available.",
+                "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 {}. Make sure the codspeed crate is available.",
+                "includes directory not found at {}. instrument hooks submodule is available.",
                 self.includes_dir.display()
             );
         }

crates/exec-harness/preload/codspeed_preload.c

@@ -7,8 +7,6 @@
 //
 // Environment variables:
 //   CODSPEED_BENCH_URI - The benchmark URI to report (required)
-//   CODSPEED_PRELOAD_LOCK - Set by the first process to prevent child processes
-//                          from re-initializing instrumentation
 
 #include <stdlib.h>
 #include <unistd.h>
@@ -22,8 +20,6 @@
 #define RUNNING_ON_VALGRIND 0
 #endif
 
-static const char *LOCK_ENV = "CODSPEED_PRELOAD_LOCK";
-
 // 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"
@@ -54,14 +50,6 @@ __attribute__((constructor)) static void codspeed_preload_init(void) {
     return;
   }
 
-  // Check if another process already owns the instrumentation
-  if (getenv(LOCK_ENV)) {
-    return;
-  }
-
-  // Set the lock to prevent child processes from initializing
-  setenv(LOCK_ENV, "1", 1);
-
   g_bench_uri = getenv(URI_ENV);
   if (!g_bench_uri) {
     return;

crates/exec-harness/src/analysis/mod.rs

@@ -1,16 +1,19 @@
+use crate::constants::INTEGRATION_NAME;
+use crate::constants::INTEGRATION_VERSION;
 use crate::prelude::*;
 
 use crate::BenchmarkCommand;
 use crate::constants;
 use crate::uri;
-use codspeed::instrument_hooks::InstrumentHooks;
+use instrument_hooks_bindings::InstrumentHooks;
+use std::path::PathBuf;
 use std::process::Command;
 
 mod ld_preload_check;
 mod preload_lib_file;
 
 pub fn perform(commands: Vec<BenchmarkCommand>) -> Result<()> {
-    let hooks = InstrumentHooks::instance();
+    let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION);
 
     for benchmark_cmd in commands {
         let name_and_uri = uri::generate_name_and_uri(&benchmark_cmd.name, &benchmark_cmd.command);
@@ -52,9 +55,15 @@ pub fn perform_with_valgrind(commands: Vec<BenchmarkCommand>) -> Result<()> {
         cmd.args(&benchmark_cmd.command[1..]);
         // Use LD_PRELOAD to inject instrumentation into the child process
         cmd.env("LD_PRELOAD", preload_lib_path);
+        // Make sure python processes output perf maps. This is usually done by `pytest-codspeed`
+        cmd.env("PYTHONPERFSUPPORT", "1");
         cmd.env(constants::URI_ENV, &name_and_uri.uri);
 
-        let status = cmd.status().context("Failed to execute command")?;
+        let mut child = cmd.spawn().context("Failed to spawn command")?;
+
+        let status = child.wait().context("Failed to execute command")?;
+
+        bail_if_command_spawned_subprocesses_under_valgrind(child.id())?;
 
         if !status.success() {
             bail!("Command exited with non-zero status: {status}");
@@ -63,3 +72,48 @@ pub fn perform_with_valgrind(commands: Vec<BenchmarkCommand>) -> Result<()> {
 
     Ok(())
 }
+
+/// Checks if the benchmark process spawned subprocesses under valgrind by looking for <pid>.out
+/// files in the profile folder.
+///
+/// The presence of <pid>.out files where <pid> is greater than the benchmark process pid indicates
+/// that the benchmark process spawned subprocesses. This .out file will be almost empty, with a 0
+/// cost reported due to the disabled instrumentation.
+///
+/// We currently do not support measuring processes that spawn subprocesses under valgrind, because
+/// valgrind will not have its instrumentation in the new process.
+/// The LD_PRELOAD trick that we use to inject our instrumentation into the benchmark process only
+/// works for the first process.
+///
+/// TODO(COD-2163): Remove this once we support nested processes under valgrind
+fn bail_if_command_spawned_subprocesses_under_valgrind(pid: u32) -> Result<()> {
+    let Some(profile_folder) = std::env::var_os("CODSPEED_PROFILE_FOLDER") else {
+        debug!("CODSPEED_PROFILE_FOLDER is not set, skipping subprocess detection");
+        return Ok(());
+    };
+
+    let profile_folder = PathBuf::from(profile_folder);
+
+    // Bail if any <pid>.out where <pid> > pid of the benchmark process exists in the profile
+    // folder, which indicates that the benchmark process spawned subprocesses.
+    for entry in std::fs::read_dir(profile_folder)? {
+        let entry = entry?;
+        let file_name = entry.file_name();
+        let file_name = file_name.to_string_lossy();
+
+        if let Some(stripped) = file_name.strip_suffix(".out") {
+            if let Ok(subprocess_pid) = stripped.parse::<u32>() {
+                if subprocess_pid > pid {
+                    bail!(
+                        "The codspeed CLI in CPU Simulation mode does not support measuring processes that spawn other processes yet.\n\n\
+                         Please either:\n\
+                         - Use the walltime measurement mode, or\n\
+                         - Benchmark a process that does not create subprocesses"
+                    )
+                }
+            }
+        }
+    }
+
+    Ok(())
+}

crates/exec-harness/src/walltime/benchmark_loop.rs

@@ -1,7 +1,8 @@
 use super::ExecutionOptions;
 use super::config::RoundOrTime;
+use crate::constants::{INTEGRATION_NAME, INTEGRATION_VERSION};
 use crate::prelude::*;
-use codspeed::instrument_hooks::InstrumentHooks;
+use instrument_hooks_bindings::InstrumentHooks;
 use std::process::Command;
 use std::time::Duration;
 
@@ -11,7 +12,7 @@ pub fn run_rounds(
     config: &ExecutionOptions,
 ) -> Result<Vec<u128>> {
     let warmup_time_ns = config.warmup_time_ns;
-    let hooks = InstrumentHooks::instance();
+    let hooks = InstrumentHooks::instance(INTEGRATION_NAME, INTEGRATION_VERSION);
 
     let do_one_round = || -> Result<(u64, u64)> {
         let mut child = Command::new(&command[0])

crates/exec-harness/src/walltime/mod.rs

@@ -3,10 +3,13 @@ mod config;
 
 pub use config::ExecutionOptions;
 pub use config::WalltimeExecutionArgs;
+use runner_shared::walltime_results::Creator;
 use runner_shared::walltime_results::WalltimeBenchmark;
 pub use runner_shared::walltime_results::WalltimeResults;
 
 use crate::BenchmarkCommand;
+use crate::constants::INTEGRATION_NAME;
+use crate::constants::INTEGRATION_VERSION;
 use crate::prelude::*;
 use crate::uri::NameAndUri;
 use crate::uri::generate_name_and_uri;
@@ -42,8 +45,15 @@ pub fn perform(commands: Vec<BenchmarkCommand>) -> Result<()> {
         walltime_benchmarks.push(walltime_benchmark);
     }
 
-    let walltime_results = WalltimeResults::from_benchmarks(walltime_benchmarks)
-        .expect("Failed to create walltime results");
+    let walltime_results = WalltimeResults::new(
+        Creator {
+            name: INTEGRATION_NAME.to_string(),
+            version: INTEGRATION_VERSION.to_string(),
+            pid: std::process::id(),
+        },
+        walltime_benchmarks,
+    )
+    .expect("Failed to create walltime results");
 
     walltime_results
         .save_to_file(