From 3c8d12bd1539a70156032d02f80d5ba78c55c3b0 Mon Sep 17 00:00:00 2001 From: rayzr522 Date: Tue, 14 Mar 2023 22:16:02 -0400 Subject: [PATCH] fix: histogram upper bound is too low --- src/benchmark.rs | 5 ++++- src/main.rs | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/benchmark.rs b/src/benchmark.rs index bdc88b7..60e0f33 100644 --- a/src/benchmark.rs +++ b/src/benchmark.rs @@ -54,9 +54,12 @@ fn join(l: Vec, sep: &str) -> String { ) } +pub const DEFAULT_TIMEOUT_SECONDS: u64 = 10; +pub const MAX_TIMEOUT_SECONDS: u64 = 120; + #[allow(clippy::too_many_arguments)] pub fn execute(benchmark_path: &str, report_path_option: Option<&str>, relaxed_interpolations: bool, no_check_certificate: bool, quiet: bool, nanosec: bool, timeout: Option<&str>, verbose: bool, tags: &Tags) -> BenchmarkResult { - let config = Arc::new(Config::new(benchmark_path, relaxed_interpolations, no_check_certificate, quiet, nanosec, timeout.map_or(10, |t| t.parse().unwrap_or(10)), verbose)); + let config = Arc::new(Config::new(benchmark_path, relaxed_interpolations, no_check_certificate, quiet, nanosec, timeout.map_or(DEFAULT_TIMEOUT_SECONDS, |t| t.parse().unwrap_or(DEFAULT_TIMEOUT_SECONDS).max(MAX_TIMEOUT_SECONDS)), verbose)); if report_path_option.is_some() { println!("{}: {}. Ignoring {} and {} properties...", "Report mode".yellow(), "on".purple(), "concurrency".yellow(), "iterations".yellow()); diff --git a/src/main.rs b/src/main.rs index a02f88b..93b9fe9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -76,7 +76,7 @@ fn app_args<'a>() -> clap::ArgMatches<'a> { .arg(Arg::with_name("list-tags").long("list-tags").help("List all benchmark tags").takes_value(false).conflicts_with_all(&["tags", "skip-tags"])) .arg(Arg::with_name("list-tasks").long("list-tasks").help("List benchmark tasks (executes --tags/--skip-tags filter)").takes_value(false)) .arg(Arg::with_name("quiet").short("q").long("quiet").help("Disables output").takes_value(false)) - .arg(Arg::with_name("timeout").short("o").long("timeout").help("Set timeout in seconds for all requests").takes_value(true)) + .arg(Arg::with_name("timeout").short("o").long("timeout").help("Set timeout in seconds for all requests (max: 120)").takes_value(true)) .arg(Arg::with_name("nanosec").short("n").long("nanosec").help("Shows statistics in nanoseconds").takes_value(false)) .arg(Arg::with_name("verbose").short("v").long("verbose").help("Toggle verbose output").takes_value(false)) .get_matches() @@ -105,7 +105,8 @@ impl DrillStats { } fn compute_stats(sub_reports: &[Report]) -> DrillStats { - let mut hist = Histogram::::new_with_bounds(1, 60 * 60 * 1000, 2).unwrap(); + // we preserve accuracy down to the microsecond level, so our upper bound is max timeout (seconds) converted to microseconds + let mut hist = Histogram::::new_with_bounds(1, benchmark::MAX_TIMEOUT_SECONDS * 1_000_000, 2).unwrap(); let mut group_by_status = HashMap::new(); for req in sub_reports { @@ -113,6 +114,7 @@ fn compute_stats(sub_reports: &[Report]) -> DrillStats { } for r in sub_reports.iter() { + // convert from milliseconds as a float to microseconds as an unsigned integer hist += (r.duration * 1_000.0) as u64; }