Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: histogram upper bound is too low #174

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,12 @@ fn join<S: ToString>(l: Vec<S>, 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());
Expand Down
6 changes: 4 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -105,14 +105,16 @@ impl DrillStats {
}

fn compute_stats(sub_reports: &[Report]) -> DrillStats {
let mut hist = Histogram::<u64>::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::<u64>::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 {
group_by_status.entry(req.status / 100).or_insert_with(Vec::new).push(req);
}

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;
}

Expand Down