Send variance to frontend

Author: rylev

site/src/api.rs

@@ -70,25 +70,6 @@ pub struct CommitResponse {
     pub commit: Option<String>,
 }
 
-pub mod data {
-    use crate::comparison::ArtifactData;
-    use collector::Bound;
-    use serde::{Deserialize, Serialize};
-
-    #[derive(Debug, Clone, Serialize, Deserialize)]
-    pub struct Request {
-        pub start: Bound,
-        pub end: Bound,
-
-        /// Which statistic to return data for
-        pub stat: String,
-    }
-
-    /// List of DateData's from oldest to newest
-    #[derive(Debug, Clone, Serialize)]
-    pub struct Response(pub Vec<ArtifactData>);
-}
-
 pub mod graph {
     use collector::Bound;
     use serde::{Deserialize, Serialize};
@@ -157,6 +138,7 @@ pub mod bootstrap {
 }
 
 pub mod comparison {
+    use crate::comparison;
     use collector::Bound;
     use database::Date;
     use serde::{Deserialize, Serialize};
@@ -171,6 +153,8 @@ pub mod comparison {
 
     #[derive(Debug, Clone, Serialize)]
     pub struct Response {
+        /// The variance data for each benchmark, if any.
+        pub variance: Option<HashMap<String, comparison::BenchmarkVariance>>,
         /// The names for the previous artifact before `a`, if any.
         pub prev: Option<String>,
 

site/src/comparison.rs

@@ -8,6 +8,7 @@ use crate::load::SiteCtxt;
 use crate::selector::{self, Tag};
 
 use collector::Bound;
+use log::debug;
 use serde::Serialize;
 
 use std::collections::HashMap;
@@ -100,6 +101,7 @@ pub async fn handle_compare(
     let is_contiguous = comparison.is_contiguous(&*conn, &master_commits).await;
 
     Ok(api::comparison::Response {
+        variance: comparison.benchmark_variances.map(|b| b.data),
         prev,
         a: comparison.a.into(),
         b: comparison.b.into(),
@@ -237,21 +239,6 @@ async fn compare_given_commits(
         Some(b) => b,
         None => return Ok(None),
     };
-    let mut prevs = Vec::with_capacity(10);
-    let mut commit = a.clone();
-    while prevs.len() < 100 {
-        match prev_commit(&commit, master_commits) {
-            Some(c) => {
-                let new = ArtifactId::Commit(database::Commit {
-                    sha: c.sha.clone(),
-                    date: database::Date(c.time),
-                });
-                commit = new.clone();
-                prevs.push(new);
-            }
-            None => break,
-        }
-    }
     let aids = Arc::new(vec![a.clone(), b.clone()]);
 
     // get all crates, cache, and profile combinations for the given stat
@@ -264,58 +251,40 @@ async fn compare_given_commits(
     // `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.query::<Option<f64>>(query.clone(), aids).await?;
-    let mut rps = ctxt
-        .query::<Option<f64>>(query, Arc::new(prevs.clone()))
-        .await?;
 
     let conn = ctxt.conn().await;
-
-    let mut others = Vec::new();
-    for aid in prevs {
-        others.push(ArtifactData::consume_one(&*conn, aid, &mut rps, master_commits).await)
-    }
-    // println!("{:#?}", others.iter().map(|o| &o.data).collect::<Vec<_>>());
-    let mut h: HashMap<String, Vec<f64>> = HashMap::new();
-    for o in others {
-        let data = &o.data;
-        for (k, v) in data {
-            for (c, val) in v {
-                h.entry(format!("{}-{}", k, c)).or_default().push(*val);
-            }
-        }
-    }
-    for (bench, results) in h {
-        println!("Bechmark: {}", bench);
-        let results_len = results.len();
-        let results_mean = results.iter().sum::<f64>() / results.len() as f64;
-        let mut deltas = results
-            .windows(2)
-            .map(|window| (window[0] - window[1]).abs())
-            .collect::<Vec<_>>();
-        deltas.sort_by(|d1, d2| d1.partial_cmp(d2).unwrap_or(std::cmp::Ordering::Equal));
-        let non_significant = deltas
-            .iter()
-            .zip(results)
-            .take_while(|(&d, r)| d / r < 0.05)
-            .collect::<Vec<_>>();
-        println!(
-            "Significant changes: {}",
-            results_len - non_significant.len()
-        );
-        let mean =
-            non_significant.iter().map(|(&d, _)| d).sum::<f64>() / (non_significant.len() as f64);
-        let coefficient_of_variance = (mean / results_mean) * 100.0;
-        println!("\tCoefficient of variance: {:.3}%", coefficient_of_variance);
-    }
-
     Ok(Some(Comparison {
+        benchmark_variances: BenchmarkVariances::calculate(ctxt, a.clone(), master_commits, stat)
+            .await?,
         a: ArtifactData::consume_one(&*conn, a.clone(), &mut responses, master_commits).await,
         b: ArtifactData::consume_one(&*conn, b.clone(), &mut responses, master_commits).await,
     }))
 }
 
+fn previous_commits(
+    mut from: ArtifactId,
+    n: usize,
+    master_commits: &[collector::MasterCommit],
+) -> Vec<ArtifactId> {
+    let mut prevs = Vec::with_capacity(n);
+    while prevs.len() < n {
+        match prev_commit(&from, master_commits) {
+            Some(c) => {
+                let new = ArtifactId::Commit(database::Commit {
+                    sha: c.sha.clone(),
+                    date: database::Date(c.time),
+                });
+                from = new.clone();
+                prevs.push(new);
+            }
+            None => break,
+        }
+    }
+    prevs
+}
+
 /// Data associated with a specific artifact
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone)]
 pub struct ArtifactData {
     /// The artifact in question
     pub artifact: ArtifactId,
@@ -345,25 +314,7 @@ impl ArtifactData {
     where
         T: Iterator<Item = (ArtifactId, Option<f64>)>,
     {
-        let mut data = HashMap::new();
-
-        for response in series {
-            let (id, point) = response.series.next().expect("must have element");
-            assert_eq!(artifact, id);
-
-            let point = if let Some(pt) = point {
-                pt
-            } else {
-                continue;
-            };
-            data.entry(format!(
-                "{}-{}",
-                response.path.get::<Crate>().unwrap(),
-                response.path.get::<Profile>().unwrap(),
-            ))
-            .or_insert_with(Vec::new)
-            .push((response.path.get::<Cache>().unwrap().to_string(), point));
-        }
+        let data = data_from_series(series);
 
         let bootstrap = conn
             .get_bootstrap(&[conn.artifact_id(&artifact).await])
@@ -405,6 +356,32 @@ impl ArtifactData {
     }
 }
 
+fn data_from_series<T>(
+    series: &mut [selector::SeriesResponse<T>],
+) -> HashMap<String, Vec<(String, f64)>>
+where
+    T: Iterator<Item = (ArtifactId, Option<f64>)>,
+{
+    let mut data = HashMap::new();
+    for response in series {
+        let (_, point) = response.series.next().expect("must have element");
+
+        let point = if let Some(pt) = point {
+            pt
+        } else {
+            continue;
+        };
+        data.entry(format!(
+            "{}-{}",
+            response.path.get::<Crate>().unwrap(),
+            response.path.get::<Profile>().unwrap(),
+        ))
+        .or_insert_with(Vec::new)
+        .push((response.path.get::<Cache>().unwrap().to_string(), point));
+    }
+    data
+}
+
 impl From<ArtifactData> for api::comparison::ArtifactData {
     fn from(data: ArtifactData) -> Self {
         api::comparison::ArtifactData {
@@ -426,6 +403,10 @@ impl From<ArtifactData> for api::comparison::ArtifactData {
 
 // A comparison of two artifacts
 pub struct Comparison {
+    /// Data on how variable benchmarks have historically been
+    ///
+    /// Is `None` if we cannot determine historical variance
+    pub benchmark_variances: Option<BenchmarkVariances>,
     pub a: ArtifactData,
     pub b: ArtifactData,
 }
@@ -482,6 +463,144 @@ impl Comparison {
     }
 }
 
+/// A description of the amount of variance a certain benchmark is historically
+/// experiencing at a given point in time.
+pub struct BenchmarkVariances {
+    /// Variance data on a per benchmark basis
+    /// Key: $benchmark-$profile-$cache
+    /// Value: `BenchmarkVariance`
+    pub data: HashMap<String, BenchmarkVariance>,
+}
+
+impl BenchmarkVariances {
+    async fn calculate(
+        ctxt: &SiteCtxt,
+        from: ArtifactId,
+        master_commits: &[collector::MasterCommit],
+        stat: String,
+    ) -> Result<Option<Self>, BoxedError> {
+        // get all crates, cache, and profile combinations for the given stat
+        let query = selector::Query::new()
+            .set::<String>(Tag::Crate, selector::Selector::All)
+            .set::<String>(Tag::Cache, selector::Selector::All)
+            .set::<String>(Tag::Profile, selector::Selector::All)
+            .set(Tag::ProcessStatistic, selector::Selector::One(stat));
+
+        let num_commits = 100;
+        let previous_commits = Arc::new(previous_commits(from, num_commits, master_commits));
+        if previous_commits.len() < num_commits {
+            return Ok(None);
+        }
+        let mut previous_commit_series = ctxt
+            .query::<Option<f64>>(query, previous_commits.clone())
+            .await?;
+
+        let mut variance_data: HashMap<String, BenchmarkVariance> = HashMap::new();
+        for _ in previous_commits.iter() {
+            let series_data = data_from_series(&mut previous_commit_series);
+            for (bench_and_profile, data) in series_data {
+                for (cache, val) in data {
+                    variance_data
+                        .entry(format!("{}-{}", bench_and_profile, cache))
+                        .or_default()
+                        .push(val);
+                }
+            }
+        }
+
+        for (bench, results) in variance_data.iter_mut() {
+            debug!("Calculating variance for: {}", bench);
+            results.calculate_description();
+        }
+        Ok(Some(Self {
+            data: variance_data,
+        }))
+    }
+}
+
+#[derive(Debug, Default, Clone, Serialize)]
+pub struct BenchmarkVariance {
+    data: Vec<f64>,
+    description: BenchmarkVarianceDescription,
+}
+
+impl BenchmarkVariance {
+    /// The ratio of change that we consider significant.
+    const SIGNFICANT_DELTA_THRESHOLD: f64 = 0.02;
+    /// The percentage of significant changes that we consider too high
+    const SIGNFICANT_CHANGE_THRESHOLD: f64 = 5.0;
+    /// The percentage of change that constitutes noisy data
+    const NOISE_THRESHOLD: f64 = 0.1;
+
+    fn push(&mut self, value: f64) {
+        self.data.push(value);
+    }
+
+    fn mean(&self) -> f64 {
+        self.data.iter().sum::<f64>() / self.data.len() as f64
+    }
+
+    fn calculate_description(&mut self) {
+        self.description = BenchmarkVarianceDescription::Normal;
+
+        let results_mean = self.mean();
+        let mut deltas = self
+            .data
+            .windows(2)
+            .map(|window| (window[0] - window[1]).abs())
+            .collect::<Vec<_>>();
+        deltas.sort_by(|d1, d2| d1.partial_cmp(d2).unwrap_or(std::cmp::Ordering::Equal));
+        let non_significant = deltas
+            .iter()
+            .zip(self.data.iter())
+            .take_while(|(&d, &r)| d / r < Self::SIGNFICANT_DELTA_THRESHOLD)
+            .collect::<Vec<_>>();
+
+        let percent_significant_changes =
+            ((self.data.len() - non_significant.len()) as f64 / self.data.len() as f64) * 100.0;
+        debug!(
+            "Percent significant changes: {:.1}%",
+            percent_significant_changes
+        );
+
+        if percent_significant_changes >= Self::SIGNFICANT_CHANGE_THRESHOLD {
+            self.description =
+                BenchmarkVarianceDescription::HighlyVariable(percent_significant_changes);
+            return;
+        }
+
+        let delta_mean =
+            non_significant.iter().map(|(&d, _)| d).sum::<f64>() / (non_significant.len() as f64);
+        let percent_change = (delta_mean / results_mean) * 100.0;
+        debug!("Percent change: {:.3}%", percent_change);
+        if percent_change > Self::NOISE_THRESHOLD {
+            self.description = BenchmarkVarianceDescription::Noisy(percent_change);
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy, Serialize)]
+#[serde(tag = "type", content = "percent")]
+pub enum BenchmarkVarianceDescription {
+    Normal,
+    /// A highly variable benchmark that produces many significant changes.
+    /// This might indicate a benchmark which is very sensitive to compiler changes.
+    ///
+    /// Cotains the percentage of significant changes.
+    HighlyVariable(f64),
+    /// A noisy benchmark which is likely to see changes in performance simply between
+    /// compiler runs.
+    ///
+    /// Contains the percent change that happens on average
+    Noisy(f64),
+}
+
+impl Default for BenchmarkVarianceDescription {
+    fn default() -> Self {
+        Self::Normal
+    }
+}
+
 /// Gets the previous commit
 pub fn prev_commit<'a>(
     artifact: &ArtifactId,

site/src/server.rs

@@ -23,8 +23,8 @@ use serde::de::DeserializeOwned;
 use serde::Serialize;
 
 pub use crate::api::{
-    self, bootstrap, comparison, dashboard, data, github, graph, info, self_profile,
-    self_profile_raw, status, triage, CommitResponse, ServerResult, StyledBenchmarkName,
+    self, bootstrap, comparison, dashboard, github, graph, info, self_profile, self_profile_raw,
+    status, triage, CommitResponse, ServerResult, StyledBenchmarkName,
 };
 use crate::db::{self, ArtifactId, Cache, Crate, Lookup, Profile};
 use crate::interpolate::Interpolated;