From 5d3ac4edf8d73ae1cc6df30988e19ee2ae746d5c Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Fri, 12 Jun 2026 09:42:53 +0100 Subject: [PATCH 1/9] Add hyperlight-ci crate for benchmark orchestration and reporting Introduce a new internal tooling crate (hyperlight-ci) that provides: - bench subcommand: Runs criterion benchmarks in parallel via criterion-swarm. Features include: - Configurable parallelism (-j N, defaults to all P-cores) - Configurable output modes (spinner, stream, summary) - Support for pre-built binaries (--binary) to skip rebuilds - Trailing args forwarded to criterion (filter, --exact, etc.) - bench-report subcommand: Generates markdown comparison tables from criterion's target/criterion/ JSON output via criterion-markdown. Features include: - Benchmark discovery via criterion-swarm - Optional allowlist filtering via --binary or trailing args - Output to stdout This replaces ad-hoc benchmark scripting with a unified tool suitable for both local development and CI report generation. Signed-off-by: Jorge Prendes --- Cargo.lock | 226 ++++++++++++++++++++++++-- Cargo.toml | 1 + src/hyperlight_ci/Cargo.toml | 19 +++ src/hyperlight_ci/src/bench.rs | 125 ++++++++++++++ src/hyperlight_ci/src/bench_report.rs | 74 +++++++++ src/hyperlight_ci/src/main.rs | 33 ++++ 6 files changed, 465 insertions(+), 13 deletions(-) create mode 100644 src/hyperlight_ci/Cargo.toml create mode 100644 src/hyperlight_ci/src/bench.rs create mode 100644 src/hyperlight_ci/src/bench_report.rs create mode 100644 src/hyperlight_ci/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index d869fdedcb..3fd605cd16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,6 +57,16 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" +[[package]] +name = "ansi-replace" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f8b155ab93213f41c886d3a46e335258428e52c7cf868e25cf099d50274496d" +dependencies = [ + "regex", + "stable-pattern", +] + [[package]] name = "anstream" version = "1.0.0" @@ -93,7 +103,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -104,7 +114,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -498,6 +508,19 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "console" +version = "0.15.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "054ccb5b10f9f2cbf51eb355ca1d05c2d279ce1804688d0db74b4733a5aeafd8" +dependencies = [ + "encode_unicode", + "libc", + "once_cell", + "unicode-width", + "windows-sys 0.59.0", +] + [[package]] name = "const-oid" version = "0.10.2" @@ -592,6 +615,19 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "cpu-pin" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb5bc1be026f7f066429ce0611e23a341db91b91ed701de4a1432d01d3ed1105" +dependencies = [ + "libc", + "mach2", + "once_cell", + "tokio", + "windows", +] + [[package]] name = "cpufeatures" version = "0.3.1" @@ -635,6 +671,17 @@ dependencies = [ "walkdir", ] +[[package]] +name = "criterion-markdown" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72741fa695d01bfab5c47a083ef370db8235c90aee6033136ce49aa7dcd2a413" +dependencies = [ + "anyhow", + "serde", + "serde_json", +] + [[package]] name = "criterion-plot" version = "0.8.2" @@ -645,6 +692,23 @@ dependencies = [ "itertools 0.13.0", ] +[[package]] +name = "criterion-swarm" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82693ab3e80479ca52770515f3837028800e365aac48a83edb86f920a6783232" +dependencies = [ + "ansi-replace", + "anyhow", + "clap", + "cpu-pin", + "indicatif", + "regex", + "serde_json", + "simple-pool", + "tokio", +] + [[package]] name = "crossbeam-channel" version = "0.5.17" @@ -840,7 +904,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -886,6 +950,12 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "encode_unicode" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" + [[package]] name = "endian-type" version = "0.1.2" @@ -928,7 +998,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1275,7 +1345,7 @@ dependencies = [ "gobject-sys", "libc", "system-deps", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -1546,6 +1616,19 @@ dependencies = [ "tracing", ] +[[package]] +name = "hyperlight-ci" +version = "0.0.0" +dependencies = [ + "anyhow", + "clap", + "criterion-markdown", + "criterion-swarm", + "serde", + "serde_json", + "tokio", +] + [[package]] name = "hyperlight-common" version = "0.17.0" @@ -1731,7 +1814,7 @@ dependencies = [ "vmm-sys-util", "windows", "windows-result", - "windows-sys", + "windows-sys 0.61.2", "windows-version", ] @@ -1933,6 +2016,19 @@ dependencies = [ "serde_core", ] +[[package]] +name = "indicatif" +version = "0.17.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "183b3088984b400f4cfac3620d5e076c84da5364016b4f49473de574b2586235" +dependencies = [ + "console", + "number_prefix", + "portable-atomic", + "unicode-width", + "web-time", +] + [[package]] name = "ipnet" version = "2.12.2" @@ -2391,7 +2487,7 @@ checksum = "4b18443e9c262bfe8fa82f51666e2642c53393f7e5c27b3e1aeab922cff5b9d8" dependencies = [ "libc", "wasi", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -2455,7 +2551,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -2488,6 +2584,12 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "number_prefix" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" + [[package]] name = "object" version = "0.40.0" @@ -2499,6 +2601,12 @@ dependencies = [ "ruzstd", ] +[[package]] +name = "object-id" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c587bd1cd63959a8520442afc0f92a875d83deea175c7b48dd9f104a2c5070a9" + [[package]] name = "oci-spec" version = "0.10.0" @@ -3347,7 +3455,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -3558,6 +3666,16 @@ version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a219298ac11a56ea9a6d2120044824d6f01aeb034955e7af7bc16858527deea" +[[package]] +name = "simple-pool" +version = "0.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "073382259dbeb56c3eaab04a1d330459f6490d1e518b2a8ee441c8bd00dbc092" +dependencies = [ + "object-id", + "parking_lot", +] + [[package]] name = "sketches-ddsketch" version = "0.3.1" @@ -3583,7 +3701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" dependencies = [ "libc", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -3601,6 +3719,15 @@ dependencies = [ "lock_api", ] +[[package]] +name = "stable-pattern" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4564168c00635f88eaed410d5efa8131afa8d8699a612c80c455a0ba05c21045" +dependencies = [ + "memchr", +] + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -3702,7 +3829,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -3777,7 +3904,7 @@ dependencies = [ "signal-hook-registry", "socket2", "tokio-macros", - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -4459,7 +4586,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys", + "windows-sys 0.61.2", ] [[package]] @@ -4569,6 +4696,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" +dependencies = [ + "windows-targets", +] + [[package]] name = "windows-sys" version = "0.61.2" @@ -4578,6 +4714,22 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + [[package]] name = "windows-threading" version = "0.2.1" @@ -4596,6 +4748,54 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + [[package]] name = "winnow" version = "0.7.15" diff --git a/Cargo.toml b/Cargo.toml index 2660825a04..0b1c83201f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ default-members = [ "src/hyperlight_testing", ] members = [ + "src/hyperlight_ci", "src/hyperlight_common", "src/hyperlight_guest", "src/hyperlight_host", diff --git a/src/hyperlight_ci/Cargo.toml b/src/hyperlight_ci/Cargo.toml new file mode 100644 index 0000000000..9243145387 --- /dev/null +++ b/src/hyperlight_ci/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "hyperlight-ci" +edition = "2021" +# fields intentionally not set, to avoid accidentally publishing this crate to crates.io +description = """ +Hyperlight's CI and development tools. +""" + +[lints] +workspace = true + +[dependencies] +anyhow = "1" +clap = { version = "4.6.1", features = ["derive"] } +criterion-markdown = "0.1.2" +criterion-swarm = "0.2" +serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" +tokio = { version = "1", features = ["rt", "macros"] } \ No newline at end of file diff --git a/src/hyperlight_ci/src/bench.rs b/src/hyperlight_ci/src/bench.rs new file mode 100644 index 0000000000..3ab0965665 --- /dev/null +++ b/src/hyperlight_ci/src/bench.rs @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! The `bench` subcommand: runs criterion benchmarks in parallel via criterion-swarm. + +use std::io::IsTerminal; +use std::path::PathBuf; + +use anyhow::Context; +use criterion_swarm::{CriterionSwarm, OutputMode}; + +/// An output mode flag for `--build-output` / `--benchmarks-output`. +#[derive(Clone, Debug)] +pub(crate) struct OutputModeFlags(OutputMode); + +impl OutputModeFlags { + /// Parse a single token into an `OutputMode` flag. + fn parse_one(s: &str) -> Result { + match s.trim().to_ascii_lowercase().as_str() { + "spinner" => Ok(OutputMode::SPINNER), + "stream" => Ok(OutputMode::STREAM), + "summary" => Ok(OutputMode::SUMMARY), + "none" | "silent" => Ok(OutputMode::SILENT), + other => Err(format!( + "unknown output mode `{other}` (expected: spinner, stream, summary, none, silent)" + )), + } + } +} + +impl std::str::FromStr for OutputModeFlags { + type Err = String; + fn from_str(s: &str) -> Result { + let mut mode = OutputMode::SILENT; + for part in s.split(',') { + mode |= Self::parse_one(part)?; + } + Ok(Self(mode)) + } +} + +/// Merge a `Vec` into a single `OutputMode` by OR-ing them together. +fn merge_output_modes(flags: &[OutputModeFlags]) -> OutputMode { + flags.iter().fold(OutputMode::SILENT, |acc, f| acc | f.0) +} + +/// Command-line arguments for the `bench` subcommand. +#[derive(clap::Args)] +pub struct BenchArgs { + /// Pre-built benchmark binary to use (skip build step; can be specified multiple times) + #[arg(long)] + pub binary: Vec, + + /// Number of benchmarks to run in parallel (0 = all P-cores, default: 0) + #[arg(long, short, default_value_t = 0)] + pub jobs: usize, + + /// Build output mode (comma-separated or repeated): spinner, stream, summary, none + #[arg(long, value_delimiter = ',')] + pub build_output: Vec, + + /// Benchmarks output mode (comma-separated or repeated): spinner, stream, summary, none + #[arg(long, value_delimiter = ',')] + pub benchmarks_output: Vec, + + /// Additional features to pass to cargo when building benchmarks (can be specified multiple times) + #[arg(short = 'F', long)] + pub features: Vec, + + /// Additional arguments to forward to criterion benchmarks + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub bench_args: Vec, +} + +pub async fn run(mut args: BenchArgs) -> anyhow::Result<()> { + let mut swarm = CriterionSwarm::builder().jobs(args.jobs); + + if !args.binary.is_empty() { + swarm = swarm.binaries(args.binary); + } + + if !args.features.is_empty() { + swarm = swarm.build_args(["--features".to_string(), args.features.join(",")]); + } + + for arg in args.bench_args { + swarm = swarm.bench_arg(arg); + } + + if args.build_output.is_empty() { + let mode = if std::io::stderr().is_terminal() { + OutputMode::SPINNER | OutputMode::SUMMARY + } else { + OutputMode::STREAM | OutputMode::SUMMARY + }; + args.build_output.push(OutputModeFlags(mode)); + } + + if args.benchmarks_output.is_empty() { + let mode = if std::io::stderr().is_terminal() { + OutputMode::SPINNER | OutputMode::STREAM | OutputMode::SUMMARY + } else { + OutputMode::STREAM | OutputMode::SUMMARY + }; + args.benchmarks_output.push(OutputModeFlags(mode)); + } + + let build_mode = merge_output_modes(&args.build_output); + let bench_mode = merge_output_modes(&args.benchmarks_output); + swarm = swarm.output( + criterion_swarm::ProgressReporter::new() + .build(build_mode) + .benchmarks(bench_mode), + ); + + let swarm = swarm + .prepare() + .await + .context("Failed to prepare criterion swarm")?; + if bench_mode == (bench_mode | OutputMode::SUMMARY) { + let total = swarm.benchmarks().len(); + let jobs = swarm.jobs().min(total); + println!("Running {total} benchmarks with parallelism {jobs}"); + } + swarm.run().await.context("Failed to run criterion swarm") +} diff --git a/src/hyperlight_ci/src/bench_report.rs b/src/hyperlight_ci/src/bench_report.rs new file mode 100644 index 0000000000..1410b94dae --- /dev/null +++ b/src/hyperlight_ci/src/bench_report.rs @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! The `bench-report` subcommand: generates a markdown table from existing +//! criterion benchmark results in `target/criterion/`. + +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use clap::Args; +use criterion_swarm::{CriterionSwarm, NoopReporter}; + +/// Command-line arguments for the `bench-report` subcommand. +#[derive(Args)] +pub struct BenchReportArgs { + /// Benchmark binary to list benchmarks from (can be specified multiple times). + /// When provided, only benchmarks available in these binaries are included. + #[arg(long)] + pub binary: Vec, + + /// Path to the criterion output directory + #[arg(long, default_value = "target/criterion")] + pub criterion_dir: PathBuf, + + /// Wrap the output in a collapsible
tag with the given summary text. + #[arg(long)] + pub collapsible: Option, + + /// Additional arguments to forward to criterion benchmarks (e.g. filter, --exact) + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub bench_args: Vec, +} + +/// Entry point for the bench-report subcommand. +pub async fn run(args: BenchReportArgs) -> Result<()> { + let allowlist = build_allowlist(&args).await?; + + let options = criterion_markdown::RenderOptions { + collapsible: args.collapsible, + }; + let markdown = + criterion_markdown::render_with_options(&args.criterion_dir, &allowlist, &options)?; + + print!("{markdown}"); + + Ok(()) +} + +/// Builds an allowlist of benchmark full_ids by discovering benchmarks via CriterionSwarm. +/// +/// All trailing arguments (filter, --exact, etc.) are forwarded as bench args +/// to CriterionSwarm so it handles filtering during discovery. +async fn build_allowlist(args: &BenchReportArgs) -> Result> { + let mut swarm = CriterionSwarm::builder(); + + if !args.binary.is_empty() { + swarm = swarm.binaries(&args.binary); + } + + for arg in &args.bench_args { + swarm = swarm.bench_arg(arg); + } + + let discovered = swarm + .output(NoopReporter) + .prepare() + .await + .context("Failed to discover benchmarks")?; + + Ok(discovered + .benchmarks() + .into_iter() + .map(str::to_string) + .collect()) +} diff --git a/src/hyperlight_ci/src/main.rs b/src/hyperlight_ci/src/main.rs new file mode 100644 index 0000000000..76f9135bac --- /dev/null +++ b/src/hyperlight_ci/src/main.rs @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +mod bench; +mod bench_report; + +use clap::{Parser, Subcommand}; + +#[derive(Parser)] +#[command( + name = "hyperlight-ci", + about = "Hyperlight's CI and development tools" +)] +struct Cli { + #[command(subcommand)] + command: Commands, +} + +#[derive(Subcommand)] +enum Commands { + /// Run benchmarks using the benchmark binary directly + Bench(bench::BenchArgs), + /// Generate a markdown table from existing criterion benchmark results + BenchReport(bench_report::BenchReportArgs), +} + +#[tokio::main(flavor = "current_thread")] +async fn main() -> anyhow::Result<()> { + let cli = Cli::parse(); + match cli.command { + Commands::Bench(args) => bench::run(args).await, + Commands::BenchReport(args) => bench_report::run(args).await, + } +} From a2465a6b826288b9ebc6221c9b3f5025b6732b65 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Fri, 12 Jun 2026 09:46:50 +0100 Subject: [PATCH 2/9] Integrate hyperlight-ci into CI workflows and Just recipes - Add cargo alias (`cargo ci`) for convenient hyperlight-ci invocation - Update dep_benchmarks workflow to use `cargo ci bench` and generate a markdown report via `cargo ci bench-report`, posting results as a PR comment per hypervisor/cpu matrix entry - Add benchmarks job to ValidatePullRequest workflow with hypervisor and cpu matrix, gated behind docs-only and build-guests checks - Grant pull-requests: write permission for PR comment posting - Simplify Justfile bench recipes to delegate to `cargo ci bench` - Update benchmarking docs to reflect the new workflow Signed-off-by: Jorge Prendes --- .cargo/config.toml | 3 ++ .github/hyperlight-bot.yml | 8 ++++ .github/workflows/ValidatePullRequest.yml | 56 +++++++++++++++++++++++ .github/workflows/dep_benchmarks.yml | 11 ++++- Justfile | 6 +-- 5 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 .github/hyperlight-bot.yml diff --git a/.cargo/config.toml b/.cargo/config.toml index b1c8f87b3c..0c3c4d34a0 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,2 +1,5 @@ [target.'cfg(target_os = "macos")'] runner = "dev/macos-sign-and-run.sh" + +[alias] # command aliases +ci = ["run", "--quiet", "--package=hyperlight-ci", "--"] diff --git a/.github/hyperlight-bot.yml b/.github/hyperlight-bot.yml new file mode 100644 index 0000000000..758fc3e550 --- /dev/null +++ b/.github/hyperlight-bot.yml @@ -0,0 +1,8 @@ +# Configuration for the hyperlight-gh-bot GitHub App. +# See: https://github.com/jprendes/hyperlight-gh-bot + +# Name of the artifact containing the comment body. +artifact_name: "pr-comment" + +# Regex matched against the job name to filter which jobs trigger the bot. +job_filter: "post-benchmark-comment" diff --git a/.github/workflows/ValidatePullRequest.yml b/.github/workflows/ValidatePullRequest.yml index 4c16a78b71..7135454feb 100644 --- a/.github/workflows/ValidatePullRequest.yml +++ b/.github/workflows/ValidatePullRequest.yml @@ -226,6 +226,61 @@ jobs: arch: ${{ matrix.arch }} target: ${{ matrix.target }} + # Run benchmarks and post results as PR comment + benchmarks: + needs: + - docs-pr + - build-guests + # Required because update-guest-locks is skipped on non-dependabot PRs, + # and a skipped dependency transitively skips all downstream jobs. + # See: https://github.com/actions/runner/issues/2205 + if: ${{ !cancelled() && !failure() }} + strategy: + fail-fast: false + matrix: + hypervisor: ['hyperv-ws2025', mshv3, kvm] + cpu: [amd, intel] + uses: ./.github/workflows/dep_benchmarks.yml + secrets: inherit + with: + docs_only: ${{ needs.docs-pr.outputs.docs-only }} + hypervisor: ${{ matrix.hypervisor }} + cpu: ${{ matrix.cpu }} + + # Combine benchmark reports into a single artifact for the hyperlight-gh-bot + # to post as a PR comment. Only runs for PRs (not merge groups) with code changes. + benchmark-comment: + name: post-benchmark-comment + needs: + - docs-pr + - benchmarks + if: ${{ !cancelled() && !failure() && needs.docs-pr.outputs.docs-only == 'false' && github.event_name == 'pull_request' }} + runs-on: ubuntu-latest + steps: + - name: Download benchmark reports + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: benchmark-report_* + path: reports/ + + - name: Combine benchmark reports + run: | + echo '## Benchmark Results' > pr-comment.md + echo '' >> pr-comment.md + for f in reports/benchmark-report_*/benchmark.md; do + [ -f "$f" ] || continue + cat "$f" >> pr-comment.md + echo '' >> pr-comment.md + done + + - name: Upload PR comment artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: pr-comment + path: pr-comment.md + if-no-files-found: warn + retention-days: 1 + spelling: name: spell check with typos runs-on: ubuntu-latest @@ -254,6 +309,7 @@ jobs: - build-test - run-examples - fuzzing + - benchmarks - spelling - license-headers if: always() diff --git a/.github/workflows/dep_benchmarks.yml b/.github/workflows/dep_benchmarks.yml index dc81ed9705..445083733e 100644 --- a/.github/workflows/dep_benchmarks.yml +++ b/.github/workflows/dep_benchmarks.yml @@ -60,7 +60,6 @@ on: required: false type: number default: 5 - env: CARGO_TERM_COLOR: always RUST_BACKTRACE: full @@ -143,6 +142,16 @@ jobs: - name: Run benchmarks run: just bench-ci main + - name: Create benchmarks report + run: cargo ci bench-report --collapsible '${{ inputs.hypervisor }} / ${{ inputs.cpu }} (${{ runner.os }})' > target/criterion/benchmark.md + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: benchmark-report_${{ runner.os }}_${{ inputs.hypervisor }}_${{ inputs.cpu }} + path: target/criterion/benchmark.md + if-no-files-found: error + retention-days: 1 + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: benchmarks_${{ runner.os }}_${{ inputs.hypervisor }}_${{ inputs.cpu_vendor }} diff --git a/Justfile b/Justfile index 9d42e66cb0..3610968d2a 100644 --- a/Justfile +++ b/Justfile @@ -436,12 +436,10 @@ bench-download os hypervisor cpu_vendor tag="": # Warning: compares to and then OVERWRITES the given baseline bench-ci baseline features="": - @# Benchmarks are always run with release builds for meaningful results - cargo bench --profile=release {{ if features =="" {''} else { "--features " + features } }} -- --verbose --save-baseline {{ baseline }} + cargo ci bench {{ if features == "" {''} else { "--features " + features } }} --verbose --save-baseline {{ baseline }} bench features="": - @# Benchmarks are always run with release builds for meaningful results - cargo bench --profile=release {{ if features =="" {''} else { "--features " + features } }} -- --verbose + cargo ci bench {{ if features == "" {''} else { "--features " + features } }} --verbose ############### ### FUZZING ### From 9ed99db548548fe062bc183ac7a1f42ebcc347f7 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Mon, 21 Sep 2026 14:33:04 +0100 Subject: [PATCH 3/9] fix(ci): pass cpu_vendor and arch to dep_benchmarks The reusable benchmark workflow declares `cpu_vendor` and a required `arch`. The pull request matrix supplies both, and the report label and artifact name use `cpu_vendor`. Signed-off-by: Jorge Prendes --- .github/workflows/ValidatePullRequest.yml | 6 ++++-- .github/workflows/dep_benchmarks.yml | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ValidatePullRequest.yml b/.github/workflows/ValidatePullRequest.yml index 7135454feb..227a6b102b 100644 --- a/.github/workflows/ValidatePullRequest.yml +++ b/.github/workflows/ValidatePullRequest.yml @@ -238,14 +238,16 @@ jobs: strategy: fail-fast: false matrix: + arch: [X64] hypervisor: ['hyperv-ws2025', mshv3, kvm] - cpu: [amd, intel] + cpu_vendor: [amd, intel] uses: ./.github/workflows/dep_benchmarks.yml secrets: inherit with: docs_only: ${{ needs.docs-pr.outputs.docs-only }} hypervisor: ${{ matrix.hypervisor }} - cpu: ${{ matrix.cpu }} + cpu_vendor: ${{ matrix.cpu_vendor }} + arch: ${{ matrix.arch }} # Combine benchmark reports into a single artifact for the hyperlight-gh-bot # to post as a PR comment. Only runs for PRs (not merge groups) with code changes. diff --git a/.github/workflows/dep_benchmarks.yml b/.github/workflows/dep_benchmarks.yml index 445083733e..8a7cda1014 100644 --- a/.github/workflows/dep_benchmarks.yml +++ b/.github/workflows/dep_benchmarks.yml @@ -143,11 +143,11 @@ jobs: run: just bench-ci main - name: Create benchmarks report - run: cargo ci bench-report --collapsible '${{ inputs.hypervisor }} / ${{ inputs.cpu }} (${{ runner.os }})' > target/criterion/benchmark.md + run: cargo ci bench-report --collapsible '${{ inputs.hypervisor }} / ${{ inputs.cpu_vendor }} (${{ runner.os }})' > target/criterion/benchmark.md - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: benchmark-report_${{ runner.os }}_${{ inputs.hypervisor }}_${{ inputs.cpu }} + name: benchmark-report_${{ runner.os }}_${{ inputs.hypervisor }}_${{ inputs.cpu_vendor }} path: target/criterion/benchmark.md if-no-files-found: error retention-days: 1 From b1338194f64d8868ee4159d088960fabf8c1e265 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Mon, 21 Sep 2026 17:54:47 +0100 Subject: [PATCH 4/9] feat(ci): filter benchmark reports through a config file `bench_report.toml` lists regular expressions matched against criterion benchmark ids. A benchmark is selected when it matches `allowlist` and no `denylist` entry, and an empty `allowlist` keeps everything the denylist does not exclude. Omitting both lists disables filtering. `cargo ci bench-report --config-file` reports the selection, and `cargo ci bench --config-file` runs it. CI keeps running every benchmark and filters at report time. Both subcommands apply the selection through `CriterionSwarm::retain`, added in criterion-swarm 0.2.1, so running and reporting cannot drift apart. An allowlist pattern that matches no benchmark fails, so a rename surfaces instead of dropping out of the comment silently. A denylist pattern matching nothing is accepted, because a benchmark may be absent on some platforms. Unknown keys are rejected so a stale key cannot silently disable filtering. The initial lists keep 47 of 116 benchmarks: those whose median drifted by at most 5% across five back-to-back runs on an idle machine, less the snapshot cold start and restore families. Signed-off-by: Jorge Prendes --- .github/workflows/dep_benchmarks.yml | 2 +- Cargo.lock | 6 +- bench_report.toml | 84 ++++++++++ src/hyperlight_ci/Cargo.toml | 6 +- src/hyperlight_ci/src/bench.rs | 24 ++- src/hyperlight_ci/src/bench_report.rs | 18 ++- src/hyperlight_ci/src/config.rs | 224 ++++++++++++++++++++++++++ src/hyperlight_ci/src/main.rs | 1 + 8 files changed, 355 insertions(+), 10 deletions(-) create mode 100644 bench_report.toml create mode 100644 src/hyperlight_ci/src/config.rs diff --git a/.github/workflows/dep_benchmarks.yml b/.github/workflows/dep_benchmarks.yml index 8a7cda1014..fcd624af6b 100644 --- a/.github/workflows/dep_benchmarks.yml +++ b/.github/workflows/dep_benchmarks.yml @@ -143,7 +143,7 @@ jobs: run: just bench-ci main - name: Create benchmarks report - run: cargo ci bench-report --collapsible '${{ inputs.hypervisor }} / ${{ inputs.cpu_vendor }} (${{ runner.os }})' > target/criterion/benchmark.md + run: cargo ci bench-report --config-file bench_report.toml --collapsible '${{ inputs.hypervisor }} / ${{ inputs.cpu_vendor }} (${{ runner.os }})' > target/criterion/benchmark.md - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/Cargo.lock b/Cargo.lock index 3fd605cd16..7884d2c947 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -694,9 +694,9 @@ dependencies = [ [[package]] name = "criterion-swarm" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82693ab3e80479ca52770515f3837028800e365aac48a83edb86f920a6783232" +checksum = "afeb455a7395f21498acd5678ceb403009947adc1bb966ac8b903dee14f8e5b6" dependencies = [ "ansi-replace", "anyhow", @@ -1624,9 +1624,11 @@ dependencies = [ "clap", "criterion-markdown", "criterion-swarm", + "regex", "serde", "serde_json", "tokio", + "toml 1.1.6+spec-1.1.0", ] [[package]] diff --git a/bench_report.toml b/bench_report.toml new file mode 100644 index 0000000000..6a0627bfb8 --- /dev/null +++ b/bench_report.toml @@ -0,0 +1,84 @@ +# Benchmarks whose results appear in pull request comments. +# +# Entries are regular expressions matched against criterion benchmark ids such +# as `sandboxes/create_initialized/default`. CI runs every benchmark and reports +# only the ones selected here, so noisy benchmarks can be dropped from the +# comment without losing their results in the uploaded criterion artifact. +# +# A benchmark is reported when it matches `allowlist` and no `denylist` entry. +# An empty `allowlist` keeps every benchmark the denylist does not exclude, and +# leaving both lists empty disables filtering. +# +# An allowlist entry matching no benchmark fails the report, so a renamed or +# deleted benchmark surfaces instead of silently vanishing from the comment. A +# denylist entry matching nothing is accepted, because a benchmark may be absent +# on some platforms. +# +# cargo ci bench-report --config-file bench_report.toml +# cargo ci bench --config-file bench_report.toml +# +# The lists below keep the benchmarks whose median drifted by at most 5% across +# five back-to-back local runs on an idle 32 core machine. CI runners are +# smaller and noisier, so treat that as a lower bound on the drift CI sees. +# Regenerate by running the suite several times with `--save-baseline` and +# comparing `target/criterion///estimates.json`. +# +# `snapshot_files/cold_start_via_snapshot` and `snapshots/restore` stay out +# whatever they measure. Both families have members far outside the threshold, +# and `snapshots/restore/small` swung 1.43x on a later run after measuring 1.25% +# over the five sampled here. +# +# The `hyperlight_common` groups were restructured by the virtqueue transport +# work, which replaced the `alloc_*`, `free*`, `recycle_pool` and +# `segmented_payload` groups with `payload_allocation` and `slot_pool`, and +# shortened the `virtq_*_allocator_strategy` names. The entries below are the +# members of the new groups that hold to the threshold. +# +# The `snapshot_files`, `snapshots`, `sandboxes/sandbox_from_snapshot` and +# `guest_calls/call_with_restore` entries keep the drift measured before that +# work landed. Taking a snapshot fails on this machine now, so those families +# could not be re-measured. +# +# The commented entries widen the allowlist back to the full suite. Uncommenting +# all of them and emptying the denylist reports all 87 benchmarks. The trailing +# count is how many extra benchmarks each one pulls in. +allowlist = [ + # "^function_call_serialization/", # adds 2 + "^guest_calls/", + # "^guest_functions_with_large_parameters/", # adds 1 + "^payload_allocation/", + "^sample_workloads/", + # "^sandboxes/", # adds 12 + # "^shared_memory/", # adds 4 + "^shared_memory/copy_to_slice/1MB$", + "^shared_memory/fill/1MB$", + # "^slot_pool/", # adds 2 + "^slot_pool/alloc_dealloc_128$", + # "^snapshot_files/", # adds 19 + "^snapshot_files/load_snapshot/large$", + "^snapshot_files/load_snapshot_unverified/large$", + "^snapshot_files/load_snapshot_unverified/medium$", + "^snapshot_files/load_snapshot_unverified/small$", + "^snapshot_files/save_snapshot/large$", + # "^snapshots/", # adds 8 + "^virtq_readonly/", + # "^virtq_readwrite/", # adds 4 + "^virtq_readwrite/slot_pool_segmented/65536$", + "^virtq_readwrite/slot_pool_segmented/8192$", +] + +# Noisy members of otherwise stable groups, with the measured drift. +# +# Every `sandboxes` benchmark drifted by more than 9%, reaching 218% on +# `create_initialized/small`, so that group is absent from the allowlist rather +# than listed here. `function_call_serialization` (9% to 15%) and +# `guest_functions_with_large_parameters` (8%) are absent for the same reason, +# as are the `slot_pool` and `virtq_readwrite` members left out above. +denylist = [ + "^guest_calls/call_with_restore/large$", # 13% + "^guest_calls/call_with_restore/medium$", # 16% + "^guest_calls/call_with_restore/small$", # 11% + "^guest_calls/interrupt_latency$", # 51% + "^virtq_readonly/slot_pool_segmented/8192$", # 19% + "^virtq_readonly/slot_pool_segmented_fragmented/8192$", # 11% +] diff --git a/src/hyperlight_ci/Cargo.toml b/src/hyperlight_ci/Cargo.toml index 9243145387..325faf8ff6 100644 --- a/src/hyperlight_ci/Cargo.toml +++ b/src/hyperlight_ci/Cargo.toml @@ -13,7 +13,9 @@ workspace = true anyhow = "1" clap = { version = "4.6.1", features = ["derive"] } criterion-markdown = "0.1.2" -criterion-swarm = "0.2" +criterion-swarm = "0.2.1" +regex = "1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -tokio = { version = "1", features = ["rt", "macros"] } \ No newline at end of file +tokio = { version = "1", features = ["rt", "macros"] } +toml = "1" \ No newline at end of file diff --git a/src/hyperlight_ci/src/bench.rs b/src/hyperlight_ci/src/bench.rs index 3ab0965665..6bf817219c 100644 --- a/src/hyperlight_ci/src/bench.rs +++ b/src/hyperlight_ci/src/bench.rs @@ -2,12 +2,15 @@ // Copyright 2025 The Hyperlight Authors. //! The `bench` subcommand: runs criterion benchmarks in parallel via criterion-swarm. +use std::collections::HashSet; use std::io::IsTerminal; use std::path::PathBuf; use anyhow::Context; use criterion_swarm::{CriterionSwarm, OutputMode}; +use crate::config::BenchConfig; + /// An output mode flag for `--build-output` / `--benchmarks-output`. #[derive(Clone, Debug)] pub(crate) struct OutputModeFlags(OutputMode); @@ -66,12 +69,22 @@ pub struct BenchArgs { #[arg(short = 'F', long)] pub features: Vec, + /// Run only the benchmarks selected by this config file + #[arg(long, value_name = "PATH")] + pub config_file: Option, + /// Additional arguments to forward to criterion benchmarks #[arg(trailing_var_arg = true, allow_hyphen_values = true)] pub bench_args: Vec, } pub async fn run(mut args: BenchArgs) -> anyhow::Result<()> { + let config = args + .config_file + .as_deref() + .map(BenchConfig::load) + .transpose()?; + let mut swarm = CriterionSwarm::builder().jobs(args.jobs); if !args.binary.is_empty() { @@ -112,10 +125,19 @@ pub async fn run(mut args: BenchArgs) -> anyhow::Result<()> { .benchmarks(bench_mode), ); - let swarm = swarm + let mut swarm = swarm .prepare() .await .context("Failed to prepare criterion swarm")?; + + if let Some(config) = &config { + let selected: HashSet = config + .select(swarm.benchmarks().into_iter().map(str::to_string))? + .into_iter() + .collect(); + swarm.retain(|name| selected.contains(name)); + } + if bench_mode == (bench_mode | OutputMode::SUMMARY) { let total = swarm.benchmarks().len(); let jobs = swarm.jobs().min(total); diff --git a/src/hyperlight_ci/src/bench_report.rs b/src/hyperlight_ci/src/bench_report.rs index 1410b94dae..a9606dcfcb 100644 --- a/src/hyperlight_ci/src/bench_report.rs +++ b/src/hyperlight_ci/src/bench_report.rs @@ -9,6 +9,8 @@ use anyhow::{Context, Result}; use clap::Args; use criterion_swarm::{CriterionSwarm, NoopReporter}; +use crate::config::BenchConfig; + /// Command-line arguments for the `bench-report` subcommand. #[derive(Args)] pub struct BenchReportArgs { @@ -25,6 +27,10 @@ pub struct BenchReportArgs { #[arg(long)] pub collapsible: Option, + /// Report only the benchmarks selected by this config file + #[arg(long, value_name = "PATH")] + pub config_file: Option, + /// Additional arguments to forward to criterion benchmarks (e.g. filter, --exact) #[arg(trailing_var_arg = true, allow_hyphen_values = true)] pub bench_args: Vec, @@ -32,24 +38,28 @@ pub struct BenchReportArgs { /// Entry point for the bench-report subcommand. pub async fn run(args: BenchReportArgs) -> Result<()> { - let allowlist = build_allowlist(&args).await?; + let mut benchmarks = discover_benchmarks(&args).await?; + + if let Some(path) = &args.config_file { + benchmarks = BenchConfig::load(path)?.select(benchmarks)?; + } let options = criterion_markdown::RenderOptions { collapsible: args.collapsible, }; let markdown = - criterion_markdown::render_with_options(&args.criterion_dir, &allowlist, &options)?; + criterion_markdown::render_with_options(&args.criterion_dir, &benchmarks, &options)?; print!("{markdown}"); Ok(()) } -/// Builds an allowlist of benchmark full_ids by discovering benchmarks via CriterionSwarm. +/// Discovers benchmark full_ids via CriterionSwarm. /// /// All trailing arguments (filter, --exact, etc.) are forwarded as bench args /// to CriterionSwarm so it handles filtering during discovery. -async fn build_allowlist(args: &BenchReportArgs) -> Result> { +async fn discover_benchmarks(args: &BenchReportArgs) -> Result> { let mut swarm = CriterionSwarm::builder(); if !args.binary.is_empty() { diff --git a/src/hyperlight_ci/src/config.rs b/src/hyperlight_ci/src/config.rs new file mode 100644 index 0000000000..355de9a7a6 --- /dev/null +++ b/src/hyperlight_ci/src/config.rs @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! The benchmark report configuration shared by the `bench` and `bench-report` subcommands. + +use std::path::Path; + +use anyhow::{Context, Result, bail}; +use regex::RegexSet; +use serde::Deserialize; + +/// Unknown keys are rejected so a stale key cannot silently disable filtering. +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct ConfigFile { + #[serde(default)] + allowlist: Vec, + #[serde(default)] + denylist: Vec, +} + +/// Benchmark id patterns selecting which results are reported. +#[derive(Debug)] +pub struct BenchConfig { + allow: RegexSet, + deny: RegexSet, +} + +impl BenchConfig { + /// Read `allowlist` and `denylist` pattern arrays from a TOML file. + pub fn load(path: &Path) -> Result { + let text = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read benchmark config {}", path.display()))?; + + Self::parse(&text).with_context(|| format!("Invalid benchmark config {}", path.display())) + } + + fn parse(text: &str) -> Result { + let file: ConfigFile = toml::from_str(text)?; + + Ok(Self { + allow: RegexSet::new(&file.allowlist)?, + deny: RegexSet::new(&file.denylist)?, + }) + } + + /// Keep the selected benchmarks, rejecting allowlist patterns that match nothing. + /// + /// Empty lists keep every benchmark. A denylist pattern matching nothing is + /// accepted, because a benchmark may be absent on some platforms. + pub fn select(&self, benchmarks: impl IntoIterator) -> Result> { + let mut used = vec![false; self.allow.len()]; + let mut selected = Vec::new(); + + for benchmark in benchmarks { + let matches = self.allow.matches(&benchmark); + for index in matches.iter() { + used[index] = true; + } + + let allowed = self.allow.is_empty() || matches.matched_any(); + if allowed && !self.deny.is_match(&benchmark) { + selected.push(benchmark); + } + } + + let stale: Vec<&str> = self + .allow + .patterns() + .iter() + .zip(&used) + .filter(|(_, used)| !**used) + .map(|(pattern, _)| pattern.as_str()) + .collect(); + + if !stale.is_empty() { + bail!( + "Benchmark allowlist patterns match no benchmark: {}", + stale.join(", ") + ); + } + + if selected.is_empty() && self.filters() { + bail!("Benchmark config excludes every benchmark"); + } + + Ok(selected) + } + + /// Whether the config restricts the reported benchmarks at all. + fn filters(&self) -> bool { + !self.allow.is_empty() || !self.deny.is_empty() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn benchmarks(names: &[&str]) -> Vec { + names.iter().map(|name| name.to_string()).collect() + } + + #[test] + fn selects_only_allowlisted_benchmarks() { + let config = BenchConfig::parse(r#"allowlist = ["^sandboxes/", "^guest_calls/"]"#).unwrap(); + let selected = config + .select(benchmarks(&[ + "sandboxes/create", + "snapshots/save", + "guest_calls/small", + ])) + .unwrap(); + + assert_eq!(selected, ["sandboxes/create", "guest_calls/small"]); + } + + #[test] + fn denylist_subtracts_from_the_allowlist() { + let config = BenchConfig::parse( + r#" + allowlist = ["^sandboxes/"] + denylist = ["^sandboxes/noisy"] + "#, + ) + .unwrap(); + + let selected = config + .select(benchmarks(&["sandboxes/create", "sandboxes/noisy_case"])) + .unwrap(); + + assert_eq!(selected, ["sandboxes/create"]); + } + + #[test] + fn empty_allowlist_keeps_everything_not_denied() { + let config = BenchConfig::parse(r#"denylist = ["^snapshots/"]"#).unwrap(); + let selected = config + .select(benchmarks(&["sandboxes/create", "snapshots/save"])) + .unwrap(); + + assert_eq!(selected, ["sandboxes/create"]); + } + + #[test] + fn rejects_allowlist_patterns_matching_nothing() { + let config = + BenchConfig::parse(r#"allowlist = ["^sandboxes/", "^renamed_away/"]"#).unwrap(); + let error = config + .select(benchmarks(&["sandboxes/create"])) + .unwrap_err() + .to_string(); + + assert!(error.contains("^renamed_away/"), "{error}"); + assert!(!error.contains("^sandboxes/"), "{error}"); + } + + #[test] + fn accepts_denylist_patterns_matching_nothing() { + let config = BenchConfig::parse( + r#" + allowlist = ["^sandboxes/"] + denylist = ["^windows_only/"] + "#, + ) + .unwrap(); + + let selected = config.select(benchmarks(&["sandboxes/create"])).unwrap(); + assert_eq!(selected, ["sandboxes/create"]); + } + + #[test] + fn rejects_a_config_that_excludes_everything() { + let config = BenchConfig::parse( + r#" + allowlist = ["^sandboxes/"] + denylist = ["^sandboxes/"] + "#, + ) + .unwrap(); + + let error = config + .select(benchmarks(&["sandboxes/create"])) + .unwrap_err() + .to_string(); + + assert!(error.contains("excludes every benchmark"), "{error}"); + } + + #[test] + fn config_without_keys_keeps_every_benchmark() { + let config = BenchConfig::parse("").unwrap(); + let selected = config + .select(benchmarks(&["sandboxes/create", "snapshots/save"])) + .unwrap(); + + assert_eq!(selected, ["sandboxes/create", "snapshots/save"]); + } + + #[test] + fn empty_lists_keep_every_benchmark() { + let config = BenchConfig::parse("allowlist = []\ndenylist = []").unwrap(); + let selected = config + .select(benchmarks(&["sandboxes/create", "snapshots/save"])) + .unwrap(); + + assert_eq!(selected, ["sandboxes/create", "snapshots/save"]); + } + + #[test] + fn rejects_unknown_keys() { + let error = BenchConfig::parse(r#"patterns = ["^sandboxes/"]"#) + .unwrap_err() + .to_string(); + assert!(error.contains("unknown field"), "{error}"); + } + + #[test] + fn rejects_invalid_pattern() { + let error = BenchConfig::parse(r#"allowlist = ["^sandboxes/("]"#) + .unwrap_err() + .to_string(); + assert!(error.contains("regex parse error"), "{error}"); + } +} diff --git a/src/hyperlight_ci/src/main.rs b/src/hyperlight_ci/src/main.rs index 76f9135bac..1ce703aae5 100644 --- a/src/hyperlight_ci/src/main.rs +++ b/src/hyperlight_ci/src/main.rs @@ -2,6 +2,7 @@ // Copyright 2025 The Hyperlight Authors. mod bench; mod bench_report; +mod config; use clap::{Parser, Subcommand}; From 193af60ae3444d85ddd709c19305861af97f150e Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Tue, 22 Sep 2026 22:39:40 +0100 Subject: [PATCH 5/9] build(ci): bump criterion-swarm to 0.2.2 0.2.2 stops pinning benchmarks to CPU 0, which hosts the boot CPU's timers, RCU kthreads and unbound kworkers. A benchmark pinned there waited 327us per timeslice to be scheduled against 47us elsewhere, running up to 4x slower. Which benchmark drew that core varied per run, so it surfaced as drift. Measured over the full suite at equal parallelism, p90 run-to-run drift falls from 257% to 6% for the sandbox benchmarks, and from 14% to 5% for the rest. One core of parallelism is given up, costing about 17% wall time per run. Signed-off-by: Jorge Prendes --- Cargo.lock | 4 ++-- src/hyperlight_ci/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7884d2c947..1d67e06ec1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -694,9 +694,9 @@ dependencies = [ [[package]] name = "criterion-swarm" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afeb455a7395f21498acd5678ceb403009947adc1bb966ac8b903dee14f8e5b6" +checksum = "4bbb01e7dd0d588f78fe0bd2fdc12e921875542754473f22b09da4f9edbc2770" dependencies = [ "ansi-replace", "anyhow", diff --git a/src/hyperlight_ci/Cargo.toml b/src/hyperlight_ci/Cargo.toml index 325faf8ff6..0f8138e317 100644 --- a/src/hyperlight_ci/Cargo.toml +++ b/src/hyperlight_ci/Cargo.toml @@ -13,7 +13,7 @@ workspace = true anyhow = "1" clap = { version = "4.6.1", features = ["derive"] } criterion-markdown = "0.1.2" -criterion-swarm = "0.2.1" +criterion-swarm = "0.2.2" regex = "1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" From 1481413e4a4e78945f2c6251237c017492224b04 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Tue, 22 Sep 2026 22:39:50 +0100 Subject: [PATCH 6/9] feat(ci): hold a sandbox resident while benchmarking A vCPU created without an in-kernel LAPIC bumps the kernel's `kvm_has_noapic_vcpu` static key, and teardown drops it again. Each transition through zero rewrites kernel text and IPIs every core. Benchmarks that create and drop sandboxes leave no resident VM between iterations, so they cross that boundary constantly: a full suite run issues 176k broadcast IPIs, against 560 with one sandbox held. `cargo ci bench` now keeps one sandbox alive for the duration of a run, so that cost lands on neither the benchmark that triggers it nor its neighbours. Excluding benchmarks that ran on CPU 0, which has its own much larger effect, median drift for the sandbox group falls from 6.5% to 1.7%. Pass `--no-ballast` to measure the cold path instead. The helper is an example rather than a dependency of this crate, keeping hyperlight-host out of the CI tool's build. It exits on stdin EOF, so it cannot outlive the run even when this process is killed without unwinding. Signed-off-by: Jorge Prendes --- src/hyperlight_ci/src/ballast.rs | 98 ++++++++++++++++++++ src/hyperlight_ci/src/bench.rs | 17 +++- src/hyperlight_ci/src/main.rs | 1 + src/hyperlight_host/examples/ballast/main.rs | 29 ++++++ 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 src/hyperlight_ci/src/ballast.rs create mode 100644 src/hyperlight_host/examples/ballast/main.rs diff --git a/src/hyperlight_ci/src/ballast.rs b/src/hyperlight_ci/src/ballast.rs new file mode 100644 index 0000000000..32113fca7c --- /dev/null +++ b/src/hyperlight_ci/src/ballast.rs @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! A resident VM held for the duration of a benchmark run. + +use std::io::{BufRead, BufReader}; +use std::path::PathBuf; +use std::process::{Child, ChildStdin, Command, Stdio}; + +use anyhow::{Context, bail}; + +/// Line the helper prints once its sandbox is live. +const READY: &str = "ballast ready"; + +/// A sandbox held alive for a whole benchmark run. +/// +/// Benchmarks that create and drop sandboxes leave the host with no resident +/// VM between iterations. Crossing that boundary toggles a kernel static key, +/// and each toggle patches kernel text and IPIs every core. That cost lands on +/// whichever benchmark happens to trigger it, so holding one sandbox resident +/// keeps it out of the measurements. +pub(crate) struct Ballast { + child: Child, + /// The helper exits when this closes, which the OS does for us if this + /// process is killed before [`Drop`] can run. + _stdin: ChildStdin, +} + +impl Ballast { + /// Build the helper, start it, and wait until its sandbox is live. + pub(crate) fn start() -> anyhow::Result { + let exe = build().context("Failed to build the ballast helper")?; + + let mut child = Command::new(&exe) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .with_context(|| format!("Failed to start the ballast helper {}", exe.display()))?; + + let stdin = child.stdin.take().expect("stdin is piped"); + let stdout = child.stdout.take().expect("stdout is piped"); + let mut line = String::new(); + BufReader::new(stdout) + .read_line(&mut line) + .context("Failed to read readiness from the ballast helper")?; + + if line.trim() != READY { + bail!("Ballast helper did not become ready"); + } + + Ok(Self { + child, + _stdin: stdin, + }) + } +} + +impl Drop for Ballast { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +/// Build the ballast example and return the executable cargo produced. +fn build() -> anyhow::Result { + let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string()); + + let output = Command::new(cargo) + .args([ + "build", + "--release", + "--package", + "hyperlight-host", + "--example", + "ballast", + "--message-format=json-render-diagnostics", + ]) + .stderr(Stdio::inherit()) + .output()?; + + if !output.status.success() { + bail!("cargo build failed for the ballast helper"); + } + + for line in output.stdout.as_slice().lines() { + let Ok(line) = line else { continue }; + let Ok(msg) = serde_json::from_str::(&line) else { + continue; + }; + if msg["reason"] == "compiler-artifact" && msg["target"]["name"] == "ballast" { + if let Some(exe) = msg["executable"].as_str() { + return Ok(PathBuf::from(exe)); + } + } + } + + bail!("cargo reported no executable for the ballast helper") +} diff --git a/src/hyperlight_ci/src/bench.rs b/src/hyperlight_ci/src/bench.rs index 6bf817219c..5c1480773f 100644 --- a/src/hyperlight_ci/src/bench.rs +++ b/src/hyperlight_ci/src/bench.rs @@ -9,6 +9,7 @@ use std::path::PathBuf; use anyhow::Context; use criterion_swarm::{CriterionSwarm, OutputMode}; +use crate::ballast::Ballast; use crate::config::BenchConfig; /// An output mode flag for `--build-output` / `--benchmarks-output`. @@ -73,6 +74,10 @@ pub struct BenchArgs { #[arg(long, value_name = "PATH")] pub config_file: Option, + /// Run without holding a sandbox resident for the duration of the run + #[arg(long)] + pub no_ballast: bool, + /// Additional arguments to forward to criterion benchmarks #[arg(trailing_var_arg = true, allow_hyphen_values = true)] pub bench_args: Vec, @@ -143,5 +148,15 @@ pub async fn run(mut args: BenchArgs) -> anyhow::Result<()> { let jobs = swarm.jobs().min(total); println!("Running {total} benchmarks with parallelism {jobs}"); } - swarm.run().await.context("Failed to run criterion swarm") + + // Held until the run finishes. + let ballast = if args.no_ballast { + None + } else { + Some(Ballast::start()?) + }; + + let result = swarm.run().await.context("Failed to run criterion swarm"); + drop(ballast); + result } diff --git a/src/hyperlight_ci/src/main.rs b/src/hyperlight_ci/src/main.rs index 1ce703aae5..24b3183438 100644 --- a/src/hyperlight_ci/src/main.rs +++ b/src/hyperlight_ci/src/main.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2025 The Hyperlight Authors. +mod ballast; mod bench; mod bench_report; mod config; diff --git a/src/hyperlight_host/examples/ballast/main.rs b/src/hyperlight_host/examples/ballast/main.rs new file mode 100644 index 0000000000..2d96fdcc17 --- /dev/null +++ b/src/hyperlight_host/examples/ballast/main.rs @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! Holds one sandbox alive and then idles. +//! +//! A vCPU created without an in-kernel LAPIC bumps the kernel's +//! `kvm_has_noapic_vcpu` static key, and teardown drops it again. Each +//! transition through zero patches kernel text, which IPIs every core. +//! Keeping one sandbox resident holds the count above zero, so a benchmark +//! that creates and drops sandboxes never crosses that boundary. +//! Parks after startup, so it consumes no CPU while a benchmark runs. + +use std::io::Read; + +use hyperlight_host::SandboxBuilder; + +fn main() -> hyperlight_host::Result<()> { + let _sandbox = + SandboxBuilder::from_file(hyperlight_testing::simple_guest_as_pathbuf()).build()?; + + // Readers wait for this line before starting to measure. + println!("ballast ready"); + + // Stdin reaches EOF when the parent closes it or dies, so the sandbox never + // outlives the run that asked for it. + let mut discard = Vec::new(); + let _ = std::io::stdin().read_to_end(&mut discard); + + Ok(()) +} From 94d24313920662cf40f2e91b6db0aef5ebeefd6b Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Wed, 23 Sep 2026 15:01:45 +0100 Subject: [PATCH 7/9] feat(ci): record what each benchmark run measured Criterion stores an id per result directory but nothing about the run as a whole, and results accumulate: a directory keeps benchmarks that no longer exist, indistinguishable from the ones just measured. CI compounds this by unpacking a baseline into the same directory before running, so its uploaded artifact holds 155 result directories for a suite of 87. `cargo ci bench` now writes `benchmarks.json` next to the results, listing the benchmarks the run covers along with a timestamp and the host it ran on. Reading an archived run no longer needs the binaries that produced it, which is the only other way to tell current results from leftovers. The list is the post-filter set, so it describes what was measured rather than what was discovered. Signed-off-by: Jorge Prendes --- src/hyperlight_ci/src/bench.rs | 4 ++ src/hyperlight_ci/src/main.rs | 1 + src/hyperlight_ci/src/manifest.rs | 102 ++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 src/hyperlight_ci/src/manifest.rs diff --git a/src/hyperlight_ci/src/bench.rs b/src/hyperlight_ci/src/bench.rs index 5c1480773f..6fe4c99390 100644 --- a/src/hyperlight_ci/src/bench.rs +++ b/src/hyperlight_ci/src/bench.rs @@ -11,6 +11,7 @@ use criterion_swarm::{CriterionSwarm, OutputMode}; use crate::ballast::Ballast; use crate::config::BenchConfig; +use crate::manifest; /// An output mode flag for `--build-output` / `--benchmarks-output`. #[derive(Clone, Debug)] @@ -149,6 +150,9 @@ pub async fn run(mut args: BenchArgs) -> anyhow::Result<()> { println!("Running {total} benchmarks with parallelism {jobs}"); } + manifest::write(swarm.benchmarks().into_iter().map(str::to_string)) + .context("Failed to write the benchmark manifest")?; + // Held until the run finishes. let ballast = if args.no_ballast { None diff --git a/src/hyperlight_ci/src/main.rs b/src/hyperlight_ci/src/main.rs index 24b3183438..587e2e23ca 100644 --- a/src/hyperlight_ci/src/main.rs +++ b/src/hyperlight_ci/src/main.rs @@ -4,6 +4,7 @@ mod ballast; mod bench; mod bench_report; mod config; +mod manifest; use clap::{Parser, Subcommand}; diff --git a/src/hyperlight_ci/src/manifest.rs b/src/hyperlight_ci/src/manifest.rs new file mode 100644 index 0000000000..a5d5e14024 --- /dev/null +++ b/src/hyperlight_ci/src/manifest.rs @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! A record of what a benchmark run measured, and on what. + +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; +use std::{env, fs}; + +use anyhow::{Context, Result}; +use serde::Serialize; + +/// Written alongside the criterion results, so a run can be interpreted without +/// the benchmark binaries that produced it. +/// +/// Criterion records an id per result directory but nothing about the run as a +/// whole. Results also accumulate: a directory carries benchmarks that no +/// longer exist, indistinguishable from the ones just measured. This lists what +/// the run actually covered. +#[derive(Serialize)] +struct Manifest { + /// Seconds since the Unix epoch. Criterion timestamps nothing, and archived + /// results lose their file times. + timestamp: u64, + host: Host, + benchmarks: Vec, +} + +#[derive(Serialize)] +struct Host { + os: &'static str, + arch: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + logical_cpus: Option, + #[serde(skip_serializing_if = "Option::is_none")] + cpu_vendor: Option, + #[serde(skip_serializing_if = "Option::is_none")] + cpu_model: Option, +} + +/// Where criterion keeps its results. +fn criterion_dir() -> PathBuf { + env::var_os("CRITERION_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("target").join("criterion")) +} + +#[cfg(target_os = "linux")] +fn cpu_vendor_and_model() -> (Option, Option) { + let Ok(text) = fs::read_to_string("/proc/cpuinfo") else { + return (None, None); + }; + let field = |key: &str| { + text.lines() + .find_map(|l| l.split_once(':').filter(|(k, _)| k.trim() == key)) + .map(|(_, v)| v.trim().to_string()) + }; + (field("vendor_id"), field("model name")) +} + +#[cfg(target_os = "windows")] +fn cpu_vendor_and_model() -> (Option, Option) { + // e.g. "Intel64 Family 6 Model 154 Stepping 3, GenuineIntel" + let model = env::var("PROCESSOR_IDENTIFIER").ok(); + let vendor = model + .as_deref() + .and_then(|m| m.rsplit_once(',')) + .map(|(_, v)| v.trim().to_string()); + (vendor, model) +} + +#[cfg(not(any(target_os = "linux", target_os = "windows")))] +fn cpu_vendor_and_model() -> (Option, Option) { + (None, None) +} + +/// Record `benchmarks` as the contents of the run about to start. +pub(crate) fn write(benchmarks: impl IntoIterator) -> Result<()> { + let (cpu_vendor, cpu_model) = cpu_vendor_and_model(); + let mut benchmarks: Vec = benchmarks.into_iter().collect(); + benchmarks.sort(); + + let manifest = Manifest { + timestamp: SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or_default(), + host: Host { + os: env::consts::OS, + arch: env::consts::ARCH, + logical_cpus: std::thread::available_parallelism().ok().map(Into::into), + cpu_vendor, + cpu_model, + }, + benchmarks, + }; + + let dir = criterion_dir(); + fs::create_dir_all(&dir).with_context(|| format!("Failed to create {}", dir.display()))?; + let path = dir.join("benchmarks.json"); + let json = serde_json::to_string_pretty(&manifest)?; + fs::write(&path, json).with_context(|| format!("Failed to write {}", path.display())) +} From 624fe0bfd466830eddea816027a93f9dc0049195 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Wed, 23 Sep 2026 16:07:54 +0100 Subject: [PATCH 8/9] feat(ci): report from the manifest a run left behind Naming the benchmarks by listing the binaries builds them first, and describes the current checkout rather than the results being reported. Those are the same thing only while reporting a local run of the current tree. `bench-report` now takes the ids from `benchmarks.json` when the results carry one, so a criterion directory from elsewhere, a CI artifact say, reads without a toolchain and without the checkout matching. Rendering the downloaded results of a full suite takes 11ms rather than a build. Explicit `--binary` or trailing bench args still list the binaries, since both name benchmarks the manifest cannot filter, as do results from before the manifest existed. Signed-off-by: Jorge Prendes --- src/hyperlight_ci/src/bench_report.rs | 15 ++++++++-- src/hyperlight_ci/src/manifest.rs | 40 +++++++++++++++++++-------- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/hyperlight_ci/src/bench_report.rs b/src/hyperlight_ci/src/bench_report.rs index a9606dcfcb..e25f79f4c1 100644 --- a/src/hyperlight_ci/src/bench_report.rs +++ b/src/hyperlight_ci/src/bench_report.rs @@ -10,6 +10,7 @@ use clap::Args; use criterion_swarm::{CriterionSwarm, NoopReporter}; use crate::config::BenchConfig; +use crate::manifest; /// Command-line arguments for the `bench-report` subcommand. #[derive(Args)] @@ -55,11 +56,19 @@ pub async fn run(args: BenchReportArgs) -> Result<()> { Ok(()) } -/// Discovers benchmark full_ids via CriterionSwarm. +/// Benchmark ids for the results being reported. /// -/// All trailing arguments (filter, --exact, etc.) are forwarded as bench args -/// to CriterionSwarm so it handles filtering during discovery. +/// A run records what it measured, so prefer that: listing the binaries builds +/// them and describes the current checkout rather than the run in hand, which +/// differ whenever results come from elsewhere. Explicit binaries or bench args +/// ask for the binaries, and older results carry no manifest. async fn discover_benchmarks(args: &BenchReportArgs) -> Result> { + if args.binary.is_empty() && args.bench_args.is_empty() { + if let Some(benchmarks) = manifest::read(&args.criterion_dir)? { + return Ok(benchmarks); + } + } + let mut swarm = CriterionSwarm::builder(); if !args.binary.is_empty() { diff --git a/src/hyperlight_ci/src/manifest.rs b/src/hyperlight_ci/src/manifest.rs index a5d5e14024..3836aafde0 100644 --- a/src/hyperlight_ci/src/manifest.rs +++ b/src/hyperlight_ci/src/manifest.rs @@ -2,12 +2,15 @@ // Copyright 2025 The Hyperlight Authors. //! A record of what a benchmark run measured, and on what. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use std::{env, fs}; use anyhow::{Context, Result}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; + +/// Name of the manifest within the criterion results directory. +const FILE_NAME: &str = "benchmarks.json"; /// Written alongside the criterion results, so a run can be interpreted without /// the benchmark binaries that produced it. @@ -16,7 +19,7 @@ use serde::Serialize; /// whole. Results also accumulate: a directory carries benchmarks that no /// longer exist, indistinguishable from the ones just measured. This lists what /// the run actually covered. -#[derive(Serialize)] +#[derive(Serialize, Deserialize)] struct Manifest { /// Seconds since the Unix epoch. Criterion timestamps nothing, and archived /// results lose their file times. @@ -25,15 +28,15 @@ struct Manifest { benchmarks: Vec, } -#[derive(Serialize)] +#[derive(Serialize, Deserialize)] struct Host { - os: &'static str, - arch: &'static str, - #[serde(skip_serializing_if = "Option::is_none")] + os: String, + arch: String, + #[serde(default, skip_serializing_if = "Option::is_none")] logical_cpus: Option, - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] cpu_vendor: Option, - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] cpu_model: Option, } @@ -85,8 +88,8 @@ pub(crate) fn write(benchmarks: impl IntoIterator) -> Result<()> .map(|d| d.as_secs()) .unwrap_or_default(), host: Host { - os: env::consts::OS, - arch: env::consts::ARCH, + os: env::consts::OS.to_string(), + arch: env::consts::ARCH.to_string(), logical_cpus: std::thread::available_parallelism().ok().map(Into::into), cpu_vendor, cpu_model, @@ -96,7 +99,20 @@ pub(crate) fn write(benchmarks: impl IntoIterator) -> Result<()> let dir = criterion_dir(); fs::create_dir_all(&dir).with_context(|| format!("Failed to create {}", dir.display()))?; - let path = dir.join("benchmarks.json"); + let path = dir.join(FILE_NAME); let json = serde_json::to_string_pretty(&manifest)?; fs::write(&path, json).with_context(|| format!("Failed to write {}", path.display())) } + +/// The benchmarks a run recorded, or `None` when it left no manifest. +pub(crate) fn read(dir: &Path) -> Result>> { + let path = dir.join(FILE_NAME); + let Ok(text) = fs::read_to_string(&path) else { + return Ok(None); + }; + + let manifest: Manifest = serde_json::from_str(&text) + .with_context(|| format!("Failed to parse {}", path.display()))?; + + Ok(Some(manifest.benchmarks)) +} From 1d894e94fd79010b8db7139124067296110e4c5c Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Wed, 23 Sep 2026 16:51:07 +0100 Subject: [PATCH 9/9] feat(ci): report the benchmarks of a CI run Comparing what CI measured meant downloading six artifacts by hand, unpacking each somewhere, and running the report once per configuration. The results are richer than the comment CI posts, holding every benchmark rather than the reported subset and the samples behind each estimate, so reaching for them is worth making cheap. `bench-report --from-run ` renders a whole run, one section per hypervisor and cpu vendor, the way the pull request comment reads. `--pr ` finds the run, taking the most recent one that still has its artifacts: the newest is often a label check, or a run whose benchmarks have not finished. Runs land under `target/ci-runs` and are reused, artifacts being immutable. A run is about 400MB unpacked, and the first report of one waits on the download. This leans on the manifest, so the ids come from what the run measured rather than from building this checkout, which need not even be the same commit. Signed-off-by: Jorge Prendes --- src/hyperlight_ci/src/bench_report.rs | 64 ++++++++--- src/hyperlight_ci/src/main.rs | 1 + src/hyperlight_ci/src/remote.rs | 151 ++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 14 deletions(-) create mode 100644 src/hyperlight_ci/src/remote.rs diff --git a/src/hyperlight_ci/src/bench_report.rs b/src/hyperlight_ci/src/bench_report.rs index e25f79f4c1..f3af98cc0a 100644 --- a/src/hyperlight_ci/src/bench_report.rs +++ b/src/hyperlight_ci/src/bench_report.rs @@ -3,14 +3,17 @@ //! The `bench-report` subcommand: generates a markdown table from existing //! criterion benchmark results in `target/criterion/`. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use clap::Args; use criterion_swarm::{CriterionSwarm, NoopReporter}; use crate::config::BenchConfig; -use crate::manifest; +use crate::{manifest, remote}; + +/// Where downloaded runs are kept. +const RUN_CACHE: &str = "target/ci-runs"; /// Command-line arguments for the `bench-report` subcommand. #[derive(Args)] @@ -24,6 +27,22 @@ pub struct BenchReportArgs { #[arg(long, default_value = "target/criterion")] pub criterion_dir: PathBuf, + /// Report the results of this CI run instead of a local directory + #[arg(long, value_name = "RUN_ID", conflicts_with = "pull_request")] + pub from_run: Option, + + /// Report the latest CI run of this pull request + #[arg(long = "pr", value_name = "NUMBER")] + pub pull_request: Option, + + /// Repository holding the CI runs + #[arg( + long, + value_name = "OWNER/NAME", + default_value = "hyperlight-dev/hyperlight" + )] + pub repo: String, + /// Wrap the output in a collapsible
tag with the given summary text. #[arg(long)] pub collapsible: Option, @@ -39,32 +58,49 @@ pub struct BenchReportArgs { /// Entry point for the bench-report subcommand. pub async fn run(args: BenchReportArgs) -> Result<()> { - let mut benchmarks = discover_benchmarks(&args).await?; - - if let Some(path) = &args.config_file { - benchmarks = BenchConfig::load(path)?.select(benchmarks)?; - } + let run = match (args.from_run, args.pull_request) { + (Some(run), _) => Some(run), + (None, Some(pr)) => Some(remote::latest_run_for(&args.repo, pr)?), + (None, None) => None, + }; - let options = criterion_markdown::RenderOptions { - collapsible: args.collapsible, + let Some(run) = run else { + let markdown = report(&args, &args.criterion_dir, args.collapsible.clone()).await?; + print!("{markdown}"); + return Ok(()); }; - let markdown = - criterion_markdown::render_with_options(&args.criterion_dir, &benchmarks, &options)?; - print!("{markdown}"); + // A run covers every hypervisor and cpu vendor, one section each. + eprintln!("Reporting run {run} of {}", args.repo); + for results in remote::fetch(&args.repo, run, Path::new(RUN_CACHE))? { + let markdown = report(&args, &results.dir, Some(results.label)).await?; + print!("{markdown}"); + } Ok(()) } +/// Render the results in `dir`. +async fn report(args: &BenchReportArgs, dir: &Path, collapsible: Option) -> Result { + let mut benchmarks = discover_benchmarks(args, dir).await?; + + if let Some(path) = &args.config_file { + benchmarks = BenchConfig::load(path)?.select(benchmarks)?; + } + + let options = criterion_markdown::RenderOptions { collapsible }; + criterion_markdown::render_with_options(dir, &benchmarks, &options) +} + /// Benchmark ids for the results being reported. /// /// A run records what it measured, so prefer that: listing the binaries builds /// them and describes the current checkout rather than the run in hand, which /// differ whenever results come from elsewhere. Explicit binaries or bench args /// ask for the binaries, and older results carry no manifest. -async fn discover_benchmarks(args: &BenchReportArgs) -> Result> { +async fn discover_benchmarks(args: &BenchReportArgs, dir: &Path) -> Result> { if args.binary.is_empty() && args.bench_args.is_empty() { - if let Some(benchmarks) = manifest::read(&args.criterion_dir)? { + if let Some(benchmarks) = manifest::read(dir)? { return Ok(benchmarks); } } diff --git a/src/hyperlight_ci/src/main.rs b/src/hyperlight_ci/src/main.rs index 587e2e23ca..5c37698e83 100644 --- a/src/hyperlight_ci/src/main.rs +++ b/src/hyperlight_ci/src/main.rs @@ -5,6 +5,7 @@ mod bench; mod bench_report; mod config; mod manifest; +mod remote; use clap::{Parser, Subcommand}; diff --git a/src/hyperlight_ci/src/remote.rs b/src/hyperlight_ci/src/remote.rs new file mode 100644 index 0000000000..272cd0e8bf --- /dev/null +++ b/src/hyperlight_ci/src/remote.rs @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2025 The Hyperlight Authors. +//! Benchmark results taken from a CI run rather than this machine. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use anyhow::{Context, Result, bail}; +use serde::Deserialize; + +/// Artifacts holding a criterion directory are named after the configuration +/// that produced them, `benchmarks_Linux_kvm_amd` and so on. +const ARTIFACT_PREFIX: &str = "benchmarks_"; + +/// How far back to look for a run that still has its benchmark artifacts. +/// They outlive the workflow by days, but not forever. +const RUNS_SEARCHED: usize = 15; + +/// One configuration's results, and where they were unpacked. +pub(crate) struct Results { + /// The configuration that produced them, `Linux_kvm_amd` and so on. + pub label: String, + pub dir: PathBuf, +} + +#[derive(Deserialize)] +struct Artifact { + name: String, + expired: bool, +} + +#[derive(Deserialize)] +struct ArtifactList { + artifacts: Vec, +} + +#[derive(Deserialize)] +struct Run { + #[serde(rename = "databaseId")] + id: u64, +} + +/// Run `gh` and hand back its stdout. +fn gh(args: &[&str]) -> Result> { + let output = Command::new("gh") + .args(args) + .output() + .context("Failed to run gh. The GitHub CLI provides the run artifacts")?; + + if !output.status.success() { + bail!( + "gh {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr).trim() + ); + } + + Ok(output.stdout) +} + +/// Names of the benchmark artifacts a run still holds. +fn artifacts(repo: &str, run: u64) -> Result> { + let path = format!("repos/{repo}/actions/runs/{run}/artifacts"); + let list: ArtifactList = serde_json::from_slice(&gh(&["api", &path])?) + .with_context(|| format!("Failed to read the artifacts of run {run}"))?; + + let mut names: Vec = list + .artifacts + .into_iter() + .filter(|a| !a.expired && a.name.starts_with(ARTIFACT_PREFIX)) + .map(|a| a.name) + .collect(); + names.sort(); + names.dedup(); + Ok(names) +} + +/// The most recent run of `pull_request` that still has benchmark artifacts. +/// +/// The newest run is not always the one to report: a run can be cancelled by +/// the next push, or be recent enough that the benchmarks have not finished. +pub(crate) fn latest_run_for(repo: &str, pull_request: u64) -> Result { + let pr = pull_request.to_string(); + let branch = gh(&[ + "pr", + "view", + &pr, + "--repo", + repo, + "--json", + "headRefName", + "--jq", + ".headRefName", + ]) + .with_context(|| format!("Failed to find pull request {pull_request}"))?; + let branch = String::from_utf8_lossy(&branch).trim().to_string(); + + let limit = RUNS_SEARCHED.to_string(); + let runs: Vec = serde_json::from_slice(&gh(&[ + "run", + "list", + "--repo", + repo, + "--branch", + &branch, + "--limit", + &limit, + "--json", + "databaseId", + ])?) + .context("Failed to list the workflow runs of the branch")?; + + for run in &runs { + if !artifacts(repo, run.id)?.is_empty() { + return Ok(run.id); + } + } + + bail!("No run of {branch} still has benchmark artifacts") +} + +/// Fetch every configuration's results from `run`, reusing what is already on +/// disk. Artifacts are immutable, so a run downloads once. +pub(crate) fn fetch(repo: &str, run: u64, cache: &Path) -> Result> { + let names = artifacts(repo, run)?; + if names.is_empty() { + bail!("Run {run} has no benchmark artifacts. They may have expired"); + } + + let run_dir = cache.join(run.to_string()); + let mut results = Vec::new(); + + for name in names { + let label = name[ARTIFACT_PREFIX.len()..].to_string(); + let dir = run_dir.join(&label); + + if !dir.join("benchmarks.json").exists() { + std::fs::create_dir_all(&dir) + .with_context(|| format!("Failed to create {}", dir.display()))?; + let (id, out) = (run.to_string(), dir.display().to_string()); + gh(&[ + "run", "download", &id, "--repo", repo, "-n", &name, "-D", &out, + ]) + .with_context(|| format!("Failed to download {name}"))?; + } + + results.push(Results { label, dir }); + } + + Ok(results) +}