Introduce TestCase to generalize benchmark parameters

Author: Kobzol

site/src/comparison.rs

@@ -6,7 +6,7 @@ use crate::api;
 use crate::db::{ArtifactId, Benchmark, Lookup, Profile, Scenario};
 use crate::github;
 use crate::load::SiteCtxt;
-use crate::selector::{self};
+use crate::selector::{self, CompileBenchmarkQuery, CompileTestCase, TestCase};
 
 use collector::benchmark::category::Category;
 use collector::Bound;
@@ -798,27 +798,28 @@ async fn compare_given_commits(
     let aids = Arc::new(vec![a.clone(), b.clone()]);
 
     // get all crates, cache, and profile combinations for the given metric
-    let query = selector::CompileBenchmarkQuery::all_for_metric(metric);
+    let query = CompileBenchmarkQuery::all_for_metric(metric);
 
     // `responses` contains series iterators. The first element in the iterator is the data
     // for `a` and the second is the data for `b`
-    let mut responses = ctxt.compile_statistic_series(query, aids).await?;
+    let mut responses = ctxt.compile_statistic_series(query.clone(), aids).await?;
 
     let conn = ctxt.conn().await;
     let statistics_for_a = statistics_from_series(&mut responses);
     let statistics_for_b = statistics_from_series(&mut responses);
 
     let mut historical_data =
-        HistoricalDataMap::calculate(ctxt, a.clone(), master_commits, metric).await?;
+        HistoricalDataMap::<CompileTestCase>::calculate(ctxt, a.clone(), master_commits, query)
+            .await?;
     let comparisons = statistics_for_a
         .into_iter()
         .filter_map(|(test_case, a)| {
             statistics_for_b
                 .get(&test_case)
                 .map(|&b| CompileTestResultComparison {
-                    benchmark: test_case.0,
-                    profile: test_case.1,
-                    scenario: test_case.2,
+                    benchmark: test_case.benchmark,
+                    profile: test_case.profile,
+                    scenario: test_case.scenario,
                     comparison: TestResultComparison {
                         metric,
                         historical_data: historical_data.data.remove(&test_case),
@@ -877,8 +878,7 @@ pub struct ArtifactDescription {
     pub bootstrap_total: u64,
 }
 
-type StatisticsMap = HashMap<TestCase, f64>;
-type TestCase = (Benchmark, Profile, Scenario);
+type StatisticsMap<TestCase> = HashMap<TestCase, f64>;
 
 impl ArtifactDescription {
     /// For the given `ArtifactId`, consume the first datapoint in each of the given `SeriesResponse`
@@ -939,11 +939,13 @@ impl ArtifactDescription {
     }
 }
 
-fn statistics_from_series<T>(series: &mut [selector::SeriesResponse<T>]) -> StatisticsMap
+fn statistics_from_series<Case: TestCase, T>(
+    series: &mut [selector::SeriesResponse<Case, T>],
+) -> StatisticsMap<Case>
 where
     T: Iterator<Item = (ArtifactId, Option<f64>)>,
 {
-    let mut stats: StatisticsMap = HashMap::new();
+    let mut stats: StatisticsMap<Case> = HashMap::new();
     for response in series {
         let (_, point) = response.series.next().expect("must have element");
 
@@ -952,14 +954,7 @@ where
         } else {
             continue;
         };
-        stats.insert(
-            (
-                response.key.benchmark,
-                response.key.profile,
-                response.key.scenario,
-            ),
-            value,
-        );
+        stats.insert(response.test_case.clone(), value);
     }
     stats
 }
@@ -1053,19 +1048,19 @@ impl ArtifactComparison {
 }
 
 /// The historical data for a certain benchmark
-pub struct HistoricalDataMap {
+pub struct HistoricalDataMap<TestCase> {
     /// Historical data on a per test case basis
-    pub data: HashMap<(Benchmark, Profile, Scenario), HistoricalData>,
+    pub data: HashMap<TestCase, HistoricalData>,
 }
 
-impl HistoricalDataMap {
+impl HistoricalDataMap<CompileTestCase> {
     const NUM_PREVIOUS_COMMITS: usize = 30;
 
     async fn calculate(
         ctxt: &SiteCtxt,
         from: ArtifactId,
         master_commits: &[collector::MasterCommit],
-        metric: Metric,
+        query: CompileBenchmarkQuery,
     ) -> Result<Self, BoxedError> {
         let mut historical_data = HashMap::new();
 
@@ -1082,16 +1077,13 @@ impl HistoricalDataMap {
             });
         }
 
-        // get all crates, cache, and profile combinations for the given metric
-        let query = selector::CompileBenchmarkQuery::all_for_metric(metric);
-
         let mut previous_commit_series = ctxt
             .compile_statistic_series(query, previous_commits.clone())
             .await?;
 
         for _ in previous_commits.iter() {
-            for (test_case, stat) in statistics_from_series(&mut previous_commit_series) {
-                historical_data.entry(test_case).or_default().push(stat);
+            for (key, stat) in statistics_from_series(&mut previous_commit_series) {
+                historical_data.entry(key).or_default().push(stat);
             }
         }
 

site/src/request_handlers/graph.rs

@@ -7,7 +7,7 @@ use crate::api::{graph, graphs, ServerResult};
 use crate::db::{self, ArtifactId, Profile, Scenario};
 use crate::interpolate::IsInterpolated;
 use crate::load::SiteCtxt;
-use crate::selector::{CompileBenchmarkQuery, Selector, SeriesResponse};
+use crate::selector::{CompileBenchmarkQuery, CompileTestCase, Selector, SeriesResponse};
 
 pub async fn handle_graph(
     request: graph::Request,
@@ -119,9 +119,9 @@ async fn create_graphs(
     }
 
     for response in interpolated_responses {
-        let benchmark = response.key.benchmark.to_string();
-        let profile = response.key.profile;
-        let scenario = response.key.scenario.to_string();
+        let benchmark = response.test_case.benchmark.to_string();
+        let profile = response.test_case.profile;
+        let scenario = response.test_case.scenario.to_string();
         let graph_series = graph_series(response.series.into_iter(), request.kind);
 
         benchmarks
@@ -172,7 +172,10 @@ fn master_artifact_ids_for_range(ctxt: &SiteCtxt, start: Bound, end: Bound) -> V
 /// test cases per profile type
 fn create_summary(
     ctxt: &SiteCtxt,
-    interpolated_responses: &[SeriesResponse<Vec<((ArtifactId, Option<f64>), IsInterpolated)>>],
+    interpolated_responses: &[SeriesResponse<
+        CompileTestCase,
+        Vec<((ArtifactId, Option<f64>), IsInterpolated)>,
+    >],
     graph_kind: GraphKind,
 ) -> ServerResult<HashMap<Profile, HashMap<String, graphs::Series>>> {
     let mut baselines = HashMap::new();
@@ -188,8 +191,8 @@ fn create_summary(
                 let baseline_responses = interpolated_responses
                     .iter()
                     .filter(|sr| {
-                        let p = sr.key.profile;
-                        let s = sr.key.scenario;
+                        let p = sr.test_case.profile;
+                        let s = sr.test_case.scenario;
                         p == profile && s == Scenario::Empty
                     })
                     .map(|sr| sr.series.iter().cloned())
@@ -205,8 +208,8 @@ fn create_summary(
         let summary_case_responses = interpolated_responses
             .iter()
             .filter(|sr| {
-                let p = sr.key.profile;
-                let s = sr.key.scenario;
+                let p = sr.test_case.profile;
+                let s = sr.test_case.scenario;
                 p == profile && s == scenario
             })
             .map(|sr| sr.series.iter().cloned())

site/src/selector.rs

@@ -30,6 +30,7 @@ use database::{Benchmark, Commit, Index, Lookup};
 
 use crate::comparison::Metric;
 use std::fmt;
+use std::hash::Hash;
 use std::ops::RangeInclusive;
 use std::sync::Arc;
 
@@ -158,21 +159,28 @@ impl<T> Selector<T> {
     }
 }
 
+/// Represents the parameters of a single benchmark execution that collects a set of statistics.
+pub trait TestCase: std::fmt::Debug + Clone + Hash + Eq + PartialEq {}
+
 #[derive(Debug)]
-pub struct SeriesResponse<T> {
-    pub key: CompileBenchmarkKey,
+pub struct SeriesResponse<Case, T> {
+    pub test_case: Case,
     pub series: T,
 }
 
-impl<T> SeriesResponse<T> {
-    pub fn map<U>(self, m: impl FnOnce(T) -> U) -> SeriesResponse<U> {
+impl<TestCase, T> SeriesResponse<TestCase, T> {
+    pub fn map<U>(self, m: impl FnOnce(T) -> U) -> SeriesResponse<TestCase, U> {
+        let SeriesResponse {
+            test_case: key,
+            series,
+        } = self;
         SeriesResponse {
-            key: self.key,
-            series: m(self.series),
+            test_case: key,
+            series: m(series),
         }
     }
 
-    pub fn interpolate(self) -> SeriesResponse<Interpolate<T>>
+    pub fn interpolate(self) -> SeriesResponse<TestCase, Interpolate<T>>
     where
         T: Iterator,
         T::Item: crate::db::Point,
@@ -231,19 +239,21 @@ impl Default for CompileBenchmarkQuery {
     }
 }
 
-#[derive(Debug)]
-pub struct CompileBenchmarkKey {
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct CompileTestCase {
     pub benchmark: Benchmark,
     pub profile: Profile,
     pub scenario: Scenario,
 }
 
+impl TestCase for CompileTestCase {}
+
 impl SiteCtxt {
     pub async fn compile_statistic_series(
         &self,
         query: CompileBenchmarkQuery,
         artifact_ids: Arc<Vec<ArtifactId>>,
-    ) -> Result<Vec<SeriesResponse<StatisticSeries>>, String> {
+    ) -> Result<Vec<SeriesResponse<CompileTestCase, StatisticSeries>>, String> {
         StatisticSeries::execute_compile_query(artifact_ids, self, query).await
     }
 }
@@ -258,7 +268,7 @@ impl StatisticSeries {
         artifact_ids: Arc<Vec<ArtifactId>>,
         ctxt: &SiteCtxt,
         query: CompileBenchmarkQuery,
-    ) -> Result<Vec<SeriesResponse<Self>>, String> {
+    ) -> Result<Vec<SeriesResponse<CompileTestCase, Self>>, String> {
         let dumped = format!("{:?}", query);
 
         let index = ctxt.index.load();
@@ -318,7 +328,7 @@ impl StatisticSeries {
                             points.into_iter()
                         },
                     },
-                    key: CompileBenchmarkKey {
+                    test_case: CompileTestCase {
                         benchmark,
                         profile,
                         scenario,