Add HC-128 RNG #74
Add HC-128 RNG #74
Conversation
That is kind of the goal here :-)
|
I should add that the output of this RNG matches the reference implementation. I have tested the first 65536 result with a couple of random seeds. |
|
This PR could probably be done directly against upstream? (Possibly it would leave some smaller details to fix later, but I think this would be easier than merging here then upstream.) |
|
Now that I see the direction you want to go with upstream, I will do that. |
| } | ||
|
|
||
| impl Hc128Rng { | ||
| pub fn init(seed: &[u32]) -> Hc128Rng { |
There was a problem hiding this comment.
I thought we were trying to use fixed-length byte arrays instead of slices of u32? Of course it may be easier to convert from byte arrays in from_rng and from_seed but why is this variable length? (It would at least be nice to have documentation of what is expected to be passed to init.)
There was a problem hiding this comment.
I use u32's here because that is what the algorithm uses, and convert in from_seed and from_rng. It was fixed-length before using le::convert_slice_32.
But you are right, I should add documentation.
One thing I noticed about le::convert_slice_32 is that it assumes seed is aligned as [u32], while it only is an [u8]. Something to fix or give a big warning.
There was a problem hiding this comment.
Ok. Yes, it's quite possible I messed up the le functions.
| let (p, q) = self.t.split_at_mut(512); | ||
| // FIXME: it would be great if we the bounds checks here could be | ||
| // optimized out, and we would not need unsafe. | ||
| // This improves performance by about 7%. |
There was a problem hiding this comment.
I wonder if this is because p and q are slices, hence the compiler doesn't know the length? You could try using array_ref instead maybe, though it's not much of a solution. Or use t directly.
There was a problem hiding this comment.
I have tried something like 20 variants. In some of them p and q where arrays, and t did not exist. If have tried macro's instead of functions, and various manual bounds checks in update. But no luck convincing LLVM they are unnecessary yet 😞 . I can't be much more precise, because I did not save or document all tries. Maybe expecting it to optimize out 7 bounds checks is to much to ask. It already reduces that down to 1 on average (but there is not any specific one, that is the average over the whole update function).
|
I just created rust-random#209; hopefully this can land on top of that (maybe add |
|
Great! That should make it much simpler (for me). |
|
Might as well merge this I guess. |
|
I should have said I converted |
|
Ah, ok. Probably the |
This implements the HC-128 cryptographically secure RNG. See #53.
It is similar to ISAAC, but proven to be secure and (in many cases) faster.
I have been sitting on this code for a few weeks hoping that I could reduce the amount of unsafe code and some duplication. But that didn't work out yet, so I am just making a PR as it is.