Skip to content

fix: histogram upper bound is too low #174

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.

Sign up for GitHub

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

Open
foxfirecodes wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: histogram upper bound is too low #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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
View file
Open in desktop
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