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

Update SmallRng and remove rand_xorshift crate #623

Merged
merged 4 commits into from Oct 17, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Expand Up @@ -32,6 +32,7 @@ members = ["rand_core", "rand_isaac", "rand_chacha", "rand_hc128", "rand_pcg", "

[dependencies]
rand_core = { path = "rand_core", version = "0.3", default-features = false }
rand_pcg = { path = "rand_pcg", version = "0.1" }
# only for deprecations and benches:
rand_isaac = { path = "rand_isaac", version = "0.1" }
rand_chacha = { path = "rand_chacha", version = "0.1" }
Expand Down
10 changes: 10 additions & 0 deletions benches/generators.rs
Expand Up @@ -13,6 +13,7 @@ extern crate rand;
extern crate rand_isaac;
extern crate rand_chacha;
extern crate rand_hc128;
extern crate rand_pcg;
extern crate rand_xorshift;

const RAND_BENCH_N: u64 = 1000;
Expand All @@ -27,6 +28,7 @@ use rand::rngs::{OsRng, JitterRng, EntropyRng};
use rand_isaac::{IsaacRng, Isaac64Rng};
use rand_chacha::ChaChaRng;
use rand_hc128::{Hc128Rng, Hc128Core};
use rand_pcg::{Lcg64Xsh32, Mcg128Xsl64};
use rand_xorshift::XorShiftRng;

macro_rules! gen_bytes {
Expand All @@ -47,6 +49,8 @@ macro_rules! gen_bytes {
}

gen_bytes!(gen_bytes_xorshift, XorShiftRng::from_entropy());
gen_bytes!(gen_bytes_lcg64_xsh32, Lcg64Xsh32::from_entropy());
gen_bytes!(gen_bytes_mcg128_xsh64, Mcg128Xsl64::from_entropy());
gen_bytes!(gen_bytes_chacha20, ChaChaRng::from_entropy());
gen_bytes!(gen_bytes_hc128, Hc128Rng::from_entropy());
gen_bytes!(gen_bytes_isaac, IsaacRng::from_entropy());
Expand All @@ -73,6 +77,8 @@ macro_rules! gen_uint {
}

gen_uint!(gen_u32_xorshift, u32, XorShiftRng::from_entropy());
gen_uint!(gen_u32_lcg64_xsh32, u32, Lcg64Xsh32::from_entropy());
gen_uint!(gen_u32_mcg128_xsh64, u32, Mcg128Xsl64::from_entropy());
gen_uint!(gen_u32_chacha20, u32, ChaChaRng::from_entropy());
gen_uint!(gen_u32_hc128, u32, Hc128Rng::from_entropy());
gen_uint!(gen_u32_isaac, u32, IsaacRng::from_entropy());
Expand All @@ -82,6 +88,8 @@ gen_uint!(gen_u32_small, u32, SmallRng::from_entropy());
gen_uint!(gen_u32_os, u32, OsRng::new().unwrap());

gen_uint!(gen_u64_xorshift, u64, XorShiftRng::from_entropy());
gen_uint!(gen_u64_lcg64_xsh32, u64, Lcg64Xsh32::from_entropy());
gen_uint!(gen_u64_mcg128_xsh64, u64, Mcg128Xsl64::from_entropy());
gen_uint!(gen_u64_chacha20, u64, ChaChaRng::from_entropy());
gen_uint!(gen_u64_hc128, u64, Hc128Rng::from_entropy());
gen_uint!(gen_u64_isaac, u64, IsaacRng::from_entropy());
Expand Down Expand Up @@ -115,6 +123,8 @@ macro_rules! init_gen {
}

init_gen!(init_xorshift, XorShiftRng);
init_gen!(init_lcg64_xsh32, Lcg64Xsh32);
init_gen!(init_mcg128_xsh64, Mcg128Xsl64);
init_gen!(init_hc128, Hc128Rng);
init_gen!(init_isaac, IsaacRng);
init_gen!(init_isaac64, Isaac64Rng);
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Expand Up @@ -245,6 +245,7 @@ extern crate rand_core;
extern crate rand_isaac; // only for deprecations
extern crate rand_chacha; // only for deprecations
extern crate rand_hc128;
extern crate rand_pcg;
extern crate rand_xorshift;

#[cfg(feature = "log")] #[macro_use] extern crate log;
Expand Down
17 changes: 10 additions & 7 deletions src/prng/mod.rs
Expand Up @@ -45,16 +45,17 @@
//!
//! | name | full name | performance | memory | quality | period | features |
//! |------|-----------|-------------|--------|---------|--------|----------|
//! | [`XorShiftRng`] | Xorshift 32/128 | ★★★☆☆ | 16 bytes | ★☆☆☆☆ | `u32` * 2<sup>128</sup> - 1 | — |
//! | [`Pcg32`] | PCG XSH RR 64/32 (LCG) | ★★★☆☆ | 16 bytes | ★★★☆☆ | `u32` * 2<sup>64</sup> | — |
//! | [`Pcg64Mcg`] | PCG XSL 128/64 (MCG) | ★★★★☆ | 16 bytes | ★★★☆☆ | `u64` * 2<sup>126</sup> | — |
//! | [`XorShiftRng`] | Xorshift 32/128 | ★★★★☆ | 16 bytes | ★☆☆☆☆ | `u32` * 2<sup>128</sup> - 1 | — |
//!
// Quality stars [not rendered in documentation]:
// 5. reserved for crypto-level (e.g. ChaCha8, ISAAC)
// 4. good performance on TestU01 and PractRand, good theory
// 5. proven cryptographic quality (e.g. ChaCha20)
// 4. potentially cryptographic, but low margin or lack of theory (e.g. ChaCha8, ISAAC)
// 3. good performance on TestU01 and PractRand, good theory
// (e.g. PCG, truncated Xorshift*)
// 3. good performance on TestU01 and PractRand, but "falling through the
// cracks" or insufficient theory (e.g. SFC, Xoshiro)
// 2. imperfect performance on tests or other limiting properties, but not
// terrible (e.g. Xoroshiro128+)
// 2. imperfect performance on tests or other limiting properties, or
// insufficient theory, but not terrible (e.g. SFC, Xoshiro, Xoroshiro128+)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I disagree with the example RNGs here. Maybe just remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

This stuff isn't in the printed doc (only two leading slashes). Yeah, it's difficult to know how to rate things; there's enough complexity to write a book on it or we could simply leave users with no hints at all. I already changed this rating system to demote PCG to 3 stars because I don't think it should be compared so closely with crypto RNGs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough. The 4 and 5 star ratings are unused anyway, because the CSPRNG sections does not have a quality rating.

Copy link
Member Author

Choose a reason for hiding this comment

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

It could, but unless all PRNGs were put in a single table it wouldn't make much sense. Further comments on this in #633.

// 1. clear deficiencies in test results, cycle length, theory, or other
// properties (e.g. Xorshift)
//
Expand Down Expand Up @@ -297,6 +298,8 @@
//! [`rngs` module]: ../rngs/index.html
//! [basic PRNGs]: #basic-pseudo-random-number-generators-prngs
//! [CSPRNGs]: #cryptographically-secure-pseudo-random-number-generators-csprngs
//! [`Pcg32`]: ../../rand_pcg/type.Pcg32.html
//! [`Pcg64Mcg`]: ../../rand_pcg/type.Pcg64Mcg.html
//! [`XorShiftRng`]: ../../rand_xorshift/struct.XorShiftRng.html
//! [`ChaChaRng`]: ../../rand_chacha/struct.ChaChaRng.html
//! [`Hc128Rng`]: ../../rand_hc128/struct.Hc128Rng.html
Expand Down
3 changes: 0 additions & 3 deletions src/rngs/mod.rs
Expand Up @@ -48,9 +48,6 @@
//!
//! - [`SmallRng`] is a PRNG chosen for low memory usage, high performance and
//! good statistical quality.
//! The current algorithm (plain Xorshift) unfortunately performs
//! poorly in statistical quality test suites (TestU01 and PractRand) and will
//! be replaced in the next major release.
//! - [`StdRng`] is a CSPRNG chosen for good performance and trust of security
//! (based on reviews, maturity and usage). The current algorithm is HC-128,
//! which is one of the recommendations by ECRYPT's eSTREAM project.
Expand Down
24 changes: 15 additions & 9 deletions src/rngs/small.rs
Expand Up @@ -9,7 +9,11 @@
//! A small fast RNG

use {RngCore, SeedableRng, Error};
use ::rand_xorshift::XorShiftRng;

#[cfg(all(rust_1_26, target_pointer_width = "64"))]
type Rng = ::rand_pcg::Pcg64Mcg;
#[cfg(not(all(rust_1_26, target_pointer_width = "64")))]
type Rng = ::rand_pcg::Pcg32;

/// An RNG recommended when small state, cheap initialization and good
/// performance are required. The PRNG algorithm in `SmallRng` is chosen to be
Expand All @@ -20,9 +24,11 @@ use ::rand_xorshift::XorShiftRng;
/// future library versions may use a different internal generator with
/// different output. Further, this generator may not be portable and can
/// produce different output depending on the architecture. If you require
/// reproducible output, use a named RNG, for example [`XorShiftRng`].
/// reproducible output, use a named RNG. Refer to the documentation on the
/// [`prng` module](../prng/index.html).
///
/// The current algorithm used on all platforms is [Xorshift].
/// The current algorithm is [`Pcg64Mcg`] on 64-bit platforms with Rust version
/// 1.26 and later, or [`Pcg32`] otherwise.
///
/// # Examples
///
Expand Down Expand Up @@ -61,10 +67,10 @@ use ::rand_xorshift::XorShiftRng;
/// [`FromEntropy`]: ../trait.FromEntropy.html
/// [`StdRng`]: struct.StdRng.html
/// [`thread_rng`]: ../fn.thread_rng.html
/// [Xorshift]: ../../rand_xorshift/struct.XorShiftRng.html
/// [`XorShiftRng`]: ../../rand_xorshift/struct.XorShiftRng.html
/// [`Pcg64Mcg`]: https://docs.rs/rand_pcg/0.1.0/rand_pcg/type.Pcg64Mcg.html
/// [`Pcg32`]: https://docs.rs/rand_pcg/0.1.0/rand_pcg/type.Pcg32.html
#[derive(Clone, Debug)]
pub struct SmallRng(XorShiftRng);
pub struct SmallRng(Rng);

impl RngCore for SmallRng {
#[inline(always)]
Expand All @@ -87,13 +93,13 @@ impl RngCore for SmallRng {
}

impl SeedableRng for SmallRng {
type Seed = <XorShiftRng as SeedableRng>::Seed;
type Seed = <Rng as SeedableRng>::Seed;

fn from_seed(seed: Self::Seed) -> Self {
SmallRng(XorShiftRng::from_seed(seed))
SmallRng(Rng::from_seed(seed))
}

fn from_rng<R: RngCore>(rng: R) -> Result<Self, Error> {
XorShiftRng::from_rng(rng).map(SmallRng)
Rng::from_rng(rng).map(SmallRng)
}
}
10 changes: 5 additions & 5 deletions src/seq/mod.rs
Expand Up @@ -512,7 +512,7 @@ pub fn sample_slice_ref<'a, R, T>(rng: &mut R, slice: &'a [T], amount: usize) ->
mod test {
use super::*;
#[cfg(feature = "alloc")] use {Rng, SeedableRng};
#[cfg(feature = "alloc")] use ::rand_xorshift::XorShiftRng;
#[cfg(feature = "alloc")] use rngs::SmallRng;
#[cfg(all(feature="alloc", not(feature="std")))]
use alloc::vec::Vec;

Expand Down Expand Up @@ -753,7 +753,7 @@ mod test {
#[cfg(feature = "alloc")]
#[allow(deprecated)]
fn test_sample_slice() {
let xor_rng = XorShiftRng::from_seed;
let seeded_rng = SmallRng::from_seed;

let mut r = ::test::rng(403);

Expand All @@ -764,16 +764,16 @@ mod test {
r.fill(&mut seed);

// assert the basics work
let regular = index::sample(&mut xor_rng(seed), length, amount);
let regular = index::sample(&mut seeded_rng(seed), length, amount);
assert_eq!(regular.len(), amount);
assert!(regular.iter().all(|e| e < length));

// also test that sampling the slice works
let vec: Vec<u32> = (0..(length as u32)).collect();
let result = sample_slice(&mut xor_rng(seed), &vec, amount);
let result = sample_slice(&mut seeded_rng(seed), &vec, amount);
assert_eq!(result, regular.iter().map(|i| i as u32).collect::<Vec<_>>());

let result = sample_slice_ref(&mut xor_rng(seed), &vec, amount);
let result = sample_slice_ref(&mut seeded_rng(seed), &vec, amount);
assert!(result.iter().zip(regular.iter()).all(|(i,j)| **i == j as u32));
}
}
Expand Down