std::rand: add MT19937 and MT19937-64. (Mersenne Twister RNG)

[huonw]

Implementations of the 32- and 64-bit variants of the Mersenne Twister
RNG algorithm. C++11 mandates "mersenne_twister_engine" in its random
number library, so this is just imitating that.

(cc #9810, separate because that PR is large enough and this is a self-contained change.)

[alexcrichton]

Shouldn't this be expressable via the SeedableRng trait? I figured you'd have two implementations like:

impl SeedableRng<u32> for MT19937Rng { ... }
impl<'self> SeedableRng<&'self [u32]> for MT19937Rng { ... }

[alexcrichton]

Oh wait, I forgot we couldn't impl the same trait twice for the same type :(

[sfackler]

You can do it with an extra layer of indirection:

trait SeedInput { ... }
impl SeedInput for u32 { ... }
impl<'self> SeedInput for &'self [u32] { ... }
impl<I: SeedInput> SeedableRng<I> for MT19937Rng { ... }

It does require callers to use SeedInput though :(

[huonw]

@sfackler ... I don't understand how what you have there is different to the code as it stands.

(Also, I'm fairly sure you don't have to use SeedInput.)

@alexcrichton yeah, it's unfortunate :(

[alexcrichton]

This looks fantastic, nice job!

Could you add some examples in the module documentation about how to use it. Just something which has the necessary imports and goes through the process of creating an Rng, reseeding it, and things like that. I don't think it really needs to demonstrate how to use Rng, but rather just how to get access to the mersenne things.

The name of the module is pretty long: std::rand::mersenne_twister::MT19937Rng, so I'd also be ok shortening it, but not really that necessary. I suppose you're not supposed to be using this that much anyway.

Regardless, with a doc-block expansion, r=me

[huonw]

The name of the module is pretty long: std::rand::mersenne_twister::MT19937Rng, so I'd also be ok shortening it, but not really that necessary

I could shorten it to std::rand::mersenne::MT19937Rng I guess; although in theory one writes use std::rand::mersenne_twister::MT19937Rng only once. (Also, I'm thinking that the Rng suffix is possibly pointless?)

[alexcrichton]

std::rand::mersenne::MT19937 seems appropriate to me, but don't lose too much sleep over the decision.

[huonw]

How about IsaacRng → Isaac and similarly for Isaac64Rng and XorShiftRng? (I guess the non-specific TaskRng, OSRng and StdRng can't really drop the Rng?)

[alexcrichton]

I'm personally always in favor of shortening names when everything has a common prefix and it's pretty much implied by where it is, so I'm all for it!

[brson]

I'd like to question how many RNG's we need in the standard library. In general I don't believe we should providing an abundance of options for any particular need, but rather a single good default. Already we have a good CSPRNG and a fast RNG. What role does this one have? Is it fast enough to replace XorShift? Is it slow enough that Isaac is better for all purposes?

[thestinger]

@brson: Xorshift is ridiculously fast but the distribution can be very bad, and without good input parameters it can end up having a very short sequence of output before it repeats itself.

I think there's a place for an RNG that's good enough for general non-cryptographic purposes, if it's actually significantly faster than ISAAC. If it's not, then we should just stick with ISAAC.

[huonw]

I agree with @thestinger, fwiw.

I'll get some numbers when I rebase on top of #9810, since that makes the benchmarks more representative.

[thestinger]

It would also be a good idea to look into replacing ISAAC with one of the eSTREAM ciphers, for both improved security (ISAAC has significant known weaknesses) and performance. Modern stream ciphers are designed for modern CPUs with vector processors.

https://en.wikipedia.org/wiki/ESTREAM

[huonw]

ISAAC has significant known weaknesses

Citation? The only ones I know of is are Pudovkina and Aumasson (wikipedia agrees), and I don't think that either are considered significant. In any case, I was/am considering "upgrading" us to the ISAAC+ Aumasson proposes, but I believe that it has had even less cryptanalysis.

It would also be a good idea to look into replacing ISAAC with one of the eSTREAM ciphers

Filed #10047.

if it's actually significantly faster than ISAAC

Hm, I don't remember it being slower.

test rand::bench::rand_isaac                         ... bench:       973 ns/iter (+/- 78) = 822 MB/s
test rand::bench::rand_isaac64                       ... bench:       481 ns/iter (+/- 38) = 1663 MB/s
test rand::bench::rand_mt19937                       ... bench:      1049 ns/iter (+/- 39) = 762 MB/s
test rand::bench::rand_mt19937_64                    ... bench:       715 ns/iter (+/- 72) = 1118 MB/s
test rand::bench::rand_shuffle_100                   ... bench:      2326 ns/iter (+/- 99)
test rand::bench::rand_std                           ... bench:       480 ns/iter (+/- 31) = 1666 MB/s
test rand::bench::rand_xorshift                      ... bench:       287 ns/iter (+/- 19) = 2787 MB/s

I'll close this unless someone really thinks that mirroring C++11 is important. (I imagine that SFMT and/or dSFMT will be much faster, but they are more complicated to implement, so not today.)