Post a summary table to PRs after perf. runs

Author: Kobzol

site/src/comparison.rs

@@ -421,6 +421,61 @@ impl ComparisonSummary {
             .unwrap();
         }
     }
+
+    /// Writes a Markdown table containing summary of relevant results.
+    pub fn write_summary_table(&self, result: &mut String) {
+        use std::fmt::Write;
+
+        fn render_stat<F: FnOnce() -> Option<f64>>(count: usize, calculate: F) -> String {
+            let value = if count > 0 {
+                calculate()
+            } else {
+                None
+            };
+            value.map(|value| format!("{value:.1}%")).unwrap_or_else(|| "N/A".to_string())
+        }
+
+        writeln!(
+            result,
+            r#"| | Regressions 😿 | Improvements 🎉 | All relevant changes |
+|:---:|:---:|:---:|:---:|"#
+        )
+        .unwrap();
+        writeln!(
+            result,
+            "| count[^1] | {} | {} | {} |",
+            self.num_regressions,
+            self.num_improvements,
+            self.num_regressions + self.num_improvements
+        )
+        .unwrap();
+        writeln!(
+            result,
+            "| mean[^2] | {} | {} | {:.1}% |",
+            render_stat(self.num_regressions, || Some(self.arithmetic_mean_of_regressions())),
+            render_stat(self.num_improvements, || Some(self.arithmetic_mean_of_improvements())),
+            self.arithmetic_mean_of_changes()
+        )
+            .unwrap();
+
+        let largest_change = self.most_relevant_changes().iter().fold(0.0, |accum: f64, item| {
+            let change = item.map(|v| v.relative_change() * 100.0).unwrap_or(0.0);
+            accum.max(change)
+        });
+
+        writeln!(
+            result,
+            "| max | {} | {} | {:.1}% |",
+            render_stat(self.num_regressions, || self.largest_regression().map(|r| r.relative_change() * 100.0)),
+            render_stat(self.num_improvements, || self.largest_improvement().map(|r| r.relative_change() * 100.0)),
+            largest_change
+        )
+            .unwrap();
+
+        writeln!(result, r#"
+[^1]: *number of relevant changes*
+[^2]: *the arithmetic mean of the percent change*"#).unwrap();
+    }
 }
 
 /// The amount of confidence we have that a comparison actually represents a real
@@ -1233,3 +1288,102 @@ fn compare_link(start: &ArtifactId, end: &ArtifactId) -> String {
         start, end
     )
 }
+
+#[cfg(test)]
+mod tests {
+    use std::collections::HashSet;
+
+    use database::{ArtifactId, Profile, Scenario};
+
+    use crate::comparison::{
+        ArtifactDescription, Comparison, ComparisonSummary, TestResultComparison,
+    };
+
+    #[test]
+    fn summary_table_only_improvements() {
+        let summary = create_summary(vec![(10.0, 5.0), (8.0, 2.0)]);
+        check_table(
+            summary, r#"
+| | Regressions 😿 | Improvements 🎉 | All relevant changes |
+|:---:|:---:|:---:|:---:|
+| count[^1] | 0 | 2 | 2 |
+| mean[^2] | N/A | -62.5% | -62.5% |
+| max | N/A | -75.0% | 0.0% |
+
+[^1]: *number of relevant changes*
+[^2]: *the arithmetic mean of the percent change*
+"#.trim_start(),
+        );
+    }
+
+    #[test]
+    fn summary_table_only_regressions() {
+        let summary = create_summary(vec![(5.0, 10.0), (1.0, 3.0)]);
+        check_table(
+            summary, r#"
+| | Regressions 😿 | Improvements 🎉 | All relevant changes |
+|:---:|:---:|:---:|:---:|
+| count[^1] | 2 | 0 | 2 |
+| mean[^2] | 150.0% | N/A | 150.0% |
+| max | 200.0% | N/A | 200.0% |
+
+[^1]: *number of relevant changes*
+[^2]: *the arithmetic mean of the percent change*
+"#.trim_start(),
+        );
+    }
+
+    #[test]
+    fn summary_table_mixed() {
+        let summary = create_summary(vec![(10.0, 5.0), (5.0, 10.0), (1.0, 3.0), (4.0, 1.0)]);
+        check_table(
+            summary, r#"
+| | Regressions 😿 | Improvements 🎉 | All relevant changes |
+|:---:|:---:|:---:|:---:|
+| count[^1] | 2 | 2 | 4 |
+| mean[^2] | 150.0% | -62.5% | 43.8% |
+| max | 200.0% | -75.0% | 200.0% |
+
+[^1]: *number of relevant changes*
+[^2]: *the arithmetic mean of the percent change*
+"#.trim_start(),
+        );
+    }
+
+    fn check_table(summary: ComparisonSummary, expected: &str) {
+        let mut result = String::new();
+        summary.write_summary_table(&mut result);
+        assert_eq!(result, expected);
+    }
+
+    fn create_summary(values: Vec<(f64, f64)>) -> ComparisonSummary {
+        let mut statistics = HashSet::new();
+        for (index, diff) in values.into_iter().enumerate() {
+            statistics.insert(TestResultComparison {
+                benchmark: index.to_string().as_str().into(),
+                profile: Profile::Check,
+                scenario: Scenario::Empty,
+                variance: None,
+                results: diff,
+            });
+        }
+
+        let comparison = Comparison {
+            a: ArtifactDescription {
+                artifact: ArtifactId::Tag("a".to_string()),
+                pr: None,
+                bootstrap: Default::default(),
+                bootstrap_total: 0,
+            },
+            b: ArtifactDescription {
+                artifact: ArtifactId::Tag("b".to_string()),
+                pr: None,
+                bootstrap: Default::default(),
+                bootstrap_total: 0,
+            },
+            statistics,
+            new_errors: Default::default(),
+        };
+        ComparisonSummary::summarize_comparison(&comparison).unwrap()
+    }
+}

site/src/github.rs

@@ -709,31 +709,14 @@ async fn categorize_benchmark(
     let num_improvements = summary.number_of_improvements();
     let num_regressions = summary.number_of_regressions();
 
-    let description = match direction {
-        Direction::Improvement => format!(
-            "{} relevant {} 🎉",
-            num_improvements,
-            ending("improvement", num_improvements)
-        ),
-        Direction::Regression => format!(
-            "{} relevant {} 😿",
-            num_regressions,
-            ending("regression", num_regressions)
-        ),
-        Direction::Mixed => format!(
-            "{} relevant {} 🎉 but {} relevant {} 😿",
-            num_improvements,
-            ending("improvement", num_improvements),
-            num_regressions,
-            ending("regression", num_regressions)
-        ),
+    let short_summary = match direction {
+        Direction::Improvement => format!("🎉 relevant {} found", ending("improvement", num_improvements)),
+        Direction::Regression => format!("😿 relevant {} found", ending("regression", num_regressions)),
+        Direction::Mixed => "mixed results".to_string(),
     };
-    let mut result = format!(
-        "This benchmark run shows {} to instruction counts.\n",
-        description
-    );
 
-    summary.write_summary_lines(&mut result, None);
+    let mut result = format!("Summary: {short_summary}\n");
+    summary.write_summary_table(&mut result);
     write!(result, "\n{}", DISAGREEMENT).unwrap();
     (result, Some(direction))
 }