diff --git a/Cargo.lock b/Cargo.lock index e74ac1128..56c91721a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -345,6 +345,15 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "cpuid" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0c4b055219ace68cabfbb8071c4aad068565d11b67e092e151b17b9fd3bdba9" +dependencies = [ + "libc", +] + [[package]] name = "criterion" version = "0.6.0" @@ -355,10 +364,12 @@ dependencies = [ "cast", "ciborium", "clap", + "cpuid", "criterion-plot", "csv", "futures", "itertools", + "mach-sys", "num-traits", "oorandom", "plotters", @@ -716,6 +727,15 @@ dependencies = [ "value-bag", ] +[[package]] +name = "mach-sys" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48460c2e82a3a0de197152fdf8d2c2d5e43adc501501553e439bf2156e6f87c7" +dependencies = [ + "fastrand", +] + [[package]] name = "memchr" version = "2.7.4" diff --git a/Cargo.toml b/Cargo.toml index 18502cdb4..93946cf8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,9 @@ categories = ["development-tools::profiling"] license = "Apache-2.0 OR MIT" exclude = ["book/*"] +[lints.clippy] +uninlined_format_args = "allow" + [dependencies] anes = "0.1.4" criterion-plot = { path = "plot", version = "0.5.0" } @@ -35,6 +38,13 @@ cast = "0.3" num-traits = { version = "0.2", default-features = false, features = ["std"] } oorandom = "11.1" regex = { version = "1.5.1", default-features = false, features = ["std"] } +rayon = "1.3" + +[target.'cfg(target_vendor="apple")'.dependencies] +mach-sys = "0.5" + +[target.'cfg(target_arch="x86_64")'.dependencies] +cpuid = "0.1.1" # Optional dependencies rayon = { version = "1.3", optional = true } diff --git a/plot/src/axis.rs b/plot/src/axis.rs index 80e245181..2d9ced35e 100644 --- a/plot/src/axis.rs +++ b/plot/src/axis.rs @@ -159,27 +159,27 @@ impl Script for (Axis, &Properties) { let axis_ = axis.display(); let mut script = if properties.hidden { - return format!("unset {}tics\n", axis_); + return format!("unset {axis_}tics\n"); } else { - format!("set {}tics nomirror ", axis_) + format!("set {axis_}tics nomirror ") }; if let Some(ref tics) = properties.tics { - script.push_str(&format!("({})", tics)) + script.push_str(&format!("({tics})")) } script.push('\n'); if let Some(ref label) = properties.label { - script.push_str(&format!("set {}label '{}'\n", axis_, label)) + script.push_str(&format!("set {axis_}label '{label}'\n")) } if let Some((low, high)) = properties.range { - script.push_str(&format!("set {}range [{}:{}]\n", axis_, low, high)) + script.push_str(&format!("set {axis_}range [{low}:{high}]\n")) } if properties.logarithmic { - script.push_str(&format!("set logscale {}\n", axis_)); + script.push_str(&format!("set logscale {axis_}\n")); } for (grid, properties) in properties.grids.iter() { diff --git a/plot/src/candlestick.rs b/plot/src/candlestick.rs index 2d8614763..8663fa311 100644 --- a/plot/src/candlestick.rs +++ b/plot/src/candlestick.rs @@ -32,7 +32,7 @@ impl Script for Properties { script.push_str(&format!("lt {} ", self.line_type.display())); if let Some(lw) = self.linewidth { - script.push_str(&format!("lw {} ", lw)) + script.push_str(&format!("lw {lw} ")) } if let Some(color) = self.color { diff --git a/plot/src/curve.rs b/plot/src/curve.rs index 39fef4b9c..a39b4615c 100644 --- a/plot/src/curve.rs +++ b/plot/src/curve.rs @@ -48,7 +48,7 @@ impl Script for Properties { script.push_str(&format!("lt {} ", self.line_type.display())); if let Some(lw) = self.linewidth { - script.push_str(&format!("lw {} ", lw)) + script.push_str(&format!("lw {lw} ")) } if let Some(color) = self.color { @@ -60,7 +60,7 @@ impl Script for Properties { } if let Some(ps) = self.point_size { - script.push_str(&format!("ps {} ", ps)) + script.push_str(&format!("ps {ps} ")) } if let Some(ref label) = self.label { diff --git a/plot/src/display.rs b/plot/src/display.rs index 8f6f1e130..c19da189c 100644 --- a/plot/src/display.rs +++ b/plot/src/display.rs @@ -38,7 +38,7 @@ impl Display> for Color { Color::Green => Cow::from("green"), Color::Magenta => Cow::from("magenta"), Color::Red => Cow::from("red"), - Color::Rgb(r, g, b) => Cow::from(format!("#{:02x}{:02x}{:02x}", r, g, b)), + Color::Rgb(r, g, b) => Cow::from(format!("#{r:02x}{g:02x}{b:02x}")), Color::White => Cow::from("white"), Color::Yellow => Cow::from("yellow"), } diff --git a/plot/src/errorbar.rs b/plot/src/errorbar.rs index 9a33f50ee..bf1042b68 100644 --- a/plot/src/errorbar.rs +++ b/plot/src/errorbar.rs @@ -41,7 +41,7 @@ impl Script for Properties { script.push_str(&format!("lt {} ", self.line_type.display())); if let Some(lw) = self.linewidth { - script.push_str(&format!("lw {} ", lw)) + script.push_str(&format!("lw {lw} ")) } if let Some(color) = self.color { @@ -53,7 +53,7 @@ impl Script for Properties { } if let Some(ps) = self.point_size { - script.push_str(&format!("ps {} ", ps)) + script.push_str(&format!("ps {ps} ")) } if let Some(ref label) = self.label { diff --git a/plot/src/filledcurve.rs b/plot/src/filledcurve.rs index dea74e1df..dbf5edc05 100644 --- a/plot/src/filledcurve.rs +++ b/plot/src/filledcurve.rs @@ -37,7 +37,7 @@ impl Script for Properties { script.push_str("fillstyle "); if let Some(opacity) = self.opacity { - script.push_str(&format!("solid {} ", opacity)) + script.push_str(&format!("solid {opacity} ")) } // TODO border shoulde be configurable diff --git a/plot/src/grid.rs b/plot/src/grid.rs index 5a1344dbb..2eedd296e 100644 --- a/plot/src/grid.rs +++ b/plot/src/grid.rs @@ -40,7 +40,7 @@ impl Script for (Axis, Grid, &Properties) { if properties.hidden { String::new() } else { - format!("set grid {}{}tics\n", grid, axis) + format!("set grid {grid}{axis}tics\n") } } } diff --git a/plot/src/key.rs b/plot/src/key.rs index b5d80683d..affa5428f 100644 --- a/plot/src/key.rs +++ b/plot/src/key.rs @@ -81,7 +81,7 @@ impl Script for Properties { } if let Some(ref title) = self.title { - script.push_str(&format!("title '{}' ", title)) + script.push_str(&format!("title '{title}' ")) } if self.boxed { diff --git a/plot/src/lib.rs b/plot/src/lib.rs index f9df8d637..092c154fc 100644 --- a/plot/src/lib.rs +++ b/plot/src/lib.rs @@ -441,11 +441,11 @@ impl Figure { )); if let Some(width) = self.box_width { - s.push_str(&format!("set boxwidth {}\n", width)) + s.push_str(&format!("set boxwidth {width}\n")) } if let Some(ref title) = self.title { - s.push_str(&format!("set title '{}'\n", title)) + s.push_str(&format!("set title '{title}'\n")) } for axis in self.axes.iter() { @@ -461,20 +461,20 @@ impl Figure { } if let Some(alpha) = self.alpha { - s.push_str(&format!("set style fill transparent solid {}\n", alpha)) + s.push_str(&format!("set style fill transparent solid {alpha}\n")) } s.push_str(&format!("set terminal {} dashed", self.terminal.display())); if let Some((width, height)) = self.size { - s.push_str(&format!(" size {}, {}", width, height)) + s.push_str(&format!(" size {width}, {height}")) } if let Some(ref name) = self.font { if let Some(size) = self.font_size { - s.push_str(&format!(" font '{},{}'", name, size)) + s.push_str(&format!(" font '{name},{size}'")) } else { - s.push_str(&format!(" font '{}'", name)) + s.push_str(&format!(" font '{name}'")) } } @@ -936,16 +936,14 @@ pub enum VersionError { impl fmt::Display for VersionError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - VersionError::Exec(err) => write!(f, "`gnuplot --version` failed: {}", err), + VersionError::Exec(err) => write!(f, "`gnuplot --version` failed: {err}"), VersionError::Error(msg) => { - write!(f, "`gnuplot --version` failed with error message:\n{}", msg) + write!(f, "`gnuplot --version` failed with error message:\n{msg}") } VersionError::OutputError => write!(f, "`gnuplot --version` returned invalid utf-8"), VersionError::ParseError(msg) => write!( f, - "`gnuplot --version` returned an unparsable version string: {}", - msg - ), + "`gnuplot --version` returned an unparsable version string: {msg}"), } } } diff --git a/src/analysis/mod.rs b/src/analysis/mod.rs index 03b82fb43..9a3c56b32 100644 --- a/src/analysis/mod.rs +++ b/src/analysis/mod.rs @@ -71,7 +71,7 @@ pub(crate) fn common( match loaded { Err(err) => panic!( "Baseline '{base}' must exist before it can be loaded; try --save-baseline {base}. Error: {err}", - base = baseline, err = err + base=criterion.baseline_directory, ), Ok(samples) => { sampling_mode = samples.sampling_mode; @@ -121,6 +121,14 @@ pub(crate) fn common( return; } + if times.contains(&f64::NAN) { + error!( + "At least one measurement of benchmark {} had a NAN.", + id.as_title() + ); + return; + } + let avg_times = iters .iter() .zip(times.iter()) diff --git a/src/bencher.rs b/src/bencher.rs index 369c1ce83..0cc4d552b 100644 --- a/src/bencher.rs +++ b/src/bencher.rs @@ -90,8 +90,13 @@ impl<'a, M: Measurement> Bencher<'a, M> { for _ in 0..self.iters { black_box(routine()); } - self.value = self.measurement.end(start); + self.value = self.measurement.end(&start); + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(self.measurement.lt(&self.measurement.zero(), &self.value)); self.elapsed_time = time_start.elapsed(); + if self.elapsed_time < Duration::from_nanos(1) { + self.elapsed_time = Duration::from_nanos(1); + } } /// Times a `routine` by executing it many times and relying on `routine` to measure its own execution time. @@ -136,7 +141,12 @@ impl<'a, M: Measurement> Bencher<'a, M> { self.iterated = true; let time_start = Instant::now(); self.value = routine(self.iters); + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(self.measurement.lt(&self.measurement.zero(), &self.value)); self.elapsed_time = time_start.elapsed(); + if self.elapsed_time < Duration::from_nanos(1) { + self.elapsed_time = Duration::from_nanos(1); + } } #[doc(hidden)] @@ -248,7 +258,7 @@ impl<'a, M: Measurement> Bencher<'a, M> { let start = self.measurement.start(); let output = routine(input); - let end = self.measurement.end(start); + let end = self.measurement.end(&start); self.value = self.measurement.add(&self.value, &end); drop(black_box(output)); @@ -264,7 +274,7 @@ impl<'a, M: Measurement> Bencher<'a, M> { let start = self.measurement.start(); outputs.extend(inputs.into_iter().map(&mut routine)); - let end = self.measurement.end(start); + let end = self.measurement.end(&start); self.value = self.measurement.add(&self.value, &end); black_box(outputs); @@ -272,8 +282,13 @@ impl<'a, M: Measurement> Bencher<'a, M> { iteration_counter += batch_size; } } + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(self.measurement.lt(&self.measurement.zero(), &self.value)); self.elapsed_time = time_start.elapsed(); + if self.elapsed_time < Duration::from_nanos(1) { + self.elapsed_time = Duration::from_nanos(1); + } } /// Times a `routine` that requires some input by generating a batch of input, then timing the @@ -336,7 +351,7 @@ impl<'a, M: Measurement> Bencher<'a, M> { let start = self.measurement.start(); let output = routine(&mut input); - let end = self.measurement.end(start); + let end = self.measurement.end(&start); self.value = self.measurement.add(&self.value, &end); drop(black_box(output)); @@ -353,7 +368,7 @@ impl<'a, M: Measurement> Bencher<'a, M> { let start = self.measurement.start(); outputs.extend(inputs.iter_mut().map(&mut routine)); - let end = self.measurement.end(start); + let end = self.measurement.end(&start); self.value = self.measurement.add(&self.value, &end); black_box(outputs); @@ -361,7 +376,12 @@ impl<'a, M: Measurement> Bencher<'a, M> { iteration_counter += batch_size; } } + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(self.measurement.lt(&self.measurement.zero(), &self.value)); self.elapsed_time = time_start.elapsed(); + if self.elapsed_time < Duration::from_nanos(1) { + self.elapsed_time = Duration::from_nanos(1); + } } // Benchmarks must actually call one of the iter methods. This causes benchmarks to fail loudly @@ -439,7 +459,12 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { black_box(routine().await); } b.value = b.measurement.end(start); + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(b.measurement.lt(&b.measurement.zero(), &b.value)); b.elapsed_time = time_start.elapsed(); + if b.elapsed_time < Duration::from_nanos(1) { + b.elapsed_time = Duration::from_nanos(1); + } }); } @@ -491,7 +516,12 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { b.iterated = true; let time_start = Instant::now(); b.value = routine(b.iters).await; + debug_assert!(self.measurement.to_f64(&self.value).is_finite()); + debug_assert!(b.measurement.lt(&b.measurement.zero(), &b.value)); b.elapsed_time = time_start.elapsed(); + if self.elapsed_time < Duration::from_nanos(1) { + self.elapsed_time = Duration::from_nanos(1); + } }) } @@ -648,8 +678,13 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { iteration_counter += batch_size; } } + debug_assert!(b.measurement.to_f64(&b.value).is_finite()); + debug_assert!(b.measurement.lt(&b.measurement.zero(), &b.value)); b.elapsed_time = time_start.elapsed(); + if b.elapsed_time < Duration::from_nanos(1) { + b.elapsed_time = Duration::from_nanos(1); + } }) } @@ -745,7 +780,12 @@ impl<'a, 'b, A: AsyncExecutor, M: Measurement> AsyncBencher<'a, 'b, A, M> { iteration_counter += batch_size; } } + debug_assert!(b.measurement.to_f64(&b.value).is_finite()); + debug_assert!(b.measurement.lt(&b.measurement.zero(), &b.value)); b.elapsed_time = time_start.elapsed(); + if b.elapsed_time < Duration::new(0, 1) { + b.elapsed_time = Duration::new(0, 1); + } }); } } diff --git a/src/benchmark_group.rs b/src/benchmark_group.rs index 7f9009b3c..fb3e276e6 100644 --- a/src/benchmark_group.rs +++ b/src/benchmark_group.rs @@ -40,7 +40,7 @@ use std::time::Duration; /// for x in 0..3 { /// for y in 0..3 { /// let point = (x, y); -/// let parameter_string = format!("{} * {}", x, y); +/// let parameter_string = format!("{x} * {y}"); /// group.bench_with_input(BenchmarkId::new("Multiply", parameter_string), &point, /// |b, (p_x, p_y)| b.iter(|| p_x * p_y)); /// } @@ -329,7 +329,7 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> { } Mode::List(_) => { if do_run { - println!("{}: benchmark", id); + println!("{id}: benchmark"); } } Mode::Test => { @@ -440,7 +440,7 @@ impl BenchmarkId { ) -> BenchmarkId { BenchmarkId { function_name: Some(function_name.into()), - parameter: Some(format!("{}", parameter)), + parameter: Some(format!("{parameter}")), } } @@ -449,7 +449,7 @@ impl BenchmarkId { pub fn from_parameter(parameter: P) -> BenchmarkId { BenchmarkId { function_name: None, - parameter: Some(format!("{}", parameter)), + parameter: Some(format!("{parameter}")), } } @@ -463,7 +463,7 @@ impl BenchmarkId { pub(crate) fn no_function_with_input(parameter: P) -> BenchmarkId { BenchmarkId { function_name: None, - parameter: Some(format!("{}", parameter)), + parameter: Some(format!("{parameter}")), } } } diff --git a/src/connection.rs b/src/connection.rs index b18a77335..ad4a9b54d 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -33,18 +33,15 @@ impl std::fmt::Display for MessageError { match self { MessageError::Deserialization(error) => write!( f, - "Failed to deserialize message to Criterion.rs benchmark:\n{}", - error + "Failed to deserialize message to Criterion.rs benchmark:\n{error}" ), MessageError::Serialization(error) => write!( f, - "Failed to serialize message to Criterion.rs benchmark:\n{}", - error + "Failed to serialize message to Criterion.rs benchmark:\n{error}" ), MessageError::Io(error) => write!( f, - "Failed to read or write message to Criterion.rs benchmark:\n{}", - error + "Failed to read or write message to Criterion.rs benchmark:\n{error}" ), } } diff --git a/src/error.rs b/src/error.rs index 6c559c686..e54a71dd4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,18 +30,17 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Error::AccessError { path, inner } => { - write!(f, "Failed to access file {:?}: {}", path, inner) + write!(f, "Failed to access file {path:?}: {inner}") } Error::CopyError { from, to, inner } => { - write!(f, "Failed to copy file {:?} to {:?}: {}", from, to, inner) + write!(f, "Failed to copy file {from:?} to {to:?}: {inner}") } Error::SerdeError { path, inner } => write!( f, - "Failed to read or write file {:?} due to serialization error: {}", - path, inner + "Failed to read or write file {path:?} due to serialization error: {inner}" ), #[cfg(feature = "csv_output")] - Error::CsvError(inner) => write!(f, "CSV error: {}", inner), + Error::CsvError(inner) => write!(f, "CSV error: {inner}"), } } } diff --git a/src/format.rs b/src/format.rs index 740476430..1340e438a 100644 --- a/src/format.rs +++ b/src/format.rs @@ -22,15 +22,15 @@ pub fn time(ns: f64) -> String { pub fn short(n: f64) -> String { if n < 10.0 { - format!("{:.4}", n) + format!("{n:.4}") } else if n < 100.0 { - format!("{:.3}", n) + format!("{n:.3}") } else if n < 1000.0 { - format!("{:.2}", n) + format!("{n:.2}") } else if n < 10000.0 { - format!("{:.1}", n) + format!("{n:.1}") } else { - format!("{:.0}", n) + format!("{n:.0}") } } @@ -39,21 +39,21 @@ fn signed_short(n: f64) -> String { let sign = if n >= 0.0 { '+' } else { '\u{2212}' }; if n_abs < 10.0 { - format!("{}{:.4}", sign, n_abs) + format!("{sign}{n_abs:.4}") } else if n_abs < 100.0 { - format!("{}{:.3}", sign, n_abs) + format!("{sign}{n_abs:.3}") } else if n_abs < 1000.0 { - format!("{}{:.2}", sign, n_abs) + format!("{sign}{n_abs:.2}") } else if n_abs < 10000.0 { - format!("{}{:.1}", sign, n_abs) + format!("{sign}{n_abs:.1}") } else { - format!("{}{:.0}", sign, n_abs) + format!("{sign}{n_abs:.0}") } } pub fn iter_count(iterations: u64) -> String { if iterations < 10_000 { - format!("{} iterations", iterations) + format!("{iterations} iterations") } else if iterations < 1_000_000 { format!("{:.0}k iterations", (iterations as f64) / 1000.0) } else if iterations < 10_000_000 { diff --git a/src/html/mod.rs b/src/html/mod.rs index 687cd2329..176c001f6 100644 --- a/src/html/mod.rs +++ b/src/html/mod.rs @@ -21,10 +21,10 @@ fn debug_context(path: &Path, context: &S) { if crate::debug_enabled() { let mut context_path = PathBuf::from(path); context_path.set_extension("json"); - println!("Writing report context to {:?}", context_path); + println!("Writing report context to {context_path:?}"); let result = fs::save(context, &context_path); if let Err(e) = result { - error!("Failed to write report context debug output: {}", e); + error!("Failed to write report context debug output: {e}"); } } } @@ -69,7 +69,7 @@ impl IndividualBenchmark { IndividualBenchmark { name: id.as_title().to_owned(), - path: format!("{}/{}", path_prefix, id.as_directory_name()), + path: format!("{path_prefix}/{}", id.as_directory_name()), regression_exists: regression_path.is_file(), } } diff --git a/src/kde.rs b/src/kde.rs index 8812142eb..5a67b0609 100644 --- a/src/kde.rs +++ b/src/kde.rs @@ -17,8 +17,12 @@ pub fn sweep_and_estimate( range: Option<(f64, f64)>, point_to_estimate: f64, ) -> (Box<[f64]>, Box<[f64]>, f64) { + debug_assert!(range.is_none() || (range.unwrap().0.is_finite() && range.unwrap().0.is_finite())); + let x_min = sample.min(); + debug_assert!(x_min.is_finite()); let x_max = sample.max(); + debug_assert!(x_max.is_finite(), "{x_max}"); let kde = Kde::new(sample, Gaussian, Bandwidth::Silverman); let h = kde.bandwidth(); @@ -27,15 +31,19 @@ pub fn sweep_and_estimate( Some((start, end)) => (start, end), None => (x_min - 3. * h, x_max + 3. * h), }; + debug_assert!(start.is_finite()); + debug_assert!(end.is_finite()); let mut xs: Vec = Vec::with_capacity(npoints); let step_size = (end - start) / (npoints - 1) as f64; + debug_assert!(step_size.is_finite()); for n in 0..npoints { xs.push(start + (step_size * n as f64)); } let ys = kde.map(&xs); let point_estimate = kde.estimate(point_to_estimate); + debug_assert!(point_to_estimate.is_finite()); (xs.into_boxed_slice(), ys, point_estimate) } diff --git a/src/measurement.rs b/src/measurement.rs index b539b5947..944f8d946 100644 --- a/src/measurement.rs +++ b/src/measurement.rs @@ -70,6 +70,7 @@ pub trait ValueFormatter { /// of that set of iterations) and `end` is called at the end of the measurement with the value /// returned by `start`. /// +/// All Measurements must be greater than 0. pub trait Measurement { /// This type represents an intermediate value for the measurements. It will be produced by the /// start function and passed to the end function. An example might be the wall-clock time as @@ -84,7 +85,7 @@ pub trait Measurement { fn start(&self) -> Self::Intermediate; /// Criterion.rs will call this after iterating the benchmark to get the measured value. - fn end(&self, i: Self::Intermediate) -> Self::Value; + fn end(&self, i: &Self::Intermediate) -> Self::Value; /// Combine two values. Criterion.rs sometimes needs to perform measurements in multiple batches /// of iterations, so the value from one batch must be added to the sum of the previous batches. @@ -93,6 +94,15 @@ pub trait Measurement { /// Return a "zero" value for the Value type which can be added to another value. fn zero(&self) -> Self::Value; + /// Return the least value greater than zero. + fn one(&self) -> Self::Value; + + /// Return true iff val < other. + fn lt(&self, val: &Self::Value, other: &Self::Value) -> bool; + + /// Print some stuff about these + fn debugprint(&self, val: &Self::Intermediate, other: &Self::Value); + /// Converts the measured value to f64 so that it can be used in statistical analysis. fn to_f64(&self, value: &Self::Value) -> f64; @@ -240,8 +250,13 @@ impl Measurement for WallTime { fn start(&self) -> Self::Intermediate { Instant::now() } - fn end(&self, i: Self::Intermediate) -> Self::Value { - i.elapsed() + fn end(&self, i: &Self::Intermediate) -> Self::Value { + let e = i.elapsed(); + if e > Duration::from_nanos(0) { + e + } else { + Duration::from_nanos(1) + } } fn add(&self, v1: &Self::Value, v2: &Self::Value) -> Self::Value { *v1 + *v2 @@ -249,6 +264,15 @@ impl Measurement for WallTime { fn zero(&self) -> Self::Value { Duration::from_secs(0) } + fn one(&self) -> Self::Value { + Duration::from_nanos(1) + } + fn lt(&self, val: &Self::Value, other: &Self::Value) -> bool { + val < other + } + fn debugprint(&self, val: &Self::Intermediate, other: &Self::Value) { + eprintln!("val: {val:?}, other: {other:?}"); + } fn to_f64(&self, val: &Self::Value) -> f64 { val.as_nanos() as f64 } @@ -256,3 +280,219 @@ impl Measurement for WallTime { &DurationFormatter } } + +#[cfg(target_vendor = "apple")] +/// A slightly better timer for Apple platforms. +pub mod plat_apple { + use mach_sys::mach_time::{mach_absolute_time, mach_timebase_info}; + use mach_sys::kern_return::KERN_SUCCESS; + use crate::measurement::ValueFormatter; + use std::mem::MaybeUninit; + use crate::{Measurement, Throughput}; + + #[derive(Default)] + /// Use the `mach_absolute_time()` clock, which is slightly better + /// in my [tests](https://github.com/zooko/measure-clocks) than + /// `Instant::now()`. + pub struct MachAbsoluteTimeMeasurement { } + + /// Formatter + pub struct MachAbsoluteTimeValueFormatter { } + + impl ValueFormatter for MachAbsoluteTimeValueFormatter { + fn scale_values(&self, typical_value: f64, values: &mut [f64]) -> &'static str { + let mut mtt1: MaybeUninit = MaybeUninit::uninit(); + let retval = unsafe { mach_timebase_info(mtt1.as_mut_ptr()) }; + assert_eq!(retval, KERN_SUCCESS); + let mtt2 = unsafe { mtt1.assume_init() }; + + let typical_as_nanos = typical_value * mtt2.numer as f64 / mtt2.denom as f64; + let (factor, unit) = if typical_as_nanos < 10f64.powi(0) { + (10f64.powi(3), "ps") + } else if typical_as_nanos < 10f64.powi(3) { + (10f64.powi(0), "ns") + } else if typical_as_nanos < 10f64.powi(6) { + (10f64.powi(-3), "µs") + } else if typical_as_nanos < 10f64.powi(9) { + (10f64.powi(-6), "ms") + } else { + (10f64.powi(-9), "s") + }; + + for val in values { + *val *= factor * mtt2.numer as f64; + *val /= mtt2.denom as f64; + } + + unit + } + + fn scale_throughputs( + &self, + _typical: f64, + _throughput: &Throughput, + _values: &mut [f64], + ) -> &'static str { + todo!(); + } + + fn scale_for_machines(&self, _values: &mut [f64]) -> &'static str { + // no scaling is needed + "ns" + } + } + + impl Measurement for MachAbsoluteTimeMeasurement { + type Intermediate = u64; + type Value = u64; + + fn start(&self) -> Self::Intermediate { + unsafe { mach_absolute_time() } + } + fn end(&self, i: &Self::Intermediate) -> Self::Value { + let t = unsafe { mach_absolute_time() }; + if t > *i { + t - *i + } else { + 1 + } + } + fn add(&self, v1: &Self::Value, v2: &Self::Value) -> Self::Value { + *v1 + *v2 + } + fn zero(&self) -> Self::Value { + 0 + } + fn one(&self) -> Self::Value { + 1 + } + fn lt(&self, val: &Self::Value, other: &Self::Value) -> bool { + val < other + } + fn debugprint(&self, val: &Self::Intermediate, other: &Self::Value) { + eprintln!("val: {val:?}, other: {other:?}"); + } + fn to_f64(&self, val: &Self::Value) -> f64 { + *val as f64 + } + fn formatter(&self) -> &dyn ValueFormatter { + &MachAbsoluteTimeValueFormatter { } + } + } +} + +#[cfg(target_arch = "x86_64")] +/// A slightly better timer for `x86_64` platforms. +pub mod plat_x86_64 { + use cpuid; + use crate::{Throughput, Measurement}; + use crate::measurement::ValueFormatter; + use core::arch::x86_64; + + #[derive(Default)] + /// Use RDTSCP instruction as a clock. + pub struct RDTSCPMeasurement { } + + /// Formatter. + pub struct RDTSCPValueFormatter { } + + impl ValueFormatter for RDTSCPValueFormatter { + fn scale_values(&self, typical_value: f64, values: &mut [f64]) -> &'static str { + // xxx maybe store the auto-detected frequency during the warm-up period in this struct instead of detecting it at this point in the run? + + let ofreq = cpuid::clock_frequency(); + debug_assert!(ofreq.is_some()); + let freq_mhz = ofreq.unwrap(); + + let typical_as_nanos = typical_value * 1000.0 / freq_mhz as f64; + let (factor, unit) = if typical_as_nanos < 10f64.powi(0) { + (10f64.powi(3), "ps") + } else if typical_as_nanos < 10f64.powi(3) { + (10f64.powi(0), "ns") + } else if typical_as_nanos < 10f64.powi(6) { + (10f64.powi(-3), "µs") + } else if typical_as_nanos < 10f64.powi(9) { + (10f64.powi(-6), "ms") + } else { + (10f64.powi(-9), "s") + }; + + for val in values { + *val *= factor * 1000.0; + *val /= freq_mhz as f64; + } + + unit + } + + fn scale_throughputs( + &self, + _typical: f64, + _throughput: &Throughput, + _values: &mut [f64], + ) -> &'static str { + todo!(); + } + + fn scale_for_machines(&self, _values: &mut [f64]) -> &'static str { + // no scaling is needed + "ns" + } + } + + impl Measurement for RDTSCPMeasurement { + type Intermediate = (u32, u64); + type Value = Option; + + fn start(&self) -> Self::Intermediate { + let mut aux = 0; + let cycs = unsafe { x86_64::__rdtscp(&mut aux) }; + ( aux, cycs ) + } + fn end(&self, i: &Self::Intermediate) -> Self::Value { + let mut aux = 0; + let cycs = unsafe { x86_64::__rdtscp(&mut aux) }; + if i.0 != aux { + None + } else { + if cycs > i.1 { + Some(cycs - i.1) + } else { + Some(1) + } + } + } + fn add(&self, v1: &Self::Value, v2: &Self::Value) -> Self::Value { + Some((*v1)? + (*v2)?) + } + fn zero(&self) -> Self::Value { + Some(0) + } + fn one(&self) -> Self::Value { + Some(1) + } + fn lt(&self, val: &Self::Value, other: &Self::Value) -> bool { + if val.is_some() && other.is_some() { + val < other + } else { + false + } + } + fn debugprint(&self, val: &Self::Intermediate, other: &Self::Value) { + eprintln!("val: {val:?}, other: {other:?}"); + } + fn to_f64(&self, oval: &Self::Value) -> f64 { + match oval { + Some(val) => { + *val as f64 + } + None => { + f64::NAN // ??? this could cause trouble... + } + } + } + fn formatter(&self) -> &dyn ValueFormatter { + &RDTSCPValueFormatter { } + } + } +} diff --git a/src/plot/gnuplot_backend/distributions.rs b/src/plot/gnuplot_backend/distributions.rs index bf3dd2d92..e03d6b2b4 100644 --- a/src/plot/gnuplot_backend/distributions.rs +++ b/src/plot/gnuplot_backend/distributions.rs @@ -67,7 +67,7 @@ fn abs_distribution( statistic ))) .configure(Axis::BottomX, |a| { - a.set(Label(format!("Average time ({})", unit))) + a.set(Label(format!("Average time ({unit})"))) .set(Range::Limits(kde_xs_sample.min(), kde_xs_sample.max())) }) .configure(Axis::LeftY, |a| a.set(Label("Density (a.u.)"))) @@ -113,7 +113,7 @@ fn abs_distribution( }, ); - let path = context.report_path(id, &format!("{}.svg", statistic)); + let path = context.report_path(id, &format!("{statistic}.svg")); debug_script(&path, &figure); figure.set(Output(path)).draw().unwrap() } @@ -276,7 +276,7 @@ fn rel_distribution( }, ); - let path = context.report_path(id, &format!("change/{}.svg", statistic)); + let path = context.report_path(id, &format!("change/{statistic}.svg")); debug_script(&path, &figure); figure.set(Output(path)).draw().unwrap() } diff --git a/src/plot/gnuplot_backend/iteration_times.rs b/src/plot/gnuplot_backend/iteration_times.rs index 4d3547063..cb7b15d08 100644 --- a/src/plot/gnuplot_backend/iteration_times.rs +++ b/src/plot/gnuplot_backend/iteration_times.rs @@ -23,7 +23,7 @@ fn iteration_times_figure( }) .configure(Axis::LeftY, |a| { a.configure(Grid::Major, |g| g.show()) - .set(Label(format!("Average Iteration Time ({})", unit))) + .set(Label(format!("Average Iteration Time ({unit})"))) }) .plot( Points { @@ -102,7 +102,7 @@ fn iteration_times_comparison_figure( }) .configure(Axis::LeftY, |a| { a.configure(Grid::Major, |g| g.show()) - .set(Label(format!("Average Iteration Time ({})", unit))) + .set(Label(format!("Average Iteration Time ({unit})"))) }) .configure(Key, |k| { k.set(Justification::Left) diff --git a/src/plot/gnuplot_backend/pdf.rs b/src/plot/gnuplot_backend/pdf.rs index 385d7a5dc..67812259a 100644 --- a/src/plot/gnuplot_backend/pdf.rs +++ b/src/plot/gnuplot_backend/pdf.rs @@ -28,7 +28,7 @@ pub(crate) fn pdf( let y_label = if exponent == 0 { "Iterations".to_owned() } else { - format!("Iterations (x 10^{})", exponent) + format!("Iterations (x 10^{exponent})") }; let (xs, ys) = kde::sweep(scaled_avg_times, KDE_POINTS, None); @@ -46,7 +46,7 @@ pub(crate) fn pdf( .set(size.unwrap_or(SIZE)) .configure(Axis::BottomX, |a| { let xs_ = Sample::new(&xs); - a.set(Label(format!("Average time ({})", unit))) + a.set(Label(format!("Average time ({unit})"))) .set(Range::Limits(xs_.min(), xs_.max())) }) .configure(Axis::LeftY, |a| { @@ -248,7 +248,7 @@ pub(crate) fn pdf_small( .set(Font(DEFAULT_FONT)) .set(size.unwrap_or(SIZE)) .configure(Axis::BottomX, |a| { - a.set(Label(format!("Average time ({})", unit))) + a.set(Label(format!("Average time ({unit})"))) .set(Range::Limits(xs_.min(), xs_.max())) }) .configure(Axis::LeftY, |a| { @@ -317,7 +317,7 @@ fn pdf_comparison_figure( .set(Font(DEFAULT_FONT)) .set(size.unwrap_or(SIZE)) .configure(Axis::BottomX, |a| { - a.set(Label(format!("Average time ({})", unit))) + a.set(Label(format!("Average time ({unit})"))) }) .configure(Axis::LeftY, |a| a.set(Label("Density (a.u.)"))) .configure(Axis::RightY, |a| a.hide()) diff --git a/src/plot/plotters_backend/distributions.rs b/src/plot/plotters_backend/distributions.rs index 448caa2f5..d4dc25164 100644 --- a/src/plot/plotters_backend/distributions.rs +++ b/src/plot/plotters_backend/distributions.rs @@ -15,12 +15,15 @@ fn abs_distribution( ) { let ci = &estimate.confidence_interval; let typical = ci.upper_bound; + debug_assert!(typical.is_finite()); let mut ci_values = [ci.lower_bound, ci.upper_bound, estimate.point_estimate]; let unit = formatter.scale_values(typical, &mut ci_values); let (lb, ub, point) = (ci_values[0], ci_values[1], ci_values[2]); let start = lb - (ub - lb) / 9.; + debug_assert!(start.is_finite()); let end = ub + (ub - lb) / 9.; + debug_assert!(end.is_finite()); let mut scaled_xs: Vec = distribution.iter().cloned().collect(); let _ = formatter.scale_values(typical, &mut scaled_xs); let scaled_xs_sample = Sample::new(&scaled_xs); @@ -56,7 +59,11 @@ fn abs_distribution( let root_area = SVGBackend::new(&path, size.unwrap_or(SIZE)).into_drawing_area(); let x_range = plotters::data::fitting_range(kde_xs_sample.iter()); + debug_assert!(x_range.start.is_finite()); + debug_assert!(x_range.end.is_finite()); let mut y_range = plotters::data::fitting_range(ys.iter()); + debug_assert!(y_range.start.is_finite()); + debug_assert!(y_range.end.is_finite()); y_range.end *= 1.1; @@ -164,6 +171,8 @@ fn rel_distribution( ) { let ci = &estimate.confidence_interval; let (lb, ub) = (ci.lower_bound, ci.upper_bound); + debug_assert!(lb.is_finite()); + debug_assert!(ub.is_finite()); let start = lb - (ub - lb) / 9.; let end = ub + (ub - lb) / 9.; diff --git a/src/routine.rs b/src/routine.rs index c89e406ff..432094365 100644 --- a/src/routine.rs +++ b/src/routine.rs @@ -115,10 +115,12 @@ pub(crate) trait Routine { .bench(measurement, &[n * 2], parameter) .first() .unwrap(); + assert!(t_now - t_prev > f64::MIN_POSITIVE); let t = (t_prev + 2. * t_now) / 5.; let stdev = (sq(t_prev - t) + sq(t_now - 2. * t)).sqrt(); // println!("Sample: {} {:.2}", n, stdev / t); let elapsed = time_start.elapsed(); + assert!(elapsed > Duration::new(0, 1)); if (stdev < target_rel_stdev * t && elapsed > minimum_bench_duration) || elapsed > maximum_bench_duration { diff --git a/src/stats/univariate/kde/mod.rs b/src/stats/univariate/kde/mod.rs index c54de55a2..ea43c6528 100644 --- a/src/stats/univariate/kde/mod.rs +++ b/src/stats/univariate/kde/mod.rs @@ -58,14 +58,30 @@ where pub fn estimate(&self, x: A) -> A { let _0 = A::cast(0); let slice = self.sample; - let h = self.bandwidth; + let mut h = self.bandwidth; + debug_assert!(h.is_finite()); + debug_assert!(!h.is_sign_negative()); + //assert!(!h.is_zero()); + if h.is_zero() { + h = A::min_positive_value(); + } let n = A::cast(slice.len()); + debug_assert!(n.is_finite()); + debug_assert!(!n.is_zero()); let sum = slice .iter() .fold(_0, |acc, &x_i| acc + self.kernel.evaluate((x - x_i) / h)); + debug_assert!(sum.is_finite()); + //assert!(!sum.is_zero()); - sum / (h * n) + let mut res = sum / (h * n); + debug_assert!(res.is_finite()); + if res.is_zero() { + res = A::min_positive_value(); + } + //assert!(!res.is_zero()); + res } } diff --git a/tests/criterion_tests.rs b/tests/criterion_tests.rs index 52811ff8e..13cbc00d1 100644 --- a/tests/criterion_tests.rs +++ b/tests/criterion_tests.rs @@ -456,14 +456,6 @@ fn test_benchmark_group_without_input() { group.finish(); } -#[test] -fn test_criterion_doesnt_panic_if_measured_time_is_zero() { - let dir = temp_dir(); - let mut c = short_benchmark(&dir); - c.bench_function("zero_time", |bencher| { - bencher.iter_custom(|_iters| Duration::new(0, 0)); - }); -} mod macros { use super::{criterion_group, criterion_main, Criterion};