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

Separate rand_thread crate? #716

Closed
dhardy opened this issue Jan 28, 2019 · 8 comments
Closed

Separate rand_thread crate? #716

dhardy opened this issue Jan 28, 2019 · 8 comments
Labels
X-discussion Type: discussion

Comments

@dhardy
Copy link
Member

dhardy commented Jan 28, 2019

Synopsis: create a new rand_thread crate to house ThreadRng and make rand depend on that.

Motivation:

Reasons not to do this:

  • this crate would only have 1-2 less dependencies than randrand_pcg (which might be removed from rand anyway) and possibly autocfg
  • this is yet another crate for users of Rand as a whole
  • likely someone would want some extra functionality, such as generating uniform integers within a given range, and thus have to add a dependency on rand anyway (or roll their own code which is easy to get wrong with bias and off-by-one errors and would miss our various specialisations and optimisations)

To be clear: my personal opinion is that we don't need this.

@vks
Copy link
Collaborator

vks commented Jan 28, 2019

I'm also not convinced by the arguments for this separation. A better way forward might be to reduce the code in the rand crate, which is currently planned anyway (see #715).

@newpavlov
Copy link
Member

I believe ThreadRng should go into it's own crate. It's a perfectly isolated piece of functionality and there are users who use only it, without any additional functionality provided by rand. Also in my opinion it will be inconsistent and a bit surprising to keep every RNG in its own crate, but not ThreadRng.

@dhardy
Copy link
Member Author

dhardy commented Jan 28, 2019

Two "votes" against this, one for. I would appreciate more users commenting on this.

@nagisa
Copy link
Contributor

nagisa commented Jan 28, 2019 via email

@newpavlov
Copy link
Member

newpavlov commented Jan 28, 2019

threadrng is actually a wrapper and that it is more of a building block

I think you misunderstand something, essentially ThreadRng is a "default" RNG. If applications don't have strict security requirements like "use only OS RNG", or performance requirements (so very-very fast non-cryptographic RNGs will be a better fit) like games or simulations, they usually will use ThreadRng as the only source of non-deterministic random data. (via thread_rng or random)

@burdges
Copy link
Contributor

burdges commented Jan 28, 2019

Another alternative might be:

  • migrate ReseedingRng from rand to either rand_core or more likely rand_os, and
  • migrate ThreadRng to rand_os.

I've forgotten any discussion around ReseedingRng's placement, but if it creates no problems then the only cost here would be rand_os users needing to compile rand_chacha or AESNI whatever, but the linker should avoid that actually appearing in code that never touches ThreadRng.

At this point, we'd have roughly this breakdown:

  • rand_core - Only used directly when writing CSPRNGs.
  • rand_os - Bytes or word oriented system randomness users, either with or without the per-thread ReseedingRng optimization. All OS abstraction handled here unless getrandom happens.
  • rand - Anything higher level around sampling and distributions.

@dhardy
Copy link
Member Author

dhardy commented Jan 29, 2019

Interesting @burdges. It might be worth coming back to this idea once we have a fully-functional getrandom for those who just want entropy.

@dhardy
Copy link
Member Author

dhardy commented Apr 19, 2019

Sounds like we're mostly in agreement to keep thread_rng within the main Rand crate. We can revisit given fresh rationale, but I see no reason to keep this open now.

@dhardy dhardy closed this as completed Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-discussion Type: discussion
Projects
None yet
Development

No branches or pull requests

5 participants