std: Implement thread::sleep
#23330
std: Implement thread::sleep
#23330
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
alexcrichton
force-pushed
the
thread-sleep
branch
from
806da80
to
466cd25
Compare
|
@sfackler all very good points on alexcrichton@806da80, thanks for the sharp eye! They should all be taken care of now |
alexcrichton
force-pushed
the
thread-sleep
branch
from
466cd25
to
560ba47
Compare
| if pthread_getattr_np(pthread_self(), &mut attr) != 0 { | ||
| panic!("failed to get thread attributes"); | ||
| } | ||
| eq!(pthread_getattr_np(pthread_self(), &mut attr), 0); |
There was a problem hiding this comment.
This reduces clarity of the error and makes harder to find the cause IMO. Also, if debug_assert_eq!, does what I think it does, this code behave incorrectly and most likely will SIGSEGV/SIGILL in light of #22642 and possibly many other cases we don’t account for yet.
I would defer this change until somebody gets rid of pthread usage here.
There was a problem hiding this comment.
This reduces clarity of the error and makes harder to find the cause IMO.
Could you expand on this some more? I expect this panic message to be more informative as it includes the line number, expression, and return value if it's not zero.
Also, if debug_assert_eq!, does what I think it does, this code behave incorrectly and most likely will SIGSEGV/SIGILL
That's a good point, I'll switch back to assert_eq!
I would defer this change until somebody gets rid of pthread usage here.
I don't quite follow, do you think we can get rid of this usage?
alexcrichton
force-pushed
the
thread-sleep
branch
3 times, most recently
from
67df986
to
a4af022
Compare
| } | ||
| } | ||
|
|
||
| pub fn sleep(dur: Duration) { |
This module had become a #[cfg] jungle, try to bring at least a small semblance of order to it!
alexcrichton
force-pushed
the
thread-sleep
branch
from
a4af022
to
cb3fb3c
Compare
This function is the current replacement for `std::old_io::timer` which will soon be deprecated. This function is unstable and has its own feature gate as it does not yet have an RFC nor has it existed for very long.
alexcrichton
force-pushed
the
thread-sleep
branch
from
cb3fb3c
to
04cf534
Compare
| request: *const libc::timespec, | ||
| remain: *mut libc::timespec) -> libc::c_int; | ||
| } | ||
| clock_nanosleep(libc::CLOCK_MONOTONIC, 0, ts, ts) |
There was a problem hiding this comment.
There is no point using clock_nanosleep if you don't use an absolute time. I'm gonna have to post the manpage again:
The fact that nanosleep() sleeps for a relative interval can be
problematic if the call is repeatedly restarted after being inter‐
rupted by signals, since the time between the interruptions and
restarts of the call will lead to drift in the time when the sleep
finally completes. This problem can be avoided by using
clock_nanosleep(2) with an absolute time value.
Right now the code is more complicated than necessary with no benefit.
There was a problem hiding this comment.
The move to clock_nanosleep was motivated by dealing with system clock drift instead of restarts because of EINTR. That form of slop is likely quite small and is already dealt with in the spec of the sleep function in std::thread (it is not precise).
There was a problem hiding this comment.
It's neither a cross-platform fix nor does it go the full way to make the implementation rock-solid on one platform. It's useless code bloat.
Funny how you always refuse to use a memcpy/memset wrapper with measurable performance impact in io code. But this haphazard fix is somehow worth two extra platform specific functions that will only be used once.
|
@bors: force The other PR is just a doc rollup |
This function is the current replacement for `std::old_io::timer` which will soon be deprecated. This function is unstable and has its own feature gate as it does not yet have an RFC nor has it existed for very long.
|
Thanks. :) |
This function is the current replacement for
std::old_io::timerwhich willsoon be deprecated. This function is unstable and has its own feature gate as it
does not yet have an RFC nor has it existed for very long.