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

Sequence functionality: API revisions #483

Merged
merged 7 commits into from Jun 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 0 additions & 54 deletions benches/misc.rs
Expand Up @@ -8,7 +8,6 @@ const RAND_BENCH_N: u64 = 1000;
use test::Bencher;

use rand::prelude::*;
use rand::seq::*;

#[bench]
fn misc_gen_bool_const(b: &mut Bencher) {
Expand Down Expand Up @@ -108,59 +107,6 @@ sample_binomial!(misc_binomial_100, 100, 0.99);
sample_binomial!(misc_binomial_1000, 1000, 0.01);
sample_binomial!(misc_binomial_1e12, 1000_000_000_000, 0.2);

#[bench]
fn misc_shuffle_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &mut [usize] = &mut [1; 100];
b.iter(|| {
rng.shuffle(x);
x[0]
})
}

#[bench]
fn misc_sample_iter_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
sample_iter(&mut rng, x, 10).unwrap_or_else(|e| e)
})
}

#[bench]
fn misc_sample_slice_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
sample_slice(&mut rng, x, 10)
})
}

#[bench]
fn misc_sample_slice_ref_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
sample_slice_ref(&mut rng, x, 10)
})
}

macro_rules! sample_indices {
($name:ident, $amount:expr, $length:expr) => {
#[bench]
fn $name(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
b.iter(|| {
sample_indices(&mut rng, $length, $amount)
})
}
}
}

sample_indices!(misc_sample_indices_10_of_1k, 10, 1000);
sample_indices!(misc_sample_indices_50_of_1k, 50, 1000);
sample_indices!(misc_sample_indices_100_of_1k, 100, 1000);

#[bench]
fn gen_1k_iter_repeat(b: &mut Bencher) {
use std::iter;
Expand Down
94 changes: 94 additions & 0 deletions benches/seq.rs
@@ -0,0 +1,94 @@
#![feature(test)]

extern crate test;
extern crate rand;

use test::Bencher;

use rand::prelude::*;
use rand::seq::*;

#[bench]
fn seq_shuffle_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &mut [usize] = &mut [1; 100];
b.iter(|| {
x.shuffle(&mut rng);
x[0]
})
}

#[bench]
fn seq_slice_choose_1_of_1000(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 1000];
b.iter(|| {
x.choose(&mut rng)
})
}

#[bench]
fn seq_slice_choose_multiple_1_of_1000(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 1000];
b.iter(|| {
x.choose_multiple(&mut rng, 1).cloned().next()
})
}

#[bench]
fn seq_slice_choose_multiple_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
let mut buf = [0; 10];
b.iter(|| {
for (v, slot) in x.choose_multiple(&mut rng, buf.len()).zip(buf.iter_mut()) {
*slot = *v;
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this does measure both the time to call choose_multiple and the time to write into buf, whereas we only really care about the former.

You could do something like x.choose_multiple(&mut rng, 10).sum::<usize>().

But I kind'a worry that the compiler would be smart enough to realize that since all values in x are 1, that it might optimize to simply returning 10. That could be fixed by simply changing one of the values in x though.

But since the writes are consecutive, they should be fast and maybe we shouldn't worry about this? I don't have strong feelings either way.

As a complete aside, it'd be really great if there was a version of Bencher::iter which you could return an Iterator to, and it would consume the iterator. But I'm not sure if there's any appetite for changing that API since apparently the idea is to deprecate it.

Copy link
Member Author

@dhardy dhardy Jun 15, 2018

Choose a reason for hiding this comment

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

I don't think this is important. (I have tested that writing to a buffer is faster than collecting into a vector.)

}
buf
})
}

#[bench]
fn seq_iter_choose_from_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
x.iter().cloned().choose(&mut rng)
})
}

#[bench]
fn seq_iter_choose_multiple_10_of_100(b: &mut Bencher) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this function need to be flagged with #[cfg(feature = "alloc")]? Or does #[bench] only work in std builds anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

I cfg'd the whole module and tested (cargo +nightly test --benches --no-default-features [--features=alloc]). The bit I don't get is why there are no error messages with alloc without std, but there aren't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, cool.

Copy link
Member Author

Choose a reason for hiding this comment

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

The cfg thing made the tests work by effectively removing all the benchmarks — took me a while to work out why no benchmarks were getting run. But we don't need to benchmark without std anyway.

let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
b.iter(|| {
x.iter().cloned().choose_multiple(&mut rng, 10)
})
}

#[bench]
fn seq_iter_choose_multiple_fill_10_of_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
let x : &[usize] = &[1; 100];
let mut buf = [0; 10];
b.iter(|| {
x.iter().cloned().choose_multiple_fill(&mut rng, &mut buf)
})
}

macro_rules! sample_indices {
($name:ident, $amount:expr, $length:expr) => {
#[bench]
fn $name(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(thread_rng()).unwrap();
b.iter(|| {
sample_indices(&mut rng, $length, $amount)
})
}
}
}

sample_indices!(seq_sample_indices_10_of_1k, 10, 1000);
sample_indices!(seq_sample_indices_50_of_1k, 50, 1000);
sample_indices!(seq_sample_indices_100_of_1k, 100, 1000);
4 changes: 2 additions & 2 deletions examples/monty-hall.rs
Expand Up @@ -63,8 +63,8 @@ fn simulate<R: Rng>(random_door: &Uniform<u32>, rng: &mut R)
// Returns the door the game host opens given our choice and knowledge of
// where the car is. The game host will never open the door with the car.
fn game_host_open<R: Rng>(car: u32, choice: u32, rng: &mut R) -> u32 {
let choices = free_doors(&[car, choice]);
rand::seq::sample_slice(rng, &choices, 1)[0]
use rand::seq::SliceRandom;
*free_doors(&[car, choice]).choose(rng).unwrap()
}

// Returns the door we switch to, given our current choice and
Expand Down
102 changes: 18 additions & 84 deletions src/lib.rs
Expand Up @@ -588,64 +588,35 @@ pub trait Rng: RngCore {

/// Return a random element from `values`.
///
/// Return `None` if `values` is empty.
///
/// # Example
///
/// ```
/// use rand::{thread_rng, Rng};
///
/// let choices = [1, 2, 4, 8, 16, 32];
/// let mut rng = thread_rng();
/// println!("{:?}", rng.choose(&choices));
/// assert_eq!(rng.choose(&choices[..0]), None);
/// ```
/// Deprecated: use [`SliceRandom::choose`] instead.
///
/// [`SliceRandom::choose`]: seq/trait.SliceRandom.html#method.choose
#[deprecated(since="0.6.0", note="use SliceRandom::choose instead")]
fn choose<'a, T>(&mut self, values: &'a [T]) -> Option<&'a T> {
if values.is_empty() {
None
} else {
Some(&values[self.gen_range(0, values.len())])
}
use seq::SliceRandom;
values.choose(self)
}

/// Return a mutable pointer to a random element from `values`.
///
/// Return `None` if `values` is empty.
/// Deprecated: use [`SliceRandom::choose_mut`] instead.
///
/// [`SliceRandom::choose_mut`]: seq/trait.SliceRandom.html#method.choose_mut
#[deprecated(since="0.6.0", note="use SliceRandom::choose_mut instead")]
fn choose_mut<'a, T>(&mut self, values: &'a mut [T]) -> Option<&'a mut T> {
if values.is_empty() {
None
} else {
let len = values.len();
Some(&mut values[self.gen_range(0, len)])
}
use seq::SliceRandom;
values.choose_mut(self)
}

/// Shuffle a mutable slice in place.
///
/// This applies Durstenfeld's algorithm for the [Fisher–Yates shuffle](
/// https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm)
/// which produces an unbiased permutation.
///
/// # Example
///
/// ```
/// use rand::{thread_rng, Rng};
///
/// let mut rng = thread_rng();
/// let mut y = [1, 2, 3];
/// rng.shuffle(&mut y);
/// println!("{:?}", y);
/// rng.shuffle(&mut y);
/// println!("{:?}", y);
/// ```
/// Deprecated: use [`SliceRandom::shuffle`] instead.
///
/// [`SliceRandom::shuffle`]: seq/trait.SliceRandom.html#method.shuffle
#[deprecated(since="0.6.0", note="use SliceRandom::shuffle instead")]
fn shuffle<T>(&mut self, values: &mut [T]) {
let mut i = values.len();
while i >= 2 {
// invariant: elements with index >= i have been locked in place.
i -= 1;
// lock element i in place.
values.swap(i, self.gen_range(0, i + 1));
}
use seq::SliceRandom;
values.shuffle(self)
}
}

Expand Down Expand Up @@ -999,46 +970,13 @@ mod test {
}
}

#[test]
fn test_choose() {
let mut r = rng(107);
assert_eq!(r.choose(&[1, 1, 1]).map(|&x|x), Some(1));

let v: &[isize] = &[];
assert_eq!(r.choose(v), None);
}

#[test]
fn test_shuffle() {
let mut r = rng(108);
let empty: &mut [isize] = &mut [];
r.shuffle(empty);
let mut one = [1];
r.shuffle(&mut one);
let b: &[_] = &[1];
assert_eq!(one, b);

let mut two = [1, 2];
r.shuffle(&mut two);
assert!(two == [1, 2] || two == [2, 1]);

let mut x = [1, 1, 1];
r.shuffle(&mut x);
let b: &[_] = &[1, 1, 1];
assert_eq!(x, b);
}

#[test]
fn test_rng_trait_object() {
use distributions::{Distribution, Standard};
let mut rng = rng(109);
let mut r = &mut rng as &mut RngCore;
r.next_u32();
r.gen::<i32>();
let mut v = [1, 1, 1];
r.shuffle(&mut v);
let b: &[_] = &[1, 1, 1];
assert_eq!(v, b);
assert_eq!(r.gen_range(0, 1), 0);
let _c: u8 = Standard.sample(&mut r);
}
Expand All @@ -1051,10 +989,6 @@ mod test {
let mut r = Box::new(rng) as Box<RngCore>;
r.next_u32();
r.gen::<i32>();
let mut v = [1, 1, 1];
r.shuffle(&mut v);
let b: &[_] = &[1, 1, 1];
assert_eq!(v, b);
assert_eq!(r.gen_range(0, 1), 0);
let _c: u8 = Standard.sample(&mut r);
}
Expand Down
1 change: 1 addition & 0 deletions src/prelude.rs
Expand Up @@ -26,3 +26,4 @@
#[doc(no_inline)] #[cfg(feature="std")] pub use rngs::ThreadRng;
#[doc(no_inline)] pub use {Rng, RngCore, CryptoRng, SeedableRng};
#[doc(no_inline)] #[cfg(feature="std")] pub use {FromEntropy, random, thread_rng};
#[doc(no_inline)] pub use seq::{SliceRandom, IteratorRandom};
4 changes: 0 additions & 4 deletions src/rngs/thread.rs
Expand Up @@ -132,10 +132,6 @@ mod test {
use Rng;
let mut r = ::thread_rng();
r.gen::<i32>();
let mut v = [1, 1, 1];
r.shuffle(&mut v);
let b: &[_] = &[1, 1, 1];
assert_eq!(v, b);
assert_eq!(r.gen_range(0, 1), 0);
}
}