Bump MSRV to 1.36 #1011
Bump MSRV to 1.36 #1011
Conversation
|
We also can replace: #![cfg_attr(not(feature = "std"), no_std)]
#[cfg(all(feature = "alloc", not(feature = "std")))] extern crate alloc;with: #[cfg(feature = "alloc")]
extern crate alloc;
#[cfg(feature = "std")]
extern crate std; |
|
Technically this shouldn't affect some crates, e.g. |
|
Also see rust-secure-code/safety-dance#54. It seems like I broke some code on big endian platforms, let me check... |
There was a problem hiding this comment.
Lots of nice changes!
Since it wasn't linked, a large chunk of this PR is about these issues: rust-secure-code/safety-dance#54
Could you check the benchmarks please? Specifically relating to the int → bytes and bytes → int conversions (generators).
rand_core/src/impls.rs
Outdated
| pub fn next_u32_via_fill<R: RngCore + ?Sized>(rng: &mut R) -> u32 { | ||
| let mut buf = [0; 4]; | ||
| rng.fill_bytes(&mut buf); | ||
| u32::from_le_bytes(buf) | ||
| u32::from_ne_bytes(buf) | ||
| } |
There was a problem hiding this comment.
?
Indeed, the old version didn't byte-swap. But fill_bytes_via_next does byte-swap. Weird?
Generating bytes is ca. 8 % slower, which is not ideal but I think acceptable. Generating Before (master): After (this PR): |
|
Quick test on my side: 4-9% slower bytes generation from ChaCha; 23% slower from HC128. Other code paths (e.g. bytes from PCG and int outputs) appear unaffected. The default byte-length is 1024; I tested 128 and 12000 with basically identical results (form the latter): I'm not really happy about this performance loss. Do we have any better options? |
|
I think we have to change UPD: On a second look, the linked code is not correct (it will not fill "tail" bytes), but I think the general idea can be understood. |
|
@newpavlov Are you talking about |
|
Yes. |
|
@newpavlov With your suggestion, I get the same performance as before (without resorting to unsafe code), however, the results are not quite correct:
Do you know what is wrong? I don't see it. |
|
fn fill_via_u32(src: &[u32], dst: &mut [u8]) -> usize {
let mut src = src.iter();
let mut chunks = dst.chunks_exact_mut(4);
for (v, chunk) in (&mut src).zip(&mut chunks) {
chunk.copy_from_slice(&v.to_le_bytes());
}
let rem = chunks.into_remainder();
if rem.len() != 0 {
let v = src.next().unwrap();
rem.copy_from_slice(&v.to_le_bytes()[..rem.len()]);
dst.len()/4 + 1
} else {
dst.len()/4
}
}Unfortunately it adds a panic on the unwrap. We could use But honestly, I don't quite like the current approach and I think it's worth to experiment with one proposed here. |
I agree, being able to efficiently provide bitwise randomness seems more future proof for algorithms that optimize for the consumed entropy, and it makes generating
At this point, we can go back to using |
|
@dhardy If you think the performance of the safe code is not acceptable, we can always go back to the old |
|
@newpavlov If bit- or byte-level counters work better here (with reasonable perf. all round), I don't have a problem with that. Also I don't think changing behaviour in a breaking release before 1.0 is a big deal; as long as we document it we're doing better than many rand libs. Personally I still feel that allowing loss-less bit-level consumption is not very useful in terms of performance, and even byte-level consumption may not be overall. But I may be wrong. |
|
@dhardy |
I did that, they performance is comparable to before now: I think this can be merged now (maybe after squashing)? |
The necessary standard library functions were stabilized with Rust 1.34. Our MSRV is 1.36.
This is possible thanks to `alloc` being implied by `std` builds since Rust 1.36.
The results from master, using unsafe code: ``` gen_bytes_chacha12: 2,733,838 ns/iter (+/- 181,694) = 374 MB/s gen_bytes_chacha20: 4,339,602 ns/iter (+/- 237,793) = 235 MB/s gen_bytes_chacha8: 1,918,279 ns/iter (+/- 103,581) = 533 MB/s ``` The results of the new code using `chunks_exact_mut` (this commit): ``` gen_bytes_chacha12: 3,049,147 ns/iter (+/- 220,631) = 335 MB/s gen_bytes_chacha20: 4,645,772 ns/iter (+/- 269,261) = 220 MB/s gen_bytes_chacha8: 2,214,954 ns/iter (+/- 1,745,600) = 462 MB/s ``` The results of using `chunks_mut` (before this commit): ``` gen_bytes_chacha12: 3,492,109 ns/iter (+/- 164,638) = 293 MB/s gen_bytes_chacha20: 5,087,706 ns/iter (+/- 249,219) = 201 MB/s gen_bytes_chacha8: 2,700,197 ns/iter (+/- 524,148) = 379 MB/s ```
This allows us to get rid of some unsafe code. I did not test whether this affects performance however.
cc @Shnatsel