Implement Random for array

[sorairolake]

Implement Random for [T; N], where Random is implemented for T.

rust-lang/libs-team#393 (comment) states:

The trait Random will initially be implemented for all iN and uN integer types, isize/usize, and bool, as well as arrays** and tuples of such values.

This PR is also based on #130703 (comment).

Tracking issue: #130703

[rustbot]

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[sorairolake]

r? @joboet

[joboet]

I think it would be better to implement this with a dedicated method on the Random trait, or at least adding a specialization for the integer primitives. Otherwise generating e.g. a [u8; _] will call into the system for each element instead of just once to fill the entire buffer, which is very wasteful and slow.

[the8472]

Otherwise generating e.g. a [u8; _] will call into the system for each element instead of just once to fill the entire buffer, which is very wasteful and slow.

That doesn't necessarily need specialization. We could add an internal BufferedRandomSource akin to a BufReader and then discard it at the end of the array filling.
We only want the OS source to be ~unbuffered to avoid fork/VM cloning issues. But if you're cloning a VM in the middle of array-filling you'll already end up with some duplicated "random" data anyway, so this should be fine I think.

[abgros]

Calling random() for every element in the array is extremely wasteful because as @joboet said it does a separate system call for each element. I benchmarked the following functions using criterion:

fn random_array() -> [u8; 1_000] {
	array::from_fn(|_| u8::random(&mut DefaultRandomSource))
}

fn random_array2() -> [u8; 1_000] {
	let mut arr = [0; 1_000];
	DefaultRandomSource.fill_bytes(&mut arr);
	arr
}

Calling fill_bytes() a single time is over 50x faster.

random_array            time:   [42.631 µs 43.067 µs 43.875 µs]
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

random_array2           time:   [617.27 ns 626.50 ns 641.50 ns]
Found 11 outliers among 100 measurements (11.00%)
  6 (6.00%) high mild
  5 (5.00%) high severe

[abgros]

Here's a version that uses fill_bytes().

Suggested change

	let mut buf = [const { MaybeUninit::uninit() }; N];
	for elem in &mut buf {
	elem.write(T::random(source));
	}
	// SAFETY: all elements of the array were initialized.
	unsafe { mem::transmute_copy(&buf) }
	let mut buf = [const { MaybeUninit::<T>::uninit() }; N];
	// SAFETY: this initializes every element in the buffer with random bytes.
	unsafe {
	let slice = core::slice::from_raw_parts_mut(&raw mut buf as *mut u8, N * mem::size_of::<T>());
	source.fill_bytes(slice);
	mem::transmute_copy(&buf)
	}

[the8472]

This is unsound for generic Ts. There's no guarantee that a Type's random impl shovels those bytes into their memory representation. A trivial example would be NonZero<u8> which could implement Random via rejection sampling. Setting it to 0 would be unsound.

[abgros]

@the8472 I did notice that, so we would need to restrict this to types that can hold arbitrary bytes (which currently includes every type implementing Random except bool). Otherwise, use a slower custom implementation with rejection sampling or modulo.

[the8472]

Specializations should be hidden behind a private trait. https://std-dev-guide.rust-lang.org/policy/specialization.html

Alternatively you can avoid specialization by implementing a buffering random source wrapper as mentioned in #136732 (comment)

[joboet]

I too would prefer a buffering implementation (in the generic case, the specialization for primitives makes sense to me) – but you can leave that for another PR.

[joboet]

This is unsound: you create a &mut [u8] and pass it to user code – but nothing guarantees that the user code won't try to read the slice, which would be undefined behaviour. My recommendation would be to use MaybeUninit::zeroed to create buf.

[dhardy]

Ideally this needs a trait bound like zerocopy::FromBytes or bytemuck::Pod, but we don't have that in std, so... the macro should be renamed to unsafe_impl_... since safety depends on the macro caller?

[dhardy]

This uses specialization to do what we couldn't do with rand::Fill, which is great except:

Any non-default impls of Random will produce different values when sampled. This makes the introduction of new non-default impls potentially a breaking change, depending on how std wants to handle reproducibility guarantees for random generators.

(My view is still that random-value generation should be left to external crates like rand, partly to avoid this reproducibility-guarantee issue.)

[joboet]

Sorry for not replying here for so long. This still exhibits the unsoundness I mentioned, and in any case is probably blocked on T-libs-api figuring out how they want to proceed with the random feature...

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[joshtriplett]

We discussed this in today's @rust-lang/libs-api meeting.

This PR is based on the previous Random trait; we've rewritten to use a Distribution<T> trait that's closer to what rand uses.

But also, this uses specialization, which we'd prefer to avoid for this (and its current implementation is unsound).

@Amanieu proposed, and @the8472 and I agreed, that we should instead have a method sample_slice with a default implementation that's element-by-element, and the standard library can override that implementation to do the faster thing.

In order for that method to be performant, it would need to use something like BorrowedBuf/BorrowedCursor, in order to work with an uninitialized array. That, in turn, would require making BorrowedBuf/BorrowedCursor generic to work on something other than u8. In the meeting, we were broadly in favor of doing that, now that we have a use case in the standard library.

As a rough sketch of how this would look after we have the BorrowedBuf rework:

impl Distribution<[T; N]> {
    fn sample(&self, source: &mut (impl RandomSource + ?Sized)) -> T {
        // Set up an uninitialized BorrowedBuf
        T::sample_slice(buf.unfilled(), source);
        out
    }
    
    fn sample_slice(buf: BorrowedCursor<'_, [T; N]>, source: &mut (impl RandomSource + ?Sized)) {
        // Flatten into a cursor over `T`, and call `T::sample_slice`
        // This allows nested arrays like `[[u32; 8]; 8]` to Just Work.
    }
}

[bors]

☔ The latest upstream changes (presumably #135634) made this pull request unmergeable. Please resolve the merge conflicts.

[sorairolake]

I'm closing this pull request because it's a hassle to support the new Random trait.

[Amanieu]

Note that we are close to landing support for arbitrary types in BufReader in #149749, which would allow implementing the proposal in #136732 (comment).