Update rand crate to 0.5

[oherrala]

This PR updates rand crate to version 0.5.

Minimum Rust version (in .travis.yml) pushed to 1.22.0.

[BurntSushi]

Thanks for putting this together!

If rand's minimum version is 1.22, then that should be quickcheck's version. The contract isn't 1.21, it's 1.22, even if rand happens to compile on 1.21 today.

Right now this PR also comes with I think quite a horrible performance penalty especially for Strings:

Uh, that does indeed look pretty bad. We should figure out what's going on with strings before merging this.

[oherrala]

I'm not really comfortable with profiling Rust code on macOS yet, but running the shrink_string benchmark using Valgrind's callgrind I think I see a lot of memmove() from VecShrinker.

Looking at the code, there's Arbitrary for String's

quickcheck/src/arbitrary.rs

Lines 572 to 576 in 2143dc8

	fn shrink(&self) -> Box<Iterator<Item=String>> {
	// Shrink a string by shrinking a vector of its characters.
	let chars: Vec<char> = self.chars().collect();
	Box::new(chars.shrink().map(|x| x.into_iter().collect::<String>()))
	}

which could produce quite a lot of VecShrink'ng.

How this is related to rand crate. I don't really know yet.

[pitdicker]

I investigated the performance differences (hoping we didn't mess up too badly in Rand). I couldn't even find back the RNG when profiling!

The benchmark already notes "Use a deterministic generator to benchmark on the same data", and this is the trouble. The number of shrinkages (if I understand quickcheck right) depends on the range generated by the RNG. Any change and the benchmarks can become a magnitude faster or slower.

We did not change ISAAC, but did change the algorithm used in gen_range. It is easy to verify this is the problem: if you change the bechmarks to initialize the RNG with some other value, like IsaacRng::new_from_u64(3), the results will be very different:

 shrink_string_1_tuple  64,943          12,420                 -52,523  -80.88%   x 5.23 
 shrink_string_2_tuple  90,180          24,018                 -66,162  -73.37%   x 3.75 
 shrink_string_3_tuple  190,919         64,927                -125,992  -65.99%   x 2.94 
 shrink_string_4_tuple  229,406         966,372                736,966  321.25%   x 0.24 
 shrink_string_5_tuple  296,096         1,094,523              798,427  269.65%   x 0.27 
 shrink_string_6_tuple  417,239         1,213,660              796,421  190.88%   x 0.34 
 shrink_string_7_tuple  1,029,913       1,280,004              250,091   24.28%   x 0.80 
 shrink_string_8_tuple  1,369,208       1,738,581              369,373   26.98%   x 0.79 

Now one is 5x faster, another 4x slower.

[pitdicker]

This PR looks good to me!

Actually, I made exactly the same changes, and then found out there was an open PR 😄.

One thing that may be a good idea:
We may deprecate ISAAC someday. Also we plan to deprecate some compatibility module re-exports someday. If you change the benchmark to use rand::prng::hc128::Hc128Rng it does not have to change again in the future.

[oherrala]

Sorry for taking so long to respond.

I agree with @pitdicker that randomness can affect the benchmark times. I also changed benchmarks to use Hc128 as @pitdicker suggested.

My feeling right now is that since benchmarks depend on how rand crate does things, it's not feasible to benchmark this PR and master branch.

[BurntSushi]

Thanks!