- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 456
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
Switch StdRng and thread_rng to HC-128 #277
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of little tweaks needed, but I approve, despite the apparent performance issue on ARMv7. Feedback in the linked thread was positive
src/lib.rs
Outdated
#[derive(Clone, Debug)] | ||
pub struct StdRng(IsaacWordRng); | ||
pub struct StdRng(Hc128Rng); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this leave IsaacWordRng
unused? We might as well remove it. Ah, you didn't update the Seed
type...
/// Like [`StdRng`], `ThreadRng` is a cryptographically secure PRNG. The current | ||
/// algorithm used is [HC-128], which is an array-based PRNG that trades memory | ||
/// usage for better performance. This makes it similar to ISAAC, the algorithm | ||
/// used in `ThreadRng` before rand 0.5. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this means it can implement CryptoRng
now.
I was too busy with the docs that I forget to check the rebase 😕. I will update the tests in a moment, but am getting some strange failures...
I would like to try a couple of benchmarks on ARM, but am not sure how to set it up. |
Wow, testing on my phone is super easy with |
Use of nautical terms reminds me that my phone is a Jolla, with native shell and GCC support... Rustup is also happy to install. Slow but managed to get some results at least:
Funny when Isaac64 beats Xorshift. |
Sounds like there is some work left to do on ARM... Travis seems to have given up, but because this passes on all other platforms I can't imagine it to fail. Ready to merge? |
I need to investigate those benchmarks further; some of them seemed to hang. Oh you still didn't implement |
Oh, Xorshift wins on u32 and u64. Tests sometimes hang but when they do give a result, the results do appear to be repeatable. Isaac still beats HC128 everywhere in performance, but not by too much.
|
Those benchmarks are very strange, |
Switch StdRng and thread_rng to HC-128
As discussed in dhardy#53.
I also changed
THREAD_RNG_RESEED_THRESHOLD
to 32 MiB, which seems a more reasonable number to me.