From 7c7277777a8cf629a906c67210b03fc1ce96019e Mon Sep 17 00:00:00 2001 From: Steve Pentland Date: Sun, 18 Mar 2018 15:13:24 -0400 Subject: [PATCH] Change the structure of everything to remove Manager trait. Manager is now a simple struct, that no longer stores the results, but instead takes in a Vec and has all exporters write from this collection. Benchmark and run both return their results, no more mut manager. Change f64 in ExportEntry to hyperfine::internal::Second. Remove multiple export files for a single type. --- src/hyperfine/benchmark.rs | 18 +++++++-------- src/hyperfine/export/mod.rs | 44 ++++++++++++------------------------- src/main.rs | 39 ++++++++++++++------------------ 3 files changed, 40 insertions(+), 61 deletions(-) diff --git a/src/hyperfine/benchmark.rs b/src/hyperfine/benchmark.rs index 7f23c75..6692d3f 100644 --- a/src/hyperfine/benchmark.rs +++ b/src/hyperfine/benchmark.rs @@ -11,7 +11,7 @@ use hyperfine::outlier_detection::{modified_zscores, OUTLIER_THRESHOLD}; use hyperfine::timer::{TimerStart, TimerStop}; use hyperfine::timer::wallclocktimer::WallClockTimer; use hyperfine::shell::execute_and_time; -use hyperfine::export::{ExportEntry, ExportManager}; +use hyperfine::export::ExportEntry; /// Results from timing a single shell command #[derive(Debug, Default, Copy, Clone)] @@ -137,8 +137,7 @@ pub fn run_benchmark( cmd: &str, shell_spawning_time: TimingResult, options: &HyperfineOptions, - exporter: &mut Option>, -) -> io::Result<()> { +) -> io::Result { println!( "{}{}: {}", "Benchmark #".bold(), @@ -267,11 +266,6 @@ pub fn run_benchmark( // Warnings let mut warnings = vec![]; - if let &mut Some(ref mut mgr) = exporter { - let entry = ExportEntry::new(cmd.to_string(), t_mean, t_stddev, user_mean, system_mean); - mgr.add_result(entry); - } - // Check execution time if times_real.iter().any(|&t| t < MIN_EXECUTION_TIME) { warnings.push(Warnings::FastExecutionTime); @@ -300,5 +294,11 @@ pub fn run_benchmark( println!(" "); - Ok(()) + Ok(ExportEntry::new( + cmd.to_string(), + t_mean, + t_stddev, + user_mean, + system_mean, + )) } diff --git a/src/hyperfine/export/mod.rs b/src/hyperfine/export/mod.rs index 716054c..95fa651 100644 --- a/src/hyperfine/export/mod.rs +++ b/src/hyperfine/export/mod.rs @@ -4,6 +4,8 @@ use self::csv::CsvExporter; use std::io::Result; +use hyperfine::internal::Second; + /// The ExportEntry is the main set of values that will /// be exported to files when requested. #[derive(Debug, Default, Clone, Serialize)] @@ -11,13 +13,13 @@ pub struct ExportEntry { /// The command that was run command: String, /// The mean run time - mean: f64, + mean: Second, /// The standard deviation of all run times - stddev: f64, + stddev: Second, /// Time spend in user space - user: f64, + user: Second, /// Time spent in system space - system: f64, + system: Second, } impl ExportEntry { @@ -47,47 +49,29 @@ trait ResultExporter { fn write(&self, values: &Vec) -> Result<()>; } -/// The ExportManager handles coordination of multiple ResultExporters -pub trait ExportManager { - /// Add a new exporter to this manager for the given type - fn add_exporter(&mut self, for_type: &ResultExportType); - - /// Add a new result to all exporters contained in the manager - fn add_result(&mut self, result: ExportEntry); - - /// Trigger writes from all exporters - fn write_results(&self) -> Result<()>; -} - /// Create a new ExportManager -pub fn create_export_manager() -> Box { - Box::new(Exporter { +pub fn create_export_manager() -> ExportManager { + ExportManager { exporters: Vec::new(), - results: Vec::new(), - }) + } } /// The Exporter is the internal implementation of the ExportManager -struct Exporter { +pub struct ExportManager { exporters: Vec>, - results: Vec, } -impl ExportManager for Exporter { - fn add_exporter(&mut self, for_type: &ResultExportType) { +impl ExportManager { + pub fn add_exporter(&mut self, for_type: &ResultExportType) { match for_type { &ResultExportType::Csv(ref file_name) => self.exporters .push(Box::from(CsvExporter::new(file_name.clone()))), }; } - fn add_result(&mut self, result: ExportEntry) { - self.results.push(result); - } - - fn write_results(&self) -> Result<()> { + pub fn write_results(&self, to_write: Vec) -> Result<()> { for exp in &self.exporters { - exp.write(&self.results)?; + exp.write(&to_write)?; } Ok(()) } diff --git a/src/main.rs b/src/main.rs index 96eeada..dd5ef53 100644 --- a/src/main.rs +++ b/src/main.rs @@ -38,7 +38,7 @@ mod hyperfine; use hyperfine::internal::{CmdFailureAction, HyperfineOptions, OutputStyleOption}; use hyperfine::benchmark::{mean_shell_spawning_time, run_benchmark}; -use hyperfine::export::{create_export_manager, ExportManager, ResultExportType}; +use hyperfine::export::{create_export_manager, ExportEntry, ResultExportType}; /// Print error message to stderr and terminate pub fn error(message: &str) -> ! { @@ -47,19 +47,17 @@ pub fn error(message: &str) -> ! { } /// Runs the benchmark for the given commands -fn run( - commands: &Vec<&str>, - options: &HyperfineOptions, - exporter: &mut Option>, -) -> io::Result<()> { +fn run(commands: &Vec<&str>, options: &HyperfineOptions) -> io::Result> { let shell_spawning_time = mean_shell_spawning_time(&options.output_style)?; + let mut timing_results = vec![]; + // Run the benchmarks for (num, cmd) in commands.iter().enumerate() { - run_benchmark(num, cmd, shell_spawning_time, options, exporter)?; + timing_results.push(run_benchmark(num, cmd, shell_spawning_time, options)?); } - Ok(()) + Ok(timing_results) } fn main() { @@ -143,8 +141,6 @@ fn main() { Arg::with_name("export-csv") .long("export-csv") .takes_value(true) - .multiple(true) - .number_of_values(1) .value_name("FILE") .help("Export the timing results to the given file in csv format."), ) @@ -180,12 +176,10 @@ fn main() { // Initial impl since we're only doing csv files, expand once we have multiple // export types. Simplest probably to do the same for each, but check for // None manager on later additions (JSON, Markdown, etc.) - let mut export_manager = match matches.values_of("export-csv") { - Some(filenames) => { + let export_manager = match matches.value_of("export-csv") { + Some(filename) => { let mut export_manager = create_export_manager(); - for filename in filenames { - export_manager.add_exporter(&ResultExportType::Csv(filename.to_string())); - } + export_manager.add_exporter(&ResultExportType::Csv(filename.to_string())); Some(export_manager) } None => None, @@ -205,13 +199,14 @@ fn main() { } let commands = matches.values_of("command").unwrap().collect(); - let res = run(&commands, &options, &mut export_manager); + let res = run(&commands, &options); - if let Err(e) = res { - error(e.description()); - } - - if let Some(exporter) = export_manager { - exporter.write_results().unwrap(); + match res { + Ok(timing_results) => { + if let Some(mut exporter) = export_manager { + exporter.write_results(timing_results).unwrap(); + } + } + Err(e) => error(e.description()), } }