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

std: Redesign Duration, implementing RFC 1040 #24920

Merged
merged 1 commit into from
May 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#![feature(box_syntax)]
#![feature(collections)]
#![feature(core)]
#![feature(duration)]
#![feature(duration_span)]
#![feature(fs_canonicalize)]
#![feature(hash)]
#![feature(into_cow)]
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/metadata/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,7 @@ fn get_metadata_section(is_osx: bool, filename: &Path) -> Result<MetadataBlob, S
let dur = Duration::span(|| {
ret = Some(get_metadata_section_imp(is_osx, filename));
});
info!("reading {:?} => {}ms", filename.file_name().unwrap(),
dur.num_milliseconds());
info!("reading {:?} => {}", filename.file_name().unwrap(), dur);
return ret.unwrap();;
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/util/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ pub fn time<T, U, F>(do_it: bool, what: &str, u: U, f: F) -> T where
};
let rv = rv.unwrap();

println!("{}time: {}.{:03} \t{}", repeat(" ").take(old).collect::<String>(),
dur.num_seconds(), dur.num_milliseconds() % 1000, what);
println!("{}time: {} \t{}", repeat(" ").take(old).collect::<String>(),
dur, what);
DEPTH.with(|slot| slot.set(old));

rv
Expand Down
82 changes: 63 additions & 19 deletions src/libstd/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,20 @@ pub struct Condvar { inner: Box<StaticCondvar> }
/// # Examples
///
/// ```
/// # #![feature(std_misc)]
/// # #![feature(static_condvar)]
/// use std::sync::{StaticCondvar, CONDVAR_INIT};
///
/// static CVAR: StaticCondvar = CONDVAR_INIT;
/// ```
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub struct StaticCondvar {
inner: sys::Condvar,
mutex: AtomicUsize,
}

/// Constant initializer for a statically allocated condition variable.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub const CONDVAR_INIT: StaticCondvar = StaticCondvar {
inner: sys::CONDVAR_INIT,
Expand Down Expand Up @@ -161,6 +161,30 @@ impl Condvar {
}
}

/// Waits on this condition variable for a notification, timing out after a
/// specified duration.
///
/// The semantics of this function are equivalent to `wait()` except that
/// the thread will be blocked for roughly no longer than `dur`. This
/// method should not be used for precise timing due to anomalies such as
/// preemption or platform differences that may not cause the maximum
/// amount of time waited to be precisely `dur`.
///
/// The returned boolean is `false` only if the timeout is known
/// to have elapsed.
///
/// Like `wait`, the lock specified will be re-acquired when this function
/// returns, regardless of whether the timeout elapsed or not.
#[unstable(feature = "wait_timeout", reason = "waiting for Duration")]
pub fn wait_timeout<'a, T>(&self, guard: MutexGuard<'a, T>,
dur: Duration)
-> LockResult<(MutexGuard<'a, T>, bool)> {
unsafe {
let me: &'static Condvar = &*(self as *const _);
me.inner.wait_timeout(guard, dur)
}
}

/// Waits on this condition variable for a notification, timing out after a
/// specified duration.
///
Expand Down Expand Up @@ -214,7 +238,7 @@ impl StaticCondvar {
/// notification.
///
/// See `Condvar::wait`.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn wait<'a, T>(&'static self, guard: MutexGuard<'a, T>)
-> LockResult<MutexGuard<'a, T>> {
Expand All @@ -235,14 +259,27 @@ impl StaticCondvar {
/// specified duration.
///
/// See `Condvar::wait_timeout`.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn wait_timeout_ms<'a, T>(&'static self, guard: MutexGuard<'a, T>, ms: u32)
-> LockResult<(MutexGuard<'a, T>, bool)> {
self.wait_timeout(guard, Duration::from_millis(ms as u64))
}

/// Waits on this condition variable for a notification, timing out after a
/// specified duration.
///
/// See `Condvar::wait_timeout`.
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn wait_timeout<'a, T>(&'static self,
guard: MutexGuard<'a, T>,
timeout: Duration)
-> LockResult<(MutexGuard<'a, T>, bool)> {
let (poisoned, success) = unsafe {
let lock = mutex::guard_lock(&guard);
self.verify(lock);
let success = self.inner.wait_timeout(lock, Duration::milliseconds(ms as i64));
let success = self.inner.wait_timeout(lock, timeout);
(mutex::guard_poison(&guard).get(), success)
};
if poisoned {
Expand All @@ -259,15 +296,16 @@ impl StaticCondvar {
/// passed and the function returns `false`.
///
/// See `Condvar::wait_timeout_with`.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn wait_timeout_with<'a, T, F>(&'static self,
guard: MutexGuard<'a, T>,
dur: Duration,
mut f: F)
-> LockResult<(MutexGuard<'a, T>, bool)>
where F: FnMut(LockResult<&mut T>) -> bool {
// This could be made more efficient by pushing the implementation into sys::condvar
// This could be made more efficient by pushing the implementation into
// sys::condvar
let start = SteadyTime::now();
let mut guard_result: LockResult<MutexGuard<'a, T>> = Ok(guard);
while !f(guard_result
Expand All @@ -277,12 +315,15 @@ impl StaticCondvar {
let now = SteadyTime::now();
let consumed = &now - &start;
let guard = guard_result.unwrap_or_else(|e| e.into_inner());
let res = self.wait_timeout_ms(guard, (dur - consumed).num_milliseconds() as u32);
let (new_guard_result, no_timeout) = match res {
Ok((new_guard, no_timeout)) => (Ok(new_guard), no_timeout),
Err(err) => {
let (new_guard, no_timeout) = err.into_inner();
(Err(PoisonError::new(new_guard)), no_timeout)
let (new_guard_result, no_timeout) = if consumed > dur {
(Ok(guard), false)
} else {
match self.wait_timeout(guard, dur - consumed) {
Ok((new_guard, no_timeout)) => (Ok(new_guard), no_timeout),
Err(err) => {
let (new_guard, no_timeout) = err.into_inner();
(Err(PoisonError::new(new_guard)), no_timeout)
}
}
};
guard_result = new_guard_result;
Expand All @@ -301,14 +342,14 @@ impl StaticCondvar {
/// Wakes up one blocked thread on this condvar.
///
/// See `Condvar::notify_one`.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn notify_one(&'static self) { unsafe { self.inner.notify_one() } }

/// Wakes up all blocked threads on this condvar.
///
/// See `Condvar::notify_all`.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub fn notify_all(&'static self) { unsafe { self.inner.notify_all() } }

Expand All @@ -318,7 +359,7 @@ impl StaticCondvar {
/// active users of the condvar, and this also doesn't prevent any future
/// users of the condvar. This method is required to be called to not leak
/// memory on all platforms.
#[unstable(feature = "std_misc",
#[unstable(feature = "static_condvar",
reason = "may be merged with Condvar in the future")]
pub unsafe fn destroy(&'static self) {
self.inner.destroy()
Expand Down Expand Up @@ -447,7 +488,9 @@ mod tests {
static S: AtomicUsize = ATOMIC_USIZE_INIT;

let g = M.lock().unwrap();
let (g, success) = C.wait_timeout_with(g, Duration::nanoseconds(1000), |_| false).unwrap();
let (g, success) = C.wait_timeout_with(g, Duration::new(0, 1000), |_| {
false
}).unwrap();
assert!(!success);

let (tx, rx) = channel();
Expand All @@ -471,7 +514,8 @@ mod tests {
});

let mut state = 0;
let (_g, success) = C.wait_timeout_with(g, Duration::days(1), |_| {
let day = 24 * 60 * 60;
let (_g, success) = C.wait_timeout_with(g, Duration::new(day, 0), |_| {
assert_eq!(state, S.load(Ordering::SeqCst));
tx.send(()).unwrap();
state += 1;
Expand Down
22 changes: 9 additions & 13 deletions src/libstd/sys/unix/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,20 @@ impl Condvar {
// https://github.com/llvm-mirror/libcxx/blob/release_35/src/condition_variable.cpp#L46
// https://github.com/llvm-mirror/libcxx/blob/release_35/include/__mutex_base#L367
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
if dur <= Duration::zero() {
return false;
}

// First, figure out what time it currently is, in both system and stable time.
// pthread_cond_timedwait uses system time, but we want to report timeout based on stable
// time.
// First, figure out what time it currently is, in both system and
Copy link
Member

Choose a reason for hiding this comment

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

This changes the behavior for a zero duration; intended?

// stable time. pthread_cond_timedwait uses system time, but we want to
// report timeout based on stable time.
let mut sys_now = libc::timeval { tv_sec: 0, tv_usec: 0 };
let stable_now = time::SteadyTime::now();
let r = ffi::gettimeofday(&mut sys_now, ptr::null_mut());
debug_assert_eq!(r, 0);

let seconds = dur.num_seconds() as libc::time_t;
let seconds = dur.secs() as libc::time_t;
let timeout = match sys_now.tv_sec.checked_add(seconds) {
Some(sec) => {
libc::timespec {
tv_sec: sec,
tv_nsec: (dur - Duration::seconds(dur.num_seconds()))
.num_nanoseconds().unwrap() as libc::c_long,
tv_nsec: dur.extra_nanos() as libc::c_long,
}
}
None => {
Expand All @@ -87,11 +82,12 @@ impl Condvar {
};

// And wait!
let r = ffi::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex), &timeout);
let r = ffi::pthread_cond_timedwait(self.inner.get(), mutex::raw(mutex),
&timeout);
debug_assert!(r == libc::ETIMEDOUT || r == 0);

// ETIMEDOUT is not a totally reliable method of determining timeout due to clock shifts,
// so do the check ourselves
// ETIMEDOUT is not a totally reliable method of determining timeout due
// to clock shifts, so do the check ourselves
&time::SteadyTime::now() - &stable_now < dur
}

Expand Down
9 changes: 2 additions & 7 deletions src/libstd/sys/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,9 @@ impl Thread {
}

pub fn sleep(dur: Duration) {
if dur < Duration::zero() {
return Thread::yield_now()
}
let seconds = dur.num_seconds();
let ns = dur - Duration::seconds(seconds);
let mut ts = libc::timespec {
Copy link
Member

Choose a reason for hiding this comment

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

There should exist From<Duration> for libc::timespec in liblibc instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately this can't be provided in liblibc because Duration doesn't exist by that point and due to the in-tree liblibc being different from the out-of-tree liblibc it unfortunately also wouldn't work to provide this in the standard library as well.

tv_sec: seconds as libc::time_t,
tv_nsec: ns.num_nanoseconds().unwrap() as libc::c_long,
tv_sec: dur.secs() as libc::time_t,
tv_nsec: dur.extra_nanos() as libc::c_long,
};

// If we're awoken with a signal then the return value will be -1 and
Expand Down
30 changes: 12 additions & 18 deletions src/libstd/sys/unix/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@

pub use self::inner::SteadyTime;

const NSEC_PER_SEC: u64 = 1_000_000_000;

#[cfg(any(target_os = "macos", target_os = "ios"))]
mod inner {
use libc;
use time::Duration;
use ops::Sub;
use sync::{Once, ONCE_INIT};
use super::NSEC_PER_SEC;

pub struct SteadyTime {
t: u64
Expand All @@ -32,11 +35,6 @@ mod inner {
t: unsafe { mach_absolute_time() },
}
}

pub fn ns(&self) -> u64 {
let info = info();
self.t * info.numer as u64 / info.denom as u64
}
}

fn info() -> &'static libc::mach_timebase_info {
Expand All @@ -59,8 +57,9 @@ mod inner {

fn sub(self, other: &SteadyTime) -> Duration {
let info = info();
let diff = self.t as i64 - other.t as i64;
Duration::nanoseconds(diff * info.numer as i64 / info.denom as i64)
let diff = self.t as u64 - other.t as u64;
let nanos = diff * info.numer as u64 / info.denom as u64;
Duration::new(nanos / NSEC_PER_SEC, (nanos % NSEC_PER_SEC) as u32)
}
}
}
Expand All @@ -70,8 +69,7 @@ mod inner {
use libc;
use time::Duration;
use ops::Sub;

const NSEC_PER_SEC: i64 = 1_000_000_000;
use super::NSEC_PER_SEC;

pub struct SteadyTime {
t: libc::timespec,
Expand Down Expand Up @@ -104,23 +102,19 @@ mod inner {
}
t
}

pub fn ns(&self) -> u64 {
self.t.tv_sec as u64 * NSEC_PER_SEC as u64 + self.t.tv_nsec as u64
}
}

impl<'a> Sub for &'a SteadyTime {
type Output = Duration;

fn sub(self, other: &SteadyTime) -> Duration {
if self.t.tv_nsec >= other.t.tv_nsec {
Duration::seconds(self.t.tv_sec as i64 - other.t.tv_sec as i64) +
Duration::nanoseconds(self.t.tv_nsec as i64 - other.t.tv_nsec as i64)
Duration::new(self.t.tv_sec as u64 - other.t.tv_sec as u64,
self.t.tv_nsec as u32 - other.t.tv_nsec as u32)
} else {
Duration::seconds(self.t.tv_sec as i64 - 1 - other.t.tv_sec as i64) +
Duration::nanoseconds(self.t.tv_nsec as i64 + NSEC_PER_SEC -
other.t.tv_nsec as i64)
Duration::new(self.t.tv_sec as u64 - 1 - other.t.tv_sec as u64,
self.t.tv_nsec as u32 + (NSEC_PER_SEC as u32) -
other.t.tv_nsec as u32)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/windows/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl Condvar {
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
let r = ffi::SleepConditionVariableSRW(self.inner.get(),
mutex::raw(mutex),
dur.num_milliseconds() as DWORD,
super::dur2timeout(dur),
0);
if r == 0 {
const ERROR_TIMEOUT: DWORD = 0x5B4;
Expand Down
22 changes: 22 additions & 0 deletions src/libstd/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use libc;
use num::Zero;
use os::windows::ffi::{OsStrExt, OsStringExt};
use path::PathBuf;
use time::Duration;

pub mod backtrace;
pub mod c;
Expand Down Expand Up @@ -151,6 +152,27 @@ fn cvt<I: PartialEq + Zero>(i: I) -> io::Result<I> {
}
}

fn dur2timeout(dur: Duration) -> libc::DWORD {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, this doesn't quite match any of the proposals floated in e.g. the timeout RFC (where zero was special-cased). I wonder if just rounding up would be simpler? We should discuss.

// Note that a duration is a (u64, u32) (seconds, nanoseconds) pair, and the
Copy link
Member

Choose a reason for hiding this comment

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

Hopefully nobody should have to take care of this themselves.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you elaborate a bit on this? I'm not quite sure what you're implying.

Copy link
Member

Choose a reason for hiding this comment

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

I’m implying that there should eventually exist a method to do such conversion on the Duration itself and that internal Duration representation shouldn’t be really relied on the end-user side (even if through the two methods).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes that definitely makes sense. We don't currently have much interoperation between low-level types and standard library types in terms of conversions like this, but I can definitely see this being a windows extension trait, for example.

// timeouts in windows APIs are typically u32 milliseconds. To translate, we
// have two pieces to take care of:
//
// * Nanosecond precision is rounded up
// * Greater than u32::MAX milliseconds (50 days) is rounded up to INFINITE
// (never time out).
dur.secs().checked_mul(1000).and_then(|ms| {
ms.checked_add((dur.extra_nanos() as u64) / 1_000_000)
}).and_then(|ms| {
ms.checked_add(if dur.extra_nanos() % 1_000_000 > 0 {1} else {0})
}).map(|ms| {
if ms > <libc::DWORD>::max_value() as u64 {
libc::INFINITE
} else {
ms as libc::DWORD
}
}).unwrap_or(libc::INFINITE)
}

fn ms_to_filetime(ms: u64) -> libc::FILETIME {
// A FILETIME is a count of 100 nanosecond intervals, so we multiply by
// 10000 b/c there are 10000 intervals in 1 ms
Expand Down