From 34eb150fefd4651431d976ad70d3b7dd4dfa7359 Mon Sep 17 00:00:00 2001 From: knidarkness Date: Mon, 14 Oct 2019 00:03:29 +0300 Subject: [PATCH] updates as requested in the review --- src/hyperfine/benchmark.rs | 82 ++++++++++++++++++++++++-------------- src/hyperfine/types.rs | 2 +- src/main.rs | 12 ++++-- 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/src/hyperfine/benchmark.rs b/src/hyperfine/benchmark.rs index 0cf1666..05af42c 100644 --- a/src/hyperfine/benchmark.rs +++ b/src/hyperfine/benchmark.rs @@ -5,8 +5,6 @@ use std::process::Stdio; use colored::*; use statistical::{mean, median, standard_deviation}; -use indicatif::{ProgressBar}; - use crate::hyperfine::format::{format_duration, format_duration_unit}; use crate::hyperfine::internal::{get_progress_bar, max, min, MIN_EXECUTION_TIME}; use crate::hyperfine::outlier_detection::{modified_zscores, OUTLIER_THRESHOLD}; @@ -96,10 +94,15 @@ pub fn mean_shell_spawning_time( show_output: bool, ) -> io::Result { const COUNT: u64 = 200; - let mut progress_bar: Option = None; - if style != OutputStyleOption::None { - progress_bar = Some(get_progress_bar(COUNT, "Measuring shell spawning time", style)); - } + let progress_bar = if style != OutputStyleOption::Disabled { + Some(get_progress_bar( + COUNT, + "Measuring shell spawning time", + style, + )) + } else { + None + }; let mut times_real: Vec = vec![]; let mut times_user: Vec = vec![]; @@ -137,10 +140,14 @@ pub fn mean_shell_spawning_time( times_user.push(r.time_user); times_system.push(r.time_system); } - } - // if let Some(bar) = progress_bar { bar.inc(1); } + } + if let Some(ref bar) = progress_bar { + bar.inc(1); + } + } + if let Some(ref bar) = progress_bar { + bar.finish_and_clear(); } - if let Some(bar) = progress_bar { bar.finish_and_clear(); } Ok(TimingResult { time_real: mean(×_real), @@ -198,7 +205,7 @@ pub fn run_benchmark( shell_spawning_time: TimingResult, options: &HyperfineOptions, ) -> io::Result { - if options.output_style != OutputStyleOption::None { + if options.output_style != OutputStyleOption::Disabled { println!( "{}{}: {}", "Benchmark #".bold(), @@ -227,14 +234,15 @@ pub fn run_benchmark( // Warmup phase if options.warmup_count > 0 { - let mut progress_bar: Option = None; - if options.output_style != OutputStyleOption::None { - progress_bar = Some(get_progress_bar( + let progress_bar = if options.output_style != OutputStyleOption::Disabled { + Some(get_progress_bar( options.warmup_count, "Performing warmup runs", options.output_style, - )); - } + )) + } else { + None + }; for _ in 0..options.warmup_count { let _ = run_preparation_command(&options.shell, &prepare_cmd, options.show_output)?; @@ -245,21 +253,25 @@ pub fn run_benchmark( options.failure_action, None, )?; - if let Some(pbar) = progress_bar.clone() { pbar.inc(1); } + if let Some(ref pbar) = progress_bar { + pbar.inc(1); + } + } + if let Some(ref bar) = progress_bar { + bar.finish_and_clear(); } - if let Some(bar) = progress_bar { bar.finish_and_clear(); } } // Set up progress bar (and spinner for initial measurement) - let mut progress_bar:Option = None; - - if options.output_style != OutputStyleOption::None { - progress_bar = Some(get_progress_bar( + let progress_bar = if options.output_style != OutputStyleOption::Disabled { + Some(get_progress_bar( options.runs.min, "Initial time measurement", options.output_style, )) - } + } else { + None + }; let prepare_res = run_preparation_command(&options.shell, &prepare_cmd, options.show_output)?; @@ -298,8 +310,10 @@ pub fn run_benchmark( all_succeeded = all_succeeded && success; // Re-configure the progress bar - if let Some(bar) = progress_bar.clone() { bar.set_length(count); } - if let Some(bar) = progress_bar.clone() { bar.inc(1); } + if let Some(ref bar) = progress_bar { + bar.set_length(count); + bar.inc(1); + } // Gather statistics for _ in 0..count_remaining { @@ -310,7 +324,9 @@ pub fn run_benchmark( format!("Current estimate: {}", mean.to_string().green()) }; - if let Some(bar) = progress_bar.clone() { bar.set_message(&msg); } + if let Some(ref bar) = progress_bar { + bar.set_message(&msg); + } let (res, success) = time_shell_command( &options.shell, @@ -326,9 +342,13 @@ pub fn run_benchmark( all_succeeded = all_succeeded && success; - if let Some(bar) = progress_bar.clone() { bar.inc(1); } + if let Some(ref bar) = progress_bar { + bar.inc(1); + } + } + if let Some(ref bar) = progress_bar { + bar.finish_and_clear(); } - if let Some(bar) = progress_bar { bar.finish_and_clear(); } // Compute statistical quantities let t_num = times_real.len(); @@ -351,7 +371,7 @@ pub fn run_benchmark( let (user_str, user_unit) = format_duration_unit(user_mean, None); let system_str = format_duration(system_mean, Some(user_unit)); - if options.output_style != OutputStyleOption::None { + if options.output_style != OutputStyleOption::Disabled { println!( " Time ({} ± {}): {:>8} ± {:>8} [User: {}, System: {}]", "mean".green().bold(), @@ -393,7 +413,7 @@ pub fn run_benchmark( warnings.push(Warnings::OutliersDetected); } - if !warnings.is_empty() && options.output_style != OutputStyleOption::None { + if !warnings.is_empty() { eprintln!(" "); for warning in &warnings { @@ -401,7 +421,9 @@ pub fn run_benchmark( } } - if options.output_style != OutputStyleOption::None { println!(" "); } + if options.output_style != OutputStyleOption::Disabled { + println!(" "); + } // Run cleanup command let cleanup_cmd = diff --git a/src/hyperfine/types.rs b/src/hyperfine/types.rs index fd2738f..bd61665 100644 --- a/src/hyperfine/types.rs +++ b/src/hyperfine/types.rs @@ -115,7 +115,7 @@ pub enum OutputStyleOption { Color, /// Disable all the output - None, + Disabled, } /// Number of runs for a benchmark diff --git a/src/main.rs b/src/main.rs index 601fb2d..18b8da7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -63,9 +63,13 @@ fn main() { match res { Ok(timing_results) => { - let unwrapped = options.unwrap(); - if unwrapped.output_style != OutputStyleOption::None { write_benchmark_comparison(&timing_results); } - let ans = export_manager.write_results(timing_results, unwrapped.time_unit); + let options = options.unwrap(); + + if options.output_style != OutputStyleOption::Disabled { + write_benchmark_comparison(&timing_results); + } + + let ans = export_manager.write_results(timing_results, options.time_unit); if let Err(e) = ans { error(&format!( "The following error occurred while exporting: {}", @@ -136,7 +140,7 @@ fn build_hyperfine_options(matches: &ArgMatches<'_>) -> Result OutputStyleOption::Basic, Some("nocolor") => OutputStyleOption::NoColor, Some("color") => OutputStyleOption::Color, - Some("none") => OutputStyleOption::None, + Some("none") => OutputStyleOption::Disabled, _ => { if !options.show_output && atty::is(Stream::Stdout) { OutputStyleOption::Full