Reduce number of syscalls in `rand` #53725

tbu- · 2018-08-26T20:31:44Z

This skips the initial zero-length getrandom call and
directly hands the user buffer to the operating system, saving one
getrandom syscall.

In case that it is statically known that the OS doesn't support `getrandom` (non-Linux) or becomes clear at runtime that `getrandom` isn't available (`ENOSYS`), the opened fd ("/dev/urandom") isn't closed after the function, so that future calls can reuse it. This saves repeated `open`/`close` system calls at the cost of one permanently open fd. Additionally, this skips the initial zero-length `getrandom` call and directly hands the user buffer to the operating system, saving one `getrandom` syscall.

rust-highfive · 2018-08-26T20:31:54Z

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

Mark-Simulacrum · 2018-08-26T20:39:03Z

r? @alexcrichton

tbu- · 2018-08-27T05:02:28Z

Program:

use std::collections::HashMap;

fn main() {
    let mut m = HashMap::new();
    m.insert("abc", "def");
}

Without this PR:

[…]
sigaltstack(NULL, {ss_sp=NULL, ss_flags=SS_DISABLE, ss_size=0}) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f072df95000
sigaltstack({ss_sp=0x7f072df95000, ss_flags=0, ss_size=8192}, NULL) = 0
getrandom("", 0, GRND_NONBLOCK)         = 0
getrandom("\x26\x53\x69\xb8\x75\xd4\xe3\x9a\x32\x11\x10\x58\xa7\x8a\x47\x4b", 16, GRND_NONBLOCK) = 16
sigaltstack({ss_sp=NULL, ss_flags=SS_DISABLE, ss_size=8192}, NULL) = 0
munmap(0x7f072df95000, 8192)            = 0
exit_group(0)                           = ?
+++ exited with 0 +++

With this PR:

[…]
sigaltstack(NULL, {ss_sp=NULL, ss_flags=SS_DISABLE, ss_size=0}) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa8d3862000
sigaltstack({ss_sp=0x7fa8d3862000, ss_flags=0, ss_size=8192}, NULL) = 0
getrandom("\x05\x08\x66\x91\x2a\xf0\x05\xf9\x44\x34\x7c\x27\xf3\xfe\x06\x4c", 16, GRND_NONBLOCK) = 16
sigaltstack({ss_sp=NULL, ss_flags=SS_DISABLE, ss_size=8192}, NULL) = 0
munmap(0x7fa8d3862000, 8192)            = 0
exit_group(0)                           = ?
+++ exited with 0 +++

The initial getrandom("", 0, GRND_NONBLOCK) = 0 gets skipped.

alexcrichton · 2018-08-27T17:12:00Z

Thanks for the PR! The changes to getrandom seem good to me, but I'm not super comfortable with effectively leaking a permanent file descriptor. This isn't really a performance sensitive piece of code, right? (It's only used sparingly on each thread I think?) In that sense is it really necessary to get the minor boost from leaving it open rather than closing and reopening?

tbu- · 2018-08-29T12:21:17Z

Done.

alexcrichton · 2018-08-29T16:07:36Z

@bors: r+

bors · 2018-08-29T16:07:39Z

📌 Commit 6d47737 has been approved by alexcrichton

…richton Reduce number of syscalls in `rand` This skips the initial zero-length `getrandom` call and directly hands the user buffer to the operating system, saving one `getrandom` syscall.

pietroalbini · 2018-08-29T20:13:07Z

@bors r-

[00:25:39] error: unused import: `libc`
[00:25:39]   --> libstd/sys/unix/rand.rs:32:9
[00:25:39]    |
[00:25:39] 32 |     use libc;
[00:25:39]    |         ^^^^
[00:25:39]    |
[00:25:39]    = note: `-D unused-imports` implied by `-D warnings`
[00:25:39] 
[00:25:39] error: unused import: `sys::os::errno`
[00:25:39]   --> libstd/sys/unix/rand.rs:33:9
[00:25:39]    |
[00:25:39] 33 |     use sys::os::errno;
[00:25:39]    |         ^^^^^^^^^^^^^^
[00:25:39] 
[00:25:43] error: aborting due to 2 previous errors

goffrie · 2018-08-31T22:24:32Z

src/libstd/sys/unix/rand.rs

        let mut read = 0;
        while read < v.len() {
            let result = getrandom(&mut v[read..]);
            if result == -1 {
                let err = errno() as libc::c_int;
                if err == libc::EINTR {
                    continue;
+                } else if err == libc::ENOSYS {
+                    GETRANDOM_UNAVAILABLE.store(true, Ordering::Relaxed);


Would this not loop infinitely in the ENOSYS case?

alexcrichton · 2018-08-31T22:26:19Z

@bors: r-

oh dear, good eyes @goffrie!

alexcrichton · 2018-09-02T21:24:13Z

@bors: r+

bors · 2018-09-02T21:24:14Z

📌 Commit b95c491 has been approved by alexcrichton

bors · 2018-09-02T21:24:25Z

⌛ Testing commit b95c491 with merge 9395f0a...

Reduce number of syscalls in `rand` This skips the initial zero-length `getrandom` call and directly hands the user buffer to the operating system, saving one `getrandom` syscall.

bors · 2018-09-02T23:46:13Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 9395f0a to master...

rust-highfive assigned Mark-Simulacrum Aug 26, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 26, 2018

rust-highfive assigned alexcrichton and unassigned Mark-Simulacrum Aug 26, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2018

pietroalbini mentioned this pull request Aug 29, 2018

Rollup of 21 pull requests #53798

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2018

Don't leak the file descriptor in rand

d6d280b

tbu- force-pushed the pr_getrandom_syscalls branch from 6d47737 to d6d280b Compare August 29, 2018 20:30

goffrie reviewed Aug 31, 2018

View reviewed changes

Fix an endless loop when getrandom is not available

b95c491

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2018

bors merged commit b95c491 into rust-lang:master Sep 2, 2018

tbu- mentioned this pull request Mar 6, 2024

Less syscalls for the copy_file_range probe #122079

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce number of syscalls in `rand` #53725

Reduce number of syscalls in `rand` #53725

tbu- commented Aug 26, 2018 •

edited

Loading

rust-highfive commented Aug 26, 2018

Mark-Simulacrum commented Aug 26, 2018

tbu- commented Aug 27, 2018

alexcrichton commented Aug 27, 2018

tbu- commented Aug 29, 2018

alexcrichton commented Aug 29, 2018

bors commented Aug 29, 2018

pietroalbini commented Aug 29, 2018

goffrie Aug 31, 2018

tbu- Sep 1, 2018

alexcrichton commented Aug 31, 2018

alexcrichton commented Sep 2, 2018

bors commented Sep 2, 2018

bors commented Sep 2, 2018

bors commented Sep 2, 2018

Reduce number of syscalls in rand #53725

Reduce number of syscalls in rand #53725

Conversation

tbu- commented Aug 26, 2018 • edited Loading

rust-highfive commented Aug 26, 2018

Mark-Simulacrum commented Aug 26, 2018

tbu- commented Aug 27, 2018

alexcrichton commented Aug 27, 2018

tbu- commented Aug 29, 2018

alexcrichton commented Aug 29, 2018

bors commented Aug 29, 2018

pietroalbini commented Aug 29, 2018

goffrie Aug 31, 2018

Choose a reason for hiding this comment

tbu- Sep 1, 2018

Choose a reason for hiding this comment

alexcrichton commented Aug 31, 2018

alexcrichton commented Sep 2, 2018

bors commented Sep 2, 2018

bors commented Sep 2, 2018

bors commented Sep 2, 2018

Reduce number of syscalls in `rand` #53725

Reduce number of syscalls in `rand` #53725

tbu- commented Aug 26, 2018 •

edited

Loading