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

More improvements to std::rand #9695

Merged
merged 22 commits into from
Oct 9, 2013
Merged

More improvements to std::rand #9695

merged 22 commits into from
Oct 9, 2013

Conversation

huonw
Copy link
Member

@huonw huonw commented Oct 3, 2013

A pile of changes to std::rand:

  • Add the 64-bit variant of the ISAAC Rng. This also splits the Rng.next() -> u32 method into Rng.next_u32() -> u32 and Rng.next_u64() -> u64 to be able to actually take advantage of the wider numbers. They have default implementations in terms of each other. (This is ~2× faster than the 32 bit one for generating anything larger than a u32 on 64-bit computers.)
  • Add ReaderRng which just wraps a reader as an RNG, useful for /dev/urandom, /dev/random, /dev/hwrng, etc. This also adds the overrideable fill_bytes method to Rng, since readers can "generate" randomness more than just 8 bytes at a time.
  • Add an interface to /dev/urandom (and the windows API) that implements Rng (os::OSRng) so that it is a first-class randomness source. This means that experimenting with things like seeding hashmaps from it will be much easier. It deletes most of the C++ supporting the old form, except for thin wrappers around the Windows API; I don't have access to a windows with Rust other than the try branch. ( Note: on unices, this means that OSRng requires the runtime, so it's not possible to use it to seed the scheduler RNG; I've replaced it with direct libc calls for reading from /dev/urandom.)
  • Add the "blessed" StdRng which means users who just want a random number generator don't need to worry about the implementation details (which will make changing the underlying implementation from Isaac to something else will be easier, if this every happen). This actually changes between the 32 and 64-bit variants of Isaac depending on the platform at the moment.
  • Add a SeedableRng trait for random number generators that can be explicitly seeded,
  • Add the ReseedingRng wrapper for reseeding a RNG after a certain amount of randomness is emitted. (The method for reseeding is controlled via the Reseeder trait from the same module)
  • changes to the task rng:
    • uses StdRng
    • it will reseed itself every 32KB, that is, after outputting 32KB of random data it will read new data from the OS (via OSRng)
  • Implements Rand for char, and makes the f32 and f64 instances more reasonable (and more similar to most other languages I've looked at).
  • Documentation, examples and tests

}
#[cfg(unix)]
#[fixed_stack_segment] #[inline(never)]
fn new_sched_rng() -> XorShiftRng {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native reading from /dev/urandom. (Bringing this to particular attention since it's going outside of Rust's safety & is fairly core to the runtime.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a bit of a shame that you can't use an explicit seed from an existing source that it's the OS, but I'm not sure that there's really a great way to avoid this.

@lilyball
Copy link
Contributor

lilyball commented Oct 3, 2013

Just from reading the PR description I like most of it. I'm not so sure about RUST_SEED though, although I understand that used to be a feature of the old rand. It just seems very surprising to me that a user could have control over my RNGs like that (e.g. if my program is a game, a user could exploit this to try and make it predictable in order to cheat, by replaying the game with the same seed). Seems to me like something that should be opt-in.

@thestinger
Copy link
Contributor

@kballard: the user has full control over all the memory in the process already, so I don't really see that as a problem

@emberian
Copy link
Member

emberian commented Oct 3, 2013

@emberian
Copy link
Member

emberian commented Oct 3, 2013

I agree with @kballard, but am not sure how it'd be implemented.

@alexcrichton
Copy link
Member

cc #4958, the initial implementation of a re-seeding IsaacRng instance (I believe that this was lost when everything moved to rust).

@@ -642,6 +636,62 @@ class raw_thread: public rust_thread {

#endif

#if defined(__WIN32__)
void
win32_require(LPCTSTR fn, BOOL ok) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From looking at these functions, there doesn't seem to be a lot of reasons why they're implemented in C++ rather than rust.

Of these four:

  • win32_require - this looks like just a fancy wrapper around FormatMessage. I think that the FormatMessage may actually already be implemented by strerror buries in the libc module, so this function can probably get removed.
  • rust_win32_rand_acquire - This looks like it may be the most difficult to put into rust. The constants PROV_RSA_FULL and the related CRYPT_* ones would be a pain to have to re-define in rust. This function seems like it could stick around in C++ (with a comment explaining why it's not in rust).
  • rust_win32_rand_gen - This seems like it could easily be put into rust.
  • rust_win32_rand_release - This also seems like it could easily be put into rust.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said on IRC, this appears to be slightly more difficult than it seems since (as far as I can tell from the MSDN docs), FormatMessage & GetLastError are different to errno & strerror. I don't have a windows computer to be able to test/iterate on this effectively, so I'm punting this to a later PR and/or filing a bug.

@alexcrichton
Copy link
Member

My name is Alex and I approve this message. (r=me with brson's comments addressed).

huonw added 22 commits October 9, 2013 22:22
This is 2x faster on 64-bit computers at generating anything larger
than 32-bits.

It has been verified against the canonical C implementation from the
website of the creator of ISAAC64.

Also, move `Rng.next` to `Rng.next_u32` and add `Rng.next_u64` to
take full advantage of the wider word width; otherwise Isaac64 will
always be squeezed down into a u32 wasting half the entropy and
offering no advantage over the 32-bit variant.
…Readers respectively.

The former reads from e.g. /dev/urandom, the latter just wraps any
std::rt::io::Reader into an interface that implements Rng.

This also adds Rng.fill_bytes for efficient implementations of the above
(reading 8 bytes at a time is inefficient when you can read 1000), and
removes the dependence on src/rt (i.e. rand_gen_seed) although this last
one requires implementing hand-seeding of the XorShiftRng used in the
scheduler on Linux/unixes, since OSRng relies on a scheduler existing to
be able to read from /dev/urandom.
This is implemented as a wrapper around another RNG. It is designed
to allow the actual implementation to be changed without changing
the external API (e.g. it currently uses a 64-bit generator on 64-
bit platforms, and a 32-bit one on 32-bit platforms; but one could
imagine that the IsaacRng may be deprecated later, and having this
ability to switch algorithms without having to update the points of
use is convenient.)

This is the recommended general use RNG.
…tes a certain number of bytes.

It is an "RNG adaptor" and so any RNG can be wrapped to have this behaviour.
This provides 2 methods: .reseed() and ::from_seed that modify and
create respecitively.

Implement this trait for the RNGs in the stdlib for which this makes
sense.
It now:
- can be explicitly seeded from user code (`seed_task_rng`) or from the
  environment (`RUST_SEED`, a positive integer)
- automatically reseeds itself from the OS *unless* it was seeded by
  either method above
- has more documentation
This lets the C++ code in the rt handle the (slightly) tricky parts of
random number generation: e.g. error detection/handling, and using the
values of the `#define`d options to the various functions.
The f32 generator now just uses a single u32, and the f64 uses a
single u64. This will make both significantly faster, especially
on 64-bit platforms.
This much better handled by directly calling out to `OSRng` where
appropriate.
bors added a commit that referenced this pull request Oct 9, 2013
A pile of changes to `std::rand`:

- Add the 64-bit variant of the ISAAC Rng. This also splits the `Rng.next() -> u32` method into `Rng.next_u32() -> u32` and `Rng.next_u64() -> u64` to be able to actually take advantage of the wider numbers. They have default implementations in terms of each other. (This is ~2× faster than the 32 bit one for generating anything larger than a `u32` on 64-bit computers.)
- Add `ReaderRng` which just wraps a reader as an RNG, useful for `/dev/urandom`, `/dev/random`, `/dev/hwrng`, etc. This also adds the overrideable `fill_bytes` method to `Rng`, since readers can "generate" randomness more than just 8 bytes at a time.
- Add an interface to `/dev/urandom` (and the windows API) that implements `Rng` (`os::OSRng`) so that it is a first-class randomness source. This means that experimenting with things like seeding hashmaps from it will be much easier. It deletes most of the C++ supporting the old form, except for thin wrappers around the Windows API; I don't have access to a windows with Rust other than the try branch. ( **Note:** on unices, this means that `OSRng` requires the runtime, so it's not possible to use it to seed the scheduler RNG; I've replaced it with direct libc calls for reading from `/dev/urandom`.)
- Add the "blessed" `StdRng` which means users who just want a random number generator don't need to worry about the implementation details (which will make changing the underlying implementation from Isaac to something else will be easier, if this every happen). This actually changes between the 32 and 64-bit variants of Isaac depending on the platform at the moment.
- Add a `SeedableRng` trait for random number generators that can be explicitly seeded, 
- Add the `ReseedingRng` wrapper for reseeding a RNG after a certain amount of randomness is emitted. (The method for reseeding is controlled via the `Reseeder` trait from the same module)
- changes to the task rng: 
 - uses `StdRng`
 - it will reseed itself every 32KB, that is, after outputting 32KB of random data it will read new data from the OS (via `OSRng`)
- Implements `Rand` for `char`, and makes the `f32` and `f64` instances more reasonable (and more similar to most other languages I've looked at).
- Documentation, examples and tests
@bors bors closed this Oct 9, 2013
@bors bors merged commit e678435 into rust-lang:master Oct 9, 2013
@huonw huonw deleted the rand2 branch October 9, 2013 13:20
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 31, 2023
[`unused_async`]: don't lint if paths reference async fn without immediate call

Fixes rust-lang#9695
Fixes rust-lang#9359

Clippy shouldn't lint unused `async` if there are paths referencing them if that path isn't the receiver of a function call, because that means that the function might be passed to some other function:
```rs
async fn f() {} // No await statements, so unused at this point

fn requires_fn_future<F: Future<Output = ()>>(_: fn() -> F) {}
requires_fn_future(f); // `f`'s asyncness is actually not unused.
```
(This isn't limited to just passing the function as a parameter to another function, it could also first be stored in a variable and later passed to another function as an argument)

This requires delaying the linting until post-crate and collecting path references to local async functions along the way.

changelog: [`unused_async`]: don't lint if paths reference async fn that require asyncness
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 this pull request may close these issues.

7 participants