diff --git a/Cargo.lock b/Cargo.lock index b719c6e3b..ba247719f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -673,9 +673,9 @@ dependencies = [ [[package]] name = "criterion-markdown" -version = "0.2.2" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d6e13f76f7a56c31550cfa23e2c9d4f312afe96f51667c244a6402695b672e8" +checksum = "04a71ec3e7be23ef7802da672ac1ba2a5be5e686ee8050b8b70eb43de4457bcf" dependencies = [ "anyhow", "serde", diff --git a/bench_report.toml b/bench_report.toml index de30b510b..dbdfa6001 100644 --- a/bench_report.toml +++ b/bench_report.toml @@ -27,13 +27,6 @@ allowlist = [ # Every entry names one benchmark, so nothing needs excluding. denylist = [] -# How far a result moves before a report calls it a change rather than noise. -# The improvements are how many times faster, `regression` the fraction of the -# baseline a result falls to. -improvement = 1.5 # 1.5x faster -strong_improvement = 1.8 # 1.8x faster -regression = 0.67 # 1.5x slower - # Whose runs `run:`, `pr:` and the rest read, `/` or `remote:` # for whichever repository a git remote points at. repo = "hyperlight-dev/hyperlight" @@ -44,3 +37,18 @@ summary_limit = 3 # End a report with the command that asks for it again, as CI does for the # pull request comment. reproduce = false + +# How far a result moves before a report calls it a change rather than noise. +# The improvements are how many times faster, `regression` the fraction of the +# baseline a result falls to. +[thresholds] +regression = 0.67 # 1.5x slower +improvement = 1.5 # 1.5x faster +strong_improvement = 1.8 # 1.8x faster + +# Icons shown for each change classification. +[thresholds.emojis] +regression = "❌" +stable = "➖" +improvement = "✅" +strong_improvement = "🌟" diff --git a/src/hyperlight_ci/Cargo.toml b/src/hyperlight_ci/Cargo.toml index 3f2ab8816..0d58fac5b 100644 --- a/src/hyperlight_ci/Cargo.toml +++ b/src/hyperlight_ci/Cargo.toml @@ -12,7 +12,7 @@ workspace = true [dependencies] anyhow = "1" clap = { version = "4.6.1", features = ["derive"] } -criterion-markdown = "0.2.2" +criterion-markdown = "0.3.0" criterion-swarm = "0.2.2" regex = "1" serde = { version = "1.0", features = ["derive"] } diff --git a/src/hyperlight_ci/src/bench_report.rs b/src/hyperlight_ci/src/bench_report.rs index 0419b8eff..4dfb5b92b 100644 --- a/src/hyperlight_ci/src/bench_report.rs +++ b/src/hyperlight_ci/src/bench_report.rs @@ -306,26 +306,56 @@ fn reproduce(args: &BenchReportArgs, repo: &str, candidate: &Origin, baseline: & fn thresholds( args: &BenchReportArgs, config: Option<&BenchConfig>, -) -> criterion_markdown::ChangeThresholds { - let from_config = |read: fn(&BenchConfig) -> Option| config.and_then(read); - let mut thresholds = criterion_markdown::ChangeThresholds::default(); +) -> criterion_markdown::Thresholds { + let config = config.map(|config| &config.thresholds); + let mut thresholds = criterion_markdown::Thresholds::default(); - if let Some(ratio) = args.improvement.or_else(|| from_config(|c| c.improvement)) { + if let Some(ratio) = args + .improvement + .or_else(|| config.and_then(|thresholds| thresholds.improvement)) + { thresholds = thresholds.improvement_ratio(ratio); } if let Some(ratio) = args .strong_improvement - .or_else(|| from_config(|c| c.strong_improvement)) + .or_else(|| config.and_then(|thresholds| thresholds.strong_improvement)) { thresholds = thresholds.strong_improvement_ratio(ratio); } - if let Some(ratio) = args.regression.or_else(|| from_config(|c| c.regression)) { + if let Some(ratio) = args + .regression + .or_else(|| config.and_then(|thresholds| thresholds.regression)) + { thresholds = thresholds.regression_ratio(ratio); } thresholds } +/// Change indicators selected by the config file, then the renderer. +fn emojis(config: Option<&BenchConfig>) -> criterion_markdown::Emojis { + let mut emojis = criterion_markdown::Emojis::default(); + let Some(config) = config else { + return emojis; + }; + let config = &config.thresholds.emojis; + + if let Some(emoji) = &config.regression { + emojis = emojis.regression(emoji); + } + if let Some(emoji) = &config.stable { + emojis = emojis.stable(emoji); + } + if let Some(emoji) = &config.improvement { + emojis = emojis.improvement(emoji); + } + if let Some(emoji) = &config.strong_improvement { + emojis = emojis.strong_improvement(emoji); + } + + emojis +} + /// Say which commits the report covers, so a reader can tell what they are /// looking at without knowing how it was asked for. fn measured(repo: &str, candidate: &Origin, baseline: &Origin) -> Option { @@ -483,7 +513,7 @@ fn describe(label: &str) -> String { async fn report( args: &BenchReportArgs, config: Option<&BenchConfig>, - thresholds: criterion_markdown::ChangeThresholds, + thresholds: criterion_markdown::Thresholds, summary_limit: Option, dir: &Path, baseline_root: Option<&Path>, @@ -497,7 +527,8 @@ async fn report( let mut renderer = criterion_markdown::Renderer::new(dir) .benchmarks(benchmarks) - .change_thresholds(thresholds); + .thresholds(thresholds) + .emojis(emojis(config)); if let Some(limit) = summary_limit { renderer = renderer.summary_limit(limit); @@ -557,6 +588,35 @@ async fn discover_benchmarks(args: &BenchReportArgs, dir: &Path) -> Result, #[serde(default)] denylist: Vec, - improvement: Option, - strong_improvement: Option, - regression: Option, + #[serde(default)] + thresholds: ThresholdConfig, repo: Option, summary_limit: Option, reproduce: Option, } +/// Change indicators selected by the benchmark report config. +#[derive(Debug, Default, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct EmojiConfig { + pub regression: Option, + pub stable: Option, + pub improvement: Option, + pub strong_improvement: Option, +} + +/// Change thresholds and their report indicators. +#[derive(Debug, Default, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ThresholdConfig { + pub improvement: Option, + pub strong_improvement: Option, + pub regression: Option, + #[serde(default)] + pub emojis: EmojiConfig, +} + /// Benchmark id patterns selecting which results are reported. #[derive(Debug)] pub struct BenchConfig { allow: RegexSet, deny: RegexSet, - /// Where a change is worth reporting, when the file says. - pub improvement: Option, - pub strong_improvement: Option, - pub regression: Option, + pub thresholds: ThresholdConfig, /// Which repository the runs belong to. pub repo: Option, /// How many changes to call out before the tables. @@ -56,9 +73,7 @@ impl BenchConfig { Ok(Self { allow: RegexSet::new(&file.allowlist)?, deny: RegexSet::new(&file.denylist)?, - improvement: file.improvement, - strong_improvement: file.strong_improvement, - regression: file.regression, + thresholds: file.thresholds, repo: file.repo, summary_limit: file.summary_limit, reproduce: file.reproduce, @@ -124,20 +139,21 @@ mod tests { #[test] fn reads_the_thresholds_when_given() { - let config = BenchConfig::parse("improvement = 1.5\nregression = 0.5").unwrap(); + let config = + BenchConfig::parse("[thresholds]\nimprovement = 1.5\nregression = 0.5").unwrap(); - assert_eq!(config.improvement, Some(1.5)); - assert_eq!(config.regression, Some(0.5)); - assert_eq!(config.strong_improvement, None); + assert_eq!(config.thresholds.improvement, Some(1.5)); + assert_eq!(config.thresholds.regression, Some(0.5)); + assert_eq!(config.thresholds.strong_improvement, None); } #[test] fn leaves_the_thresholds_alone_when_absent() { let config = BenchConfig::parse(r#"allowlist = ["^sandboxes/"]"#).unwrap(); - assert_eq!(config.improvement, None); - assert_eq!(config.strong_improvement, None); - assert_eq!(config.regression, None); + assert_eq!(config.thresholds.improvement, None); + assert_eq!(config.thresholds.strong_improvement, None); + assert_eq!(config.thresholds.regression, None); } #[test]