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

Speed up Uniform Duration sampling. #583

Merged
merged 6 commits into from
Aug 23, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
27 changes: 27 additions & 0 deletions benches/distributions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const RAND_BENCH_N: u64 = 1000;

use std::mem::size_of;
use test::Bencher;
use std::time::Duration;

use rand::{Rng, FromEntropy};
use rand::rngs::SmallRng;
Expand Down Expand Up @@ -54,6 +55,26 @@ macro_rules! distr_float {
}
}

macro_rules! distr_duration {
($fnn:ident, $distr:expr) => {
#[bench]
fn $fnn(b: &mut Bencher) {
let mut rng = SmallRng::from_entropy();
let distr = $distr;

b.iter(|| {
let mut accum = Duration::new(0, 0);
for _ in 0..::RAND_BENCH_N {
let x: Duration = distr.sample(&mut rng);
accum = accum.checked_add(x).unwrap_or(Duration::new(u64::max_value(), 999_999_999));
}
accum
});
b.bytes = size_of::<Duration>() as u64 * ::RAND_BENCH_N;
}
}
}

macro_rules! distr {
($fnn:ident, $ty:ty, $distr:expr) => {
#[bench]
Expand Down Expand Up @@ -85,6 +106,12 @@ distr_int!(distr_uniform_i128, i128, Uniform::new(-123_456_789_123i128, 123_456_
distr_float!(distr_uniform_f32, f32, Uniform::new(2.26f32, 2.319));
distr_float!(distr_uniform_f64, f64, Uniform::new(2.26f64, 2.319));

distr_duration!(distr_uniform_duration_largest, Uniform::new(Duration::new(0, 0), Duration::new(u64::max_value(), 999_999_999)));
distr_duration!(distr_uniform_duration_large, Uniform::new(Duration::new(0, 0), Duration::new(u64::max_value()/1000, 1_000_000_000/2)));
distr_duration!(distr_uniform_duration_one, Uniform::new(Duration::new(0, 0), Duration::new(1, 0)));
distr_duration!(distr_uniform_duration_variety, Uniform::new(Duration::new(10000, 423423), Duration::new(200000, 6969954)));


// standard
distr_int!(distr_standard_i8, i8, Standard);
distr_int!(distr_standard_i16, i16, Standard);
Expand Down
82 changes: 55 additions & 27 deletions src/distributions/uniform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,18 +693,23 @@ uniform_float_impl! { f64x8, u64x8, f64, u64, 64 - 52 }
#[cfg(feature = "std")]
#[derive(Clone, Copy, Debug)]
pub struct UniformDuration {
offset: Duration,
mode: UniformDurationMode,
offset: u32,
Copy link
Member

@dhardy dhardy Aug 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The offset is only used by Large mode so can be moved there.

Edit: sorry, I see your comment about this. Fine. It should make no difference to the size of the struct either way.

}

#[cfg(feature = "std")]
#[derive(Debug, Copy, Clone)]
enum UniformDurationMode {
Small {
secs: u64,
nanos: Uniform<u32>,
},
Medium {
nanos: Uniform<u64>,
},
Large {
size: Duration,
max_secs: u64,
max_nanos: u32,
secs: Uniform<u64>,
}
}
Expand Down Expand Up @@ -737,52 +742,75 @@ impl UniformSampler for UniformDuration {
let low = *low_b.borrow();
let high = *high_b.borrow();
assert!(low <= high, "Uniform::new_inclusive called with `low > high`");
let size = high - low;
let nanos = size
.as_secs()
.checked_mul(1_000_000_000)
.and_then(|n| n.checked_add(size.subsec_nanos() as u64));

let mode = match nanos {
Some(nanos) => {
UniformDurationMode::Small {
nanos: Uniform::new_inclusive(0, nanos),
}

let low_s = low.as_secs();
let low_n = low.subsec_nanos();
let mut high_s = high.as_secs();
let mut high_n = high.subsec_nanos();

if high_n < low_n {
high_s = high_s - 1;
high_n = high_n + 1_000_000_000;
} else {
high_s = high_s;
high_n = high_n;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant

}

let mode = if low_s == high_s {
UniformDurationMode::Small {
secs: low_s,
nanos: Uniform::new_inclusive(low_n, high_n),
}
None => {
} else {
let max = high_s
.checked_mul(1_000_000_000)
.and_then(|n| n.checked_add(high_n as u64));

if let Some(higher_bound) = max {
let lower_bound = low_s * 1_000_000_000 + low_n as u64;
UniformDurationMode::Medium {
nanos: Uniform::new_inclusive(lower_bound, higher_bound),
}
} else {
// An offset is applied to simplify generation of nanoseconds
let max_nanos = high_n - low_n;
UniformDurationMode::Large {
size: size,
secs: Uniform::new_inclusive(0, size.as_secs()),
max_secs: high_s,
max_nanos,
secs: Uniform::new_inclusive(low_s, high_s),
}
}
};

UniformDuration {
mode,
offset: low,
offset: low_n,
}
}

#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Duration {
let d = match self.mode {
UniformDurationMode::Small { nanos } => {
match self.mode {
UniformDurationMode::Small { secs, nanos } => {
let n = nanos.sample(rng);
Duration::new(secs, n)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this is now incorrect since it's possible that n > 10^9 (in this case new should panic). Hopefully using an if to check here won't have too big an impact.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duration::new() actually handles that already. In the documentation it says:

If the number of nanoseconds is greater than 1 billion (the number of nanoseconds in a second), then it will carry over into the seconds provided.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a debug_assert?

}
UniformDurationMode::Medium { nanos } => {
let nanos = nanos.sample(rng);
Duration::new(nanos / 1_000_000_000, (nanos % 1_000_000_000) as u32)
}
UniformDurationMode::Large { size, secs } => {
UniformDurationMode::Large { max_secs, max_nanos, secs } => {
// constant folding means this is at least as fast as `gen_range`
let nano_range = Uniform::new(0, 1_000_000_000);
loop {
let d = Duration::new(secs.sample(rng), nano_range.sample(rng));
if d <= size {
break d;
let s = secs.sample(rng);
let n = nano_range.sample(rng);
if !(s == max_secs && n > max_nanos) {
let sum = n + self.offset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After adding the offset back, s * 1_000_000_000 + sum is correct, but again sum may be more than 10^9. Again, using an if to check whether to decrement sum and increment s is probably the best option.

break Duration::new(s, sum);
}
}
}
};

self.offset + d
}
}
}

Expand Down