Refactor bench params

src/json.rs

@@ -2,7 +2,9 @@ use core::fmt;
 use std::io::Read;
 use std::{fs::File, path::Path};
 
-use serde::Deserialize;
+use anyhow::{anyhow, bail};
+use chrono::{DateTime, Utc};
+use serde::{de, Deserialize};
 use serde_json::de::{StrRead, StreamDeserializer};
 use serde_json::{Deserializer, Error, Value};
 
@@ -17,7 +19,7 @@ pub struct BenchData {
 pub struct BenchId {
     pub group_name: String,
     pub bench_name: String,
-    pub params: String,
+    pub params: BenchParams,
 }
 
 // Assumes three `String` elements in a Criterion bench ID: <group>/<name>/<params>
@@ -31,20 +33,56 @@ impl<'de> Deserialize<'de> for BenchId {
         let s = String::deserialize(deserializer)?;
         let id = s.split('/').collect::<Vec<&str>>();
         if id.len() != 3 {
-            Err(serde::de::Error::custom("Expected 3 bench ID elements"))
+            Err(de::Error::custom("Expected 3 bench ID elements"))
         } else {
-            let bench_name = id[1].replace('_', ":");
             Ok(BenchId {
                 group_name: id[0].to_owned(),
-                // Criterion converts `:` to `_` in the timestamp as the former is valid JSON syntax,
-                // so we convert `_` back to `:` when deserializing
-                bench_name,
-                params: id[2].to_owned(),
+                bench_name: id[1].to_owned(),
+                params: BenchParams::try_from(id[2])
+                    .map_err(|e| de::Error::custom(format!("{}", e)))?,
             })
         }
     }
 }
 
+#[derive(Debug, PartialEq)]
+pub struct BenchParams {
+    pub commit_hash: String,
+    pub commit_timestamp: DateTime<Utc>,
+    pub params: String,
+}
+
+impl TryFrom<&str> for BenchParams {
+    type Error = anyhow::Error;
+    // Splits a <commit-hash>-<commit-date>-<params> input into a (String, `DateTime`, String) object
+    // E.g. `dd2a8e6-2024-02-20T22:48:21-05:00-rc-100` becomes ("dd2a8e6", `<DateTime>`, "rc-100")
+    fn try_from(value: &str) -> anyhow::Result<Self> {
+        let (commit_hash, rest) = value
+            .split_once('-')
+            .ok_or_else(|| anyhow!("Invalid format for bench params"))?;
+        let arr: Vec<&str> = rest.split_inclusive('-').collect();
+        // Criterion converts `:` to `_` in the timestamp as the former is valid JSON syntax,
+        // so we convert `_` back to `:` when deserializing
+        let mut date: String = arr[..4]
+            .iter()
+            .flat_map(|s| s.chars())
+            .collect::<String>()
+            .replace('_', ":");
+        date.pop();
+        let params = arr[4..].iter().flat_map(|s| s.chars()).collect();
+
+        let commit_timestamp = DateTime::parse_from_rfc3339(&date).map_or_else(
+            |e| bail!("Failed to parse string into `DateTime`: {}", e),
+            |dt| Ok(dt.with_timezone(&Utc)),
+        )?;
+        Ok(Self {
+            commit_hash: commit_hash.to_owned(),
+            commit_timestamp,
+            params,
+        })
+    }
+}
+
 #[derive(Debug, Deserialize)]
 pub struct BenchResult {
     #[serde(rename = "estimate")]
@@ -145,3 +183,21 @@ where
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::json::BenchParams;
+    use chrono::{DateTime, Utc};
+
+    #[test]
+    fn parse_bench_params() {
+        let s = "dd2a8e6-2024-02-20T22:48:21-05:00-rc-100";
+        let params = BenchParams::try_from(s).unwrap();
+        let params_expected = BenchParams {
+            commit_hash: "dd2a8e6".into(),
+            commit_timestamp: DateTime::parse_from_rfc3339("2024-02-20T22:48:21-05:00").map(|dt| dt.with_timezone(&Utc)).unwrap(),
+            params: "rc-100".into()
+        };
+        assert_eq!(params, params_expected);
+    }
+}

src/main.rs

@@ -11,25 +11,6 @@ use json::read_json_from_file;
 
 use crate::plot::{generate_plots, Plots};
 
-// TODO: Switch to camino
-// Gets all JSON paths in the current directory, optionally ending in a given suffix
-// E.g. if `suffix` is `abc1234.json` it will return "*abc1234.json"
-fn get_json_paths(suffix: Option<&str>) -> std::io::Result<Vec<std::path::PathBuf>> {
-    let suffix = suffix.unwrap_or(".json");
-    let entries = std::fs::read_dir(".")?
-        .flatten()
-        .filter_map(|e| {
-            let ext = e.path();
-            if ext.to_str()?.ends_with(suffix) {
-                Some(ext)
-            } else {
-                None
-            }
-        })
-        .collect::<Vec<_>>();
-    Ok(entries)
-}
-
 // Benchmark files to plot, e.g. `LURK_BENCH_FILES=fibonacci-abc1234,fibonacci-def5678`
 fn bench_files_env() -> anyhow::Result<Vec<String>> {
     std::env::var("LURK_BENCH_FILES")
@@ -71,6 +52,25 @@ fn write_plots_to_file(plot_data: &Plots) -> Result<(), io::Error> {
     file.write_all(json_data.as_bytes())
 }
 
+// TODO: Switch to camino
+// Gets all JSON paths in the current directory, optionally ending in a given suffix
+// E.g. if `suffix` is `abc1234.json` it will return "*abc1234.json"
+fn get_json_paths(suffix: Option<&str>) -> std::io::Result<Vec<std::path::PathBuf>> {
+    let suffix = suffix.unwrap_or(".json");
+    let entries = std::fs::read_dir(".")?
+        .flatten()
+        .filter_map(|e| {
+            let ext = e.path();
+            if ext.to_str()?.ends_with(suffix) {
+                Some(ext)
+            } else {
+                None
+            }
+        })
+        .collect::<Vec<_>>();
+    Ok(entries)
+}
+
 fn main() {
     // If existing plot data is found on disk, only read and add benchmark files specified by `LURK_BENCH_FILES`
     // Data is stored in a `HashMap` so duplicates are ignored

src/plot.rs

@@ -1,4 +1,3 @@
-use anyhow::bail;
 use plotters::prelude::*;
 
 use chrono::{serde::ts_seconds, DateTime, Duration, Utc};
@@ -89,20 +88,6 @@ fn style(idx: usize) -> PaletteColor<Palette99> {
     Palette99::pick(idx)
 }
 
-// Splits a <commit-hash>-<commit-date> input into a (String, `DateTime`) object
-fn parse_commit_str(input: &str) -> anyhow::Result<(String, DateTime<Utc>)> {
-    // Splits at the first `-` as the size is known (assumes UTF-8)
-    let (commit, date) = input.split_at(8);
-    let mut commit = commit.to_owned();
-    commit.pop();
-
-    let date = DateTime::parse_from_rfc3339(date).map_or_else(
-        |e| bail!("Failed to parse string into `DateTime`: {}", e),
-        |dt| Ok(dt.with_timezone(&Utc)),
-    )?;
-    Ok((commit, date))
-}
-
 // Plots of benchmark results over time/Git history. This data structure is persistent between runs,
 // saved to disk in `plot-data.json`, and is meant to be append-only to preserve historical results.
 //
@@ -123,26 +108,25 @@ impl Plots {
     // and adds the data to the `Plots` struct.
     pub fn add_data(&mut self, bench_data: &Vec<BenchData>) {
         for bench in bench_data {
-            let (commit_hash, commit_date) =
-                parse_commit_str(&bench.id.bench_name).expect("Timestamp parse error");
+            let id = &bench.id;
             let point = Point {
-                x: commit_date,
+                x: id.params.commit_timestamp,
                 y: bench.result.time,
-                label: commit_hash,
+                label: id.params.commit_hash.clone(),
             };
 
-            if self.0.get(&bench.id.group_name).is_none() {
-                self.0.insert(bench.id.group_name.to_owned(), Plot::new());
+            if self.0.get(&id.group_name).is_none() {
+                self.0.insert(id.group_name.to_owned(), Plot::new());
             }
-            let plot = self.0.get_mut(&bench.id.group_name).unwrap();
+            let plot = self.0.get_mut(&id.group_name).unwrap();
 
-            plot.x_axis.set_min_max(commit_date);
+            plot.x_axis.set_min_max(id.params.commit_timestamp);
             plot.y_axis.set_min_max(point.y);
 
-            if plot.lines.get(&bench.id.params).is_none() {
-                plot.lines.insert(bench.id.params.to_owned(), vec![]);
+            if plot.lines.get(&id.params.params).is_none() {
+                plot.lines.insert(id.params.params.to_owned(), vec![]);
             }
-            plot.lines.get_mut(&bench.id.params).unwrap().push(point);
+            plot.lines.get_mut(&id.params.params).unwrap().push(point);
         }
         // Sort each data point in each line for each plot
         for plot in self.0.iter_mut() {