use output dir, use hyperfine_iteration (WIP)

Introduce a mandatory `--output-dir` option when running benchmarks.
This will, in the future, be used to store benchmark configuration,
results and run artifacts.

Currently it will store configuration and the final run artifact, as the
changes needed in hyperfine to expose the run iteration are not
available yet. (see: sharkdp/hyperfine#807 )

Author: willcl-ark

Diff for: benchmark.yml

@@ -11,7 +11,6 @@ global:
     prepare: ./scripts/prepare.sh
     conclude: ./scripts/conclude.sh
     cleanup: ./scripts/cleanup.sh
-    export_json: results.json
     shell: /bin/bash
     show_output: true
 
@@ -50,7 +49,6 @@ benchmarks:
       # (bitcoin) [network] applied automatically. {dbcache} is an explicit
       # (additional) parameterisation from [parameter_lists] below.
       command: "bitcoind -dbcache={dbcache} -stopatheight=160001"
-      prepare: echo "don't snapshot"
       warmup: 0
       runs: 1
 

Diff for: scripts/cleanup.sh

@@ -3,20 +3,14 @@ set -e
 echo "Running cleanup.sh"
 
 # Scripts always recieve the same arguments from benchkit in the same order:
-#
-# pub struct ScriptArgs {
-#     pub binary: String,
-#     pub connect_address: String,
-#     pub network: String,
-#     pub snapshot_path: PathBuf,
-#     pub tmp_data_dir: PathBuf,
-# }
 
-if [ "$#" -ne 5 ]; then
-    echo "Error: Required arguments missing"
-    exit 1
-fi
-
-TMP_DATADIR="$5"
+# BINARY="$1"
+# CONNECT_ADDRESS="$2"
+# NETWORK="$3"
+# OUT_DIR="$4"
+# SNAPSHOT_PATH="$5"
+TMP_DATADIR="$6"
+# ITERATION="$7"
+# COMMIT="$8"
 
 rm -Rf "${TMP_DATADIR:?}"/*

Diff for: scripts/conclude.sh

@@ -3,29 +3,25 @@ set -e
 echo "Running conclude.sh"
 
 # Scripts always recieve the same arguments from benchkit in the same order:
-#
-# pub struct ScriptArgs {
-#     pub binary: String,
-#     pub connect_address: String,
-#     pub network: String,
-#     pub snapshot_path: PathBuf,
-#     pub tmp_data_dir: PathBuf,
-# }
-
-if [ "$#" -ne 5 ]; then
-    echo "Error: Required arguments missing"
-    exit 1
-fi
 
 # BINARY="$1"
 # CONNECT_ADDRESS="$2"
-# NETWORK="$3"
-# SNAPSHOT_PATH="$4"
-TMP_DATADIR="$5"
-# echo "TMP_DATADIR: ${TMP_DATADIR}"
+NETWORK="$3"
+OUT_DIR="$4"
+# SNAPSHOT_PATH="$5"
+TMP_DATADIR="$6"
+# ITERATION="$7"
+COMMIT="$8"
 
-# Next we move datadir files to the outdir
-# mv "$5"/debug.log "$2"/
+# Move datadir files to the outdir
+echo "Moving debug.log to $OUT_DIR/$COMMIT"
+mkdir -p "$OUT_DIR"/"$COMMIT"
+# TODO: include $ITERATION in this filepath
+if [ "$NETWORK" = "mainnet" ]; then
+    mv "$TMP_DATADIR"/debug.log "$OUT_DIR"/"$COMMIT"/
+else
+    mv "$TMP_DATADIR/$NETWORK/debug.log" "$OUT_DIR"/"$COMMIT"/
+fi
 
 echo "Cleaning datadir contents from ${TMP_DATADIR}"
 rm -Rf "${TMP_DATADIR:?}"/*

Diff for: scripts/prepare.sh

@@ -3,30 +3,15 @@ set -e
 echo "Running prepare.sh"
 
 # Scripts always recieve the same arguments from benchkit in the same order:
-#
-# pub struct ScriptArgs {
-#     pub binary: String,
-#     pub connect_address: String,
-#     pub network: String,
-#     pub snapshot_path: PathBuf,
-#     pub tmp_data_dir: PathBuf,
-# }
-
-if [ "$#" -ne 5 ]; then
-    echo "Error: Required arguments missing"
-    exit 1
-fi
 
 BINARY="$1"
 CONNECT_ADDRESS="$2"
 NETWORK="$3"
-SNAPSHOT_PATH="$4"
-TMP_DATADIR="$5"
-# echo "BINARY: ${BINARY}"
-# echo "CONNECT_ADDRESS: ${CONNECT_ADDRESS}"
-# echo "NETWORK: ${NETWORK}"
-# echo "SNAPSHOT_PATH: ${SNAPSHOT_PATH}"
-# echo "TMP_DATADIR: ${TMP_DATADIR}"
+# OUT_DIR="$4"
+SNAPSHOT_PATH="$5"
+TMP_DATADIR="$6"
+# ITERATION="$7"
+# COMMIT="$8"
 
 mkdir -p "${TMP_DATADIR}"
 rm -Rf "${TMP_DATADIR:?}/*"

Diff for: scripts/setup.sh

@@ -3,26 +3,15 @@ set -e
 echo "Running setup.sh"
 
 # Scripts always recieve the same arguments from benchkit in the same order:
-#
-# pub struct ScriptArgs {
-#     pub binary: String,
-#     pub connect_address: String,
-#     pub network: String,
-#     pub snapshot_path: PathBuf,
-#     pub tmp_data_dir: PathBuf,
-# }
-
-if [ "$#" -ne 5 ]; then
-    echo "Error: Required arguments missing"
-    exit 1
-fi
 
 # BINARY="$1"
 # CONNECT_ADDRESS="$2"
 # NETWORK="$3"
-# SNAPSHOT_PATH="$4"
-TMP_DATADIR="$5"
-# echo "TMP_DATADIR: ${TMP_DATADIR}"
+# OUT_DIR="$4"
+# SNAPSHOT_PATH="$5"
+TMP_DATADIR="$6"
+# ITERATION="$7"
+# COMMIT="$8"
 
 echo "Creating datadir ${TMP_DATADIR} with mkdir -p"
 mkdir -p "${TMP_DATADIR}"

Diff for: src/benchmarks/hooks.rs

@@ -9,6 +9,7 @@ pub struct ScriptArgs {
     pub binary: String,
     pub connect_address: String,
     pub network: String,
+    pub out_dir: PathBuf,
     pub snapshot_path: PathBuf,
     pub tmp_data_dir: PathBuf,
 }
@@ -39,13 +40,15 @@ impl HookManager {
         for hook_type in hook_types.iter() {
             if let Some(value) = options.get_mut(*hook_type) {
                 if let Some(script) = value.as_str() {
-                    // Construct the new script command with arguments in a fixed order
+                    // Construct the new script command with arguments in a fixed order + the
+                    // hyperfine iteration counter
                     let new_script = format!(
-                        "{} {} {} {} {} {}",
+                        "{} {} {} {} {} {} {} \"$HYPERFINE_ITERATION\" {{commit}}",
                         script,
                         script_args.binary,
                         script_args.connect_address,
                         script_args.network,
+                        script_args.out_dir.display(),
                         script_args.snapshot_path.display(),
                         script_args.tmp_data_dir.display(),
                     );

Diff for: src/benchmarks/mod.rs

@@ -3,7 +3,7 @@ use clap::ValueEnum;
 use log::{debug, info};
 use serde_json::{json, Value};
 use std::collections::HashMap;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::Command;
 
 mod build;
@@ -20,36 +20,57 @@ use crate::types::Network;
 
 pub struct Runner {
     config: GlobalConfig,
+    out_dir: PathBuf,
 }
 
 impl Runner {
-    pub fn new(config: GlobalConfig) -> Result<Self> {
-        Ok(Self { config })
+    pub fn new(
+        config: GlobalConfig,
+        out_dir: PathBuf,
+        app_config: &PathBuf,
+        bench_config: &PathBuf,
+    ) -> Result<Self> {
+        // Configure stage
+        debug!("Using output directory: {}", out_dir.display());
+        std::fs::create_dir_all(&out_dir)?;
+        if std::fs::read_dir(&out_dir)?.next().is_some() {
+            anyhow::bail!(
+                "Output directory '{}' is not empty. Please clear it before running benchmarks",
+                out_dir.display()
+            );
+        }
+        let app_config_name = app_config.file_name().unwrap_or_default();
+        let bench_config_name = bench_config.file_name().unwrap_or_default();
+        std::fs::copy(app_config, out_dir.join(app_config_name))?;
+        std::fs::copy(bench_config, out_dir.join(bench_config_name))?;
+
+        Ok(Self { config, out_dir })
     }
 
-    pub async fn run(&self) -> Result<()> {
-        for bench in &self.config.bench.benchmarks {
+    pub async fn run(&self, name: Option<&str>) -> Result<()> {
+        let benchmarks = match name {
+            Some(n) => {
+                let bench = self
+                    .config
+                    .bench
+                    .benchmarks
+                    .iter()
+                    .find(|b| b.name == n)
+                    .with_context(|| format!("Benchmark not found: {}", n))?;
+                vec![bench]
+            }
+            None => self.config.bench.benchmarks.iter().collect(),
+        };
+
+        for bench in benchmarks {
+            // TODO: Remove this check to enable runs without AssumeUTXO
             self.check_snapshot(bench, &self.config.app.snapshot_dir)
                 .await?;
             self.run_benchmark(bench).await?;
         }
         Ok(())
     }
 
-    pub async fn run_single(&self, name: &str) -> Result<()> {
-        let bench = self
-            .config
-            .bench
-            .benchmarks
-            .iter()
-            .find(|b| b.name == name)
-            .with_context(|| format!("Benchmark not found: {}", name))?;
-
-        self.check_snapshot(bench, &self.config.app.snapshot_dir)
-            .await?;
-        self.run_benchmark(bench).await
-    }
-
     async fn check_snapshot(&self, bench: &SingleConfig, snapshot_dir: &Path) -> Result<()> {
         // Check if we have the correct snapshot
         let network = Network::from_str(&bench.network, true)
@@ -82,9 +103,6 @@ This can be downloaded with `benchkit snapshot download {}`",
         }
         merged_hyperfine.extend(bench.hyperfine.clone());
 
-        // Create a temporary output directory
-        // let out_dir = tempfile::TempDir::new()?.into_path();
-
         // Update command to use full binary path and apply chain= param
         if let Some(Value::String(command)) = merged_hyperfine.get_mut("command") {
             let new_command = command.replace(
@@ -116,6 +134,7 @@ This can be downloaded with `benchkit snapshot download {}`",
         let script_args = ScriptArgs {
             binary: format!("{}/bitcoind-{{commit}}", self.config.app.bin_dir.display()),
             connect_address: bench.connect.clone().unwrap_or_default(),
+            out_dir: self.out_dir.clone(),
             network: bench.network.clone(),
             snapshot_path,
             tmp_data_dir: self.config.bench.global.tmp_data_dir.clone(),
@@ -127,11 +146,11 @@ This can be downloaded with `benchkit snapshot download {}`",
             .with_context(|| "Failed to add hyperfine script hooks")?;
 
         // Add commits to parameter-lists if not already present
-        let param_lists = merged_hyperfine
+        let parameter_lists = merged_hyperfine
             .entry("parameter_lists".to_string())
             .or_insert_with(|| Value::Array(Vec::new()));
 
-        if let Value::Array(lists) = param_lists {
+        if let Value::Array(lists) = parameter_lists {
             // Check if commits parameter list already exists
             if !lists
                 .iter()
@@ -146,31 +165,27 @@ This can be downloaded with `benchkit snapshot download {}`",
             }
         }
 
-        // Check the export path before running hyperfine
-        let export_path = merged_hyperfine
-            .get("export_json")
-            .and_then(Value::as_str)
-            .with_context(|| {
-                format!(
-                    "Missing required 'export_json' field in benchmark '{}'",
-                    bench.name
-                )
-            })?;
+        // Hardcode the export path to the top-level of the out_dir
+        let export_path = self.out_dir.join("results.json");
+        merged_hyperfine.insert(
+            "export_json".to_string(),
+            Value::String(export_path.to_string_lossy().into_owned()),
+        );
 
         // Run hyperfine with merged options
         self.run_hyperfine(bench, &merged_hyperfine)?;
 
         // Check for and process results
-        if !Path::new(export_path).exists() {
+        if !Path::new(&export_path).exists() {
             anyhow::bail!(
                 "Expected JSON results file not found at '{}' for benchmark '{}'",
-                export_path,
+                export_path.display(),
                 bench.name
             );
         }
 
-        let results_json = std::fs::read_to_string(export_path)
-            .with_context(|| format!("Failed to read results file: {}", export_path))?;
+        let results_json = std::fs::read_to_string(&export_path)
+            .with_context(|| format!("Failed to read results file: {}", export_path.display()))?;
 
         // Store results in database
         crate::database::store_results(
@@ -180,11 +195,6 @@ This can be downloaded with `benchkit snapshot download {}`",
             self.config.bench.run_id,
         )
         .await?;
-
-        // Cleanup
-        std::fs::remove_file(export_path)
-            .with_context(|| format!("Failed to remove results file: {}", export_path))?;
-
         Ok(())
     }
 

Diff for: src/config.rs

@@ -52,8 +52,8 @@ pub fn load_app_config(app_config_path: &PathBuf) -> Result<AppConfig> {
 
     // Resolve relative paths to absolute paths and create directories
     for path in [
-        &mut config.home_dir,
         &mut config.bin_dir,
+        &mut config.home_dir,
         &mut config.snapshot_dir,
     ]
     .iter_mut()