Change how we measure absolute magnitude to depend on the metric

Author: rylev

docs/comparison-analysis.md

@@ -58,6 +58,7 @@ The relevance test run summary is determined by the number of significant and re
 
 Magnitude is a combination of two factors:
 * how large a change is regardless of the direction of the change
+  * The point at which a change is considered in a different magnitude category depends on the metric being analyzed.
 * how much that change went over the significance threshold
 
 As an example, if a change that is large in absolute terms only exceeds the significance threshold by a small factor, then the overall magnitude of the change is considered small.

site/src/comparison.rs

@@ -210,6 +210,31 @@ impl Metric {
             Self::CpuClock => "cpu-clock",
         }
     }
+
+    /// Determines the magntiude of a relative change in percentage terms
+    ///
+    /// Takes into account how noisy the stat is. For example, instruction
+    /// count which is normally not very noisy has smaller thresholds than
+    /// max-rss which can be noisy.
+    fn absolute_magnitude(&self, change: f64) -> Magnitude {
+        let noise_factor = if self.is_typically_noisy() { 2.0 } else { 1.0 };
+        let change = change / noise_factor;
+        if change < 0.2 {
+            Magnitude::VerySmall
+        } else if change < 1.0 {
+            Magnitude::Small
+        } else if change < 2.0 {
+            Magnitude::Medium
+        } else if change < 5.0 {
+            Magnitude::Large
+        } else {
+            Magnitude::VeryLarge
+        }
+    }
+
+    fn is_typically_noisy(&self) -> bool {
+        !matches!(self, Self::Instructions)
+    }
 }
 
 /// A summary of a given comparison
@@ -638,6 +663,7 @@ async fn compare_given_commits(
                     benchmark: test_case.0,
                     profile: test_case.1,
                     scenario: test_case.2,
+                    stat,
                     historical_data: historical_data.data.remove(&test_case),
                     results: (a, b),
                 })
@@ -1015,6 +1041,7 @@ pub struct TestResultComparison {
     benchmark: Benchmark,
     profile: Profile,
     scenario: Scenario,
+    stat: Stat,
     historical_data: Option<HistoricalData>,
     results: (f64, f64),
 }
@@ -1083,17 +1110,7 @@ impl TestResultComparison {
         } else {
             Magnitude::VeryLarge
         };
-        let absolute_magnitude = if change < 0.002 {
-            Magnitude::VerySmall
-        } else if change < 0.01 {
-            Magnitude::Small
-        } else if change < 0.02 {
-            Magnitude::Medium
-        } else if change < 0.05 {
-            Magnitude::Large
-        } else {
-            Magnitude::VeryLarge
-        };
+        let absolute_magnitude = self.stat.absolute_magnitude(change * 100.0);
         fn as_u8(m: Magnitude) -> u8 {
             match m {
                 Magnitude::VerySmall => 1,
@@ -1457,6 +1474,7 @@ mod tests {
                 benchmark: index.to_string().as_str().into(),
                 profile: Profile::Check,
                 scenario: Scenario::Empty,
+                stat: Stat::Instructions,
                 historical_data: None,
                 results: (before, after),
             });