Skip to content
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

Optimise Rng::gen for arrays #1282

Closed
dhardy opened this issue Feb 6, 2023 · 3 comments · Fixed by #1479
Closed

Optimise Rng::gen for arrays #1282

dhardy opened this issue Feb 6, 2023 · 3 comments · Fixed by #1479

Comments

@dhardy
Copy link
Member

dhardy commented Feb 6, 2023

Summary

Currently, rng.gen::<[u8; 4]>() will invoke the RNG four times. In contrast, rng.fill(&mut array) would do so once. Ideally we should optimise the former to do the same as the latter.

Details

Fill only supports arrays and slices over a few types:

  • bool, char, f32, f64: these invoke rng.gen() for each element (wasteful for bool, but required for the rest)
  • u8: straight byte copy
  • Other integer types: byte copy + to_le (for portability)

rng.gen() does not support slices (unsized) but supports arrays over anything supported by the Standard distribution.

We could:

  1. Keep things as they are: each method has its advantages, but it's confusing that rng.fill(&mut array) is not equivalent to array = rng.gen().
  2. Aim to use specialization to use the Fill method where possible. But who knows when Rust will support this.
  3. Limit rng.gen() to types supported by Fill (API breaking change). We can try to support enough types to satisfy most users, but cannot support user-defined types.

If we choose (3), we could extend this such that both methods support all generable element types once Specialization is stable.

Motivation

  • Intuitive behaviour: it's not intuitive that one method is potentially much faster than the other and generates different results
  • Performance
@TheIronBorn
Copy link
Collaborator

If we choose option 1 we should document that the methods will produce different results

@dhardy
Copy link
Member Author

dhardy commented Feb 7, 2023

This documents speed (but not value differences): https://docs.rs/rand/latest/rand/trait.Rng.html#arrays-and-tuples

Yes, looks like docs should be better.

@dhardy dhardy mentioned this issue Feb 20, 2023
23 tasks
@JustusFluegel
Copy link
Contributor

I don't know if my opinion is in anyway relevant here (not really involved in the project) but if it is I probably agree that 1 is the best option, keeping the flexibility that .gen<T> has, and if 2 is ever possible maybe do that then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants