From 7f7873b22a864c4f2bcbe789d6f371486028e8f2 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 12 Aug 2022 15:15:06 +0200 Subject: [PATCH 1/2] Add the metric in question to markdown table --- site/src/comparison.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/site/src/comparison.rs b/site/src/comparison.rs index be0addd7f..b1cdf4819 100644 --- a/site/src/comparison.rs +++ b/site/src/comparison.rs @@ -525,6 +525,16 @@ pub fn write_summary_table( with_footnotes: bool, result: &mut String, ) { + let metric = match primary + .relevant_comparisons + .first() + .or(secondary.relevant_comparisons.first()) + .map(|m| m.metric.as_str()) + { + Some(m) => m, + None => return, + }; + fn render_stat Option>(count: usize, calculate: F) -> String { let value = if count > 0 { calculate() } else { None }; value @@ -627,7 +637,7 @@ pub fn write_summary_table( // This code attempts to space the table cells evenly so that the data is // easy to read for anyone who is viewing the Markdown source. let column_labels = [ - " ".to_string(), // we want at least 10 spaces to accommodate "count[^2]" + format!("({metric})",), format!("mean{}", if with_footnotes { "[^1]" } else { "" }), "max".to_string(), format!("count{}", if with_footnotes { "[^2]" } else { "" }), @@ -1383,8 +1393,6 @@ mod tests { (Category::Primary, 1.0, 3.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | 146.7% | 200.0% | 3 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | - | - | 0 | @@ -1404,8 +1412,6 @@ mod tests { (Category::Primary, 4.0, 1.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | - | - | 0 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | -71.7% | -80.0% | 3 | @@ -1425,8 +1431,6 @@ mod tests { (Category::Secondary, 4.0, 1.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | - | - | 0 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | - | - | 0 | @@ -1446,8 +1450,6 @@ mod tests { (Category::Secondary, 1.0, 3.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | - | - | 0 | | Regressions ❌
(secondary) | 146.7% | 200.0% | 3 | | Improvements ✅
(primary) | - | - | 0 | @@ -1468,8 +1470,6 @@ mod tests { (Category::Primary, 4.0, 1.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | 150.0% | 200.0% | 2 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | -62.5% | -75.0% | 2 | @@ -1492,8 +1492,6 @@ mod tests { (Category::Primary, 4.0, 1.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | 150.0% | 200.0% | 2 | | Regressions ❌
(secondary) | 100.0% | 100.0% | 1 | | Improvements ✅
(primary) | -62.5% | -75.0% | 2 | @@ -1512,8 +1510,6 @@ mod tests { (Category::Primary, 5.0, 6.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | 20.0% | 20.0% | 1 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | -50.0% | -50.0% | 1 | @@ -1532,8 +1528,6 @@ mod tests { (Category::Primary, 6.0, 5.0), ], r#" -| | mean[^1] | max | count[^2] | -|:----------:|:--------:|:---:|:---------:| | Regressions ❌
(primary) | 100.0% | 100.0% | 1 | | Regressions ❌
(secondary) | - | - | 0 | | Improvements ✅
(primary) | -16.7% | -16.7% | 1 | @@ -1588,6 +1582,7 @@ mod tests { let mut result = String::new(); write_summary_table(&primary, &secondary, true, &mut result); - assert_eq!(result, expected); + let header = "| (instructions:u) | mean[^1] | max | count[^2] |\n|:----------------:|:--------:|:---:|:---------:|\n"; + assert_eq!(result, format!("{header}{expected}")); } } From b24b75e0adc953a42e0d7169f33f2da44279d2c0 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 12 Aug 2022 16:04:49 +0200 Subject: [PATCH 2/2] Only add metric in triage --- site/src/comparison.rs | 29 +++++++++++++++------------ site/src/github/comparison_summary.rs | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/site/src/comparison.rs b/site/src/comparison.rs index b1cdf4819..8ac49a820 100644 --- a/site/src/comparison.rs +++ b/site/src/comparison.rs @@ -96,7 +96,7 @@ pub async fn handle_triage( .clone() .summarize_by_category(&benchmark_map); let mut result = String::from("**Summary**:\n\n"); - write_summary_table(&primary, &secondary, false, &mut result); + write_summary_table(&primary, &secondary, false, true, &mut result); result } None => String::from("**ERROR**: no data found for end bound"), @@ -513,7 +513,7 @@ async fn write_triage_summary( let link = &compare_link(start, end); write!(&mut result, " [(Comparison Link)]({})\n\n", link).unwrap(); - write_summary_table(&primary, &secondary, false, &mut result); + write_summary_table(&primary, &secondary, false, true, &mut result); result } @@ -523,17 +523,20 @@ pub fn write_summary_table( primary: &ArtifactComparisonSummary, secondary: &ArtifactComparisonSummary, with_footnotes: bool, + include_metric: bool, result: &mut String, ) { - let metric = match primary - .relevant_comparisons - .first() - .or(secondary.relevant_comparisons.first()) - .map(|m| m.metric.as_str()) - { - Some(m) => m, - None => return, - }; + let metric = include_metric + .then(|| { + primary + .relevant_comparisons + .first() + .or(secondary.relevant_comparisons.first()) + .map(|m| m.metric.as_str()) + }) + .flatten() + // we want at least 10 spaces to accommodate "count[^2]" + .unwrap_or(" "); fn render_stat Option>(count: usize, calculate: F) -> String { let value = if count > 0 { calculate() } else { None }; @@ -637,7 +640,7 @@ pub fn write_summary_table( // This code attempts to space the table cells evenly so that the data is // easy to read for anyone who is viewing the Markdown source. let column_labels = [ - format!("({metric})",), + format!("({metric})"), format!("mean{}", if with_footnotes { "[^1]" } else { "" }), "max".to_string(), format!("count{}", if with_footnotes { "[^2]" } else { "" }), @@ -1581,7 +1584,7 @@ mod tests { let secondary = ArtifactComparisonSummary::summarize(secondary_comparisons); let mut result = String::new(); - write_summary_table(&primary, &secondary, true, &mut result); + write_summary_table(&primary, &secondary, true, true, &mut result); let header = "| (instructions:u) | mean[^1] | max | count[^2] |\n|:----------------:|:--------:|:---:|:---------:|\n"; assert_eq!(result, format!("{header}{expected}")); } diff --git a/site/src/github/comparison_summary.rs b/site/src/github/comparison_summary.rs index 1a8b78db0..153e2e185 100644 --- a/site/src/github/comparison_summary.rs +++ b/site/src/github/comparison_summary.rs @@ -214,7 +214,7 @@ fn write_metric_summary( ) .unwrap(); - write_summary_table(&primary, &secondary, true, message); + write_summary_table(&primary, &secondary, true, false, message); if hidden { message.push_str("\n");