From 3d985a12f7603f89b082dda9a500c5b84cbe8f3b Mon Sep 17 00:00:00 2001 From: Wenyu Zhao Date: Fri, 17 May 2024 12:59:12 +1000 Subject: [PATCH] Refactor `Value` --- harness/src/bencher.rs | 104 ++++++++++++++++++++++++++++++++--------- harness/src/probe.rs | 33 +++++++------ harness/src/record.rs | 18 +++---- probes/perf/src/lib.rs | 6 +-- 4 files changed, 113 insertions(+), 48 deletions(-) diff --git a/harness/src/bencher.rs b/harness/src/bencher.rs index 47043e5..ab0f97e 100644 --- a/harness/src/bencher.rs +++ b/harness/src/bencher.rs @@ -1,3 +1,5 @@ +use std::convert::TryFrom; +use std::fmt; use std::{ cell::RefCell, path::PathBuf, @@ -44,23 +46,83 @@ pub struct BenchArgs { pub current_build: Option, } -pub trait Value: ToString + 'static {} +#[derive(Debug, Clone, Copy)] +pub enum Value { + F64(f64), + F32(f32), + Usize(usize), + Isize(isize), + U64(u64), + I64(i64), + U32(u32), + I32(i32), + U16(u16), + I16(i16), + U8(u8), + I8(i8), + Bool(bool), +} + +impl Value { + pub(crate) fn into_string(self) -> String { + match self { + Value::F64(v) => v.to_string(), + Value::F32(v) => v.to_string(), + Value::Usize(v) => v.to_string(), + Value::Isize(v) => v.to_string(), + Value::U64(v) => v.to_string(), + Value::I64(v) => v.to_string(), + Value::U32(v) => v.to_string(), + Value::I32(v) => v.to_string(), + Value::U16(v) => v.to_string(), + Value::I16(v) => v.to_string(), + Value::U8(v) => v.to_string(), + Value::I8(v) => v.to_string(), + Value::Bool(v) => v.to_string(), + } + } +} + +macro_rules! impl_helper_traits { + ($variant: ident, $t:ty) => { + impl From<$t> for Value { + fn from(v: $t) -> Self { + Value::$variant(v) + } + } -impl Value for f64 {} -impl Value for f32 {} -impl Value for usize {} -impl Value for isize {} -impl Value for u128 {} -impl Value for i128 {} -impl Value for u64 {} -impl Value for i64 {} -impl Value for u32 {} -impl Value for i32 {} -impl Value for u16 {} -impl Value for i16 {} -impl Value for u8 {} -impl Value for i8 {} -impl Value for bool {} + impl TryFrom for $t { + type Error = (); + + fn try_from(v: Value) -> Result { + match v { + Value::$variant(v) => Ok(v), + _ => Err(()), + } + } + } + }; +} + +impl_helper_traits!(F64, f64); +impl_helper_traits!(F32, f32); +impl_helper_traits!(Usize, usize); +impl_helper_traits!(Isize, isize); +impl_helper_traits!(U64, u64); +impl_helper_traits!(I64, i64); +impl_helper_traits!(U32, u32); +impl_helper_traits!(I32, i32); +impl_helper_traits!(U16, u16); +impl_helper_traits!(I16, i16); +impl_helper_traits!(U8, u8); +impl_helper_traits!(I8, i8); +impl_helper_traits!(Bool, bool); + +impl fmt::Display for Value { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.into_string()) + } +} pub struct BenchTimer<'a> { start_time: std::time::Instant, @@ -96,7 +158,7 @@ pub struct Bencher { max_iterations: usize, elapsed: Mutex>, probes: RefCell, - extra_stats: Mutex)>>, + extra_stats: Mutex>, state: Mutex, } @@ -226,11 +288,11 @@ impl Bencher { /// /// Please ensure you are collecting the statistics in a cheap way during the timing phase, /// and call this function only after the timing phase. - pub fn add_stat(&self, name: impl AsRef, value: impl Value) { + pub fn add_stat(&self, name: impl AsRef, value: impl Into) { self.extra_stats .lock() .unwrap() - .push((name.as_ref().to_owned(), Box::new(value))); + .push((name.as_ref().to_owned(), value.into())); } /// Returns the wall-clock time of the last timing phase. @@ -239,8 +301,8 @@ impl Bencher { *self.elapsed.lock().unwrap() } - /// Returns the value of a counter as a floating point number. - pub fn get_raw_counter_value(&self, name: impl AsRef) -> Option { + /// Returns the value of a counter. + pub fn get_raw_counter_value(&self, name: impl AsRef) -> Option { self.probes.borrow().get_value(name.as_ref()) } } diff --git a/harness/src/probe.rs b/harness/src/probe.rs index 33e5061..db9a55e 100644 --- a/harness/src/probe.rs +++ b/harness/src/probe.rs @@ -7,17 +7,20 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize}; use crate::bencher::Value; struct Counters { - counters: Vec<(String, f32)>, + counters: Vec<(String, Value)>, } impl Counters { pub(crate) fn new(walltime: Duration) -> Self { Self { - counters: vec![("time".to_owned(), walltime.as_micros() as f32 / 1000.0)], + counters: vec![( + "time".to_owned(), + (walltime.as_micros() as f32 / 1000.0).into(), + )], } } - fn merge(&mut self, values: HashMap) { + fn merge(&mut self, values: HashMap) { let mut values = values.iter().collect::>(); values.sort_by_key(|x| x.0.as_str()); for (k, v) in values { @@ -25,7 +28,7 @@ impl Counters { } } - fn get_value(&self, name: &str) -> Option { + fn get_value(&self, name: &str) -> Option { self.counters .iter() .find(|(k, _)| k == name) @@ -57,7 +60,7 @@ pub trait Probe { fn end(&mut self, benchmark: &str, iteration: usize, warmup: bool) {} - fn report(&mut self) -> HashMap { + fn report(&mut self) -> HashMap { HashMap::new() } @@ -79,9 +82,12 @@ impl Probe for BaseProbe { self.elapsed = self.start.unwrap().elapsed(); } - fn report(&mut self) -> HashMap { + fn report(&mut self) -> HashMap { let mut values = HashMap::new(); - values.insert("time".to_owned(), self.elapsed.as_micros() as f32 / 1000.0); + values.insert( + "time".to_owned(), + (self.elapsed.as_micros() as f32 / 1000.0).into(), + ); values } } @@ -168,20 +174,17 @@ impl ProbeManager { self.counters = counters; } - pub(crate) fn get_value(&self, name: &str) -> Option { + pub(crate) fn get_value(&self, name: &str) -> Option { self.counters.get_value(name) } - pub(crate) fn get_counter_values( - &self, - extra_stats: Vec<(String, Box)>, - ) -> HashMap> { + pub(crate) fn get_counter_values(&self, extra: Vec<(String, Value)>) -> HashMap { // Collect all stats - let mut stats_map: HashMap> = HashMap::new(); + let mut stats_map: HashMap = HashMap::new(); for (name, value) in &self.counters.counters { - stats_map.insert(name.clone(), Box::new(*value)); + stats_map.insert(name.clone(), *value); } - for (name, value) in extra_stats { + for (name, value) in extra { stats_map.insert(name.clone(), value); } stats_map diff --git a/harness/src/record.rs b/harness/src/record.rs index 7fc2cb2..6297517 100644 --- a/harness/src/record.rs +++ b/harness/src/record.rs @@ -20,28 +20,28 @@ pub(crate) struct Record<'a> { pub format: StatPrintFormat, pub iteration: usize, pub is_timing_iteration: bool, - pub stats: HashMap>, + pub stats: HashMap, } impl<'a> Record<'a> { - fn dump_counters_stderr_table(&self, stats: &[(String, Box)]) { + fn dump_counters_stderr_table(&self, stats: &[(String, Value)]) { for (name, _) in stats { eprint!("{}\t", name); } eprintln!(); for (_, value) in stats { - eprint!("{}\t", value.to_string()); + eprint!("{}\t", value.into_string()); } eprintln!(); } - fn dump_counters_stderr_yaml(&self, stats: &[(String, Box)]) { + fn dump_counters_stderr_yaml(&self, stats: &[(String, Value)]) { for (name, value) in stats { - eprintln!("{}: {}", name, value.to_string()); + eprintln!("{}: {}", name, value.into_string()); } } - fn dump_counters_stderr(&self, stats: &[(String, Box)], format: StatPrintFormat) { + fn dump_counters_stderr(&self, stats: &[(String, Value)], format: StatPrintFormat) { let force_table = std::env::var("HARNESS_LOG_STAT_FORMAT") == Ok("table".to_owned()); if force_table { return self.dump_counters_stderr_table(stats); @@ -52,7 +52,7 @@ impl<'a> Record<'a> { } } - fn dump_counters_csv(&self, stats: &[(String, Box)]) { + fn dump_counters_csv(&self, stats: &[(String, Value)]) { if let Some(csv) = self.csv { if !csv.exists() { let mut headers = "bench,build,invocation,iteration".to_owned(); @@ -71,7 +71,7 @@ impl<'a> Record<'a> { self.iteration ); for (_, value) in stats { - record += &format!(",{}", value.to_string()); + record += &format!(",{}", value.into_string()); } let mut csv = OpenOptions::new().append(true).open(csv).unwrap(); writeln!(csv, "{record}").unwrap(); @@ -81,7 +81,7 @@ impl<'a> Record<'a> { pub fn dump_values(mut self) { let mut stats_map = std::mem::take(&mut self.stats); let time = stats_map.remove("time"); - let mut stats: Vec<(String, Box)> = vec![]; + let mut stats: Vec<(String, Value)> = vec![]; for (name, value) in stats_map { stats.push((name.clone(), value)); } diff --git a/probes/perf/src/lib.rs b/probes/perf/src/lib.rs index f263986..221abd1 100644 --- a/probes/perf/src/lib.rs +++ b/probes/perf/src/lib.rs @@ -1,9 +1,9 @@ #[cfg(target_os = "linux")] use std::collections::HashMap; -use harness::probe::Probe; #[cfg(target_os = "linux")] use harness::probe::ProbeArgs; +use harness::{probe::Probe, Value}; #[harness::probe] #[derive(Default)] @@ -58,11 +58,11 @@ impl Probe for PerfEventProbe { } /// Report data after the timing iteration. - fn report(&mut self) -> HashMap { + fn report(&mut self) -> HashMap { let mut values = HashMap::new(); for (i, e) in self.events.iter().enumerate() { let v = e.read().unwrap().value as f32; - values.insert(self.event_names[i].clone(), v); + values.insert(self.event_names[i].clone(), v.into()); } values }