From 4c75d36d0e81508d4e7614104abb44fa19179c03 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 16 Aug 2013 20:14:30 -0700 Subject: [PATCH 01/14] std: Reduce TLS access --- src/libstd/rt/sched.rs | 5 ++--- src/libstd/unstable/sync.rs | 28 ++++++++++++++++------------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index 5ec2df32c48ca..a8f3d01351b28 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -563,11 +563,10 @@ impl Scheduler { // run the cleanup job, as expected by the previously called // swap_contexts function. unsafe { - let sched = Local::unsafe_borrow::(); - (*sched).run_cleanup_job(); + let task = Local::unsafe_borrow::(); + (*task).sched.get_mut_ref().run_cleanup_job(); // Must happen after running the cleanup job (of course). - let task = Local::unsafe_borrow::(); (*task).death.check_killed((*task).unwinder.unwinding); } } diff --git a/src/libstd/unstable/sync.rs b/src/libstd/unstable/sync.rs index d7f9988edeff9..6fa0e0eb8c132 100644 --- a/src/libstd/unstable/sync.rs +++ b/src/libstd/unstable/sync.rs @@ -281,20 +281,24 @@ impl Drop for UnsafeAtomicRcBox{ */ // FIXME(#8140) should not be pub pub unsafe fn atomically(f: &fn() -> U) -> U { - use rt::task::Task; + use rt::task::{Task, GreenTask, SchedTask}; use rt::local::Local; - use rt::in_green_task_context; - - if in_green_task_context() { - let t = Local::unsafe_borrow::(); - do (|| { - (*t).death.inhibit_deschedule(); - f() - }).finally { - (*t).death.allow_deschedule(); + + match Local::try_unsafe_borrow::() { + Some(t) => { + match (*t).task_type { + GreenTask(_) => { + do (|| { + (*t).death.inhibit_deschedule(); + f() + }).finally { + (*t).death.allow_deschedule(); + } + } + SchedTask => f() + } } - } else { - f() + None => f() } } From 30a7a5b8fa0f0c6262322d353020ff556708082e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 16 Aug 2013 23:14:55 -0700 Subject: [PATCH 02/14] Define cfg(rtopt) when optimizing. Turn off runtime sanity checks Naturally, and sadly, turning off sanity checks in the runtime is a noticable performance win. The particular test I'm running goes from ~1.5 s to ~1.3s. Sanity checks are turned *on* when not optimizing, or when cfg includes `rtdebug` or `rtassert`. --- Makefile.in | 3 ++- src/libstd/macros.rs | 6 ++++-- src/libstd/rt/util.rs | 3 +++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile.in b/Makefile.in index a9a41a073d033..0575f48c4c467 100644 --- a/Makefile.in +++ b/Makefile.in @@ -96,7 +96,8 @@ ifdef CFG_DISABLE_OPTIMIZE $(info cfg: disabling rustc optimization (CFG_DISABLE_OPTIMIZE)) CFG_RUSTC_FLAGS += else - CFG_RUSTC_FLAGS += -O + # The rtopt cfg turns off runtime sanity checks + CFG_RUSTC_FLAGS += -O --cfg rtopt endif ifdef CFG_ENABLE_DEBUG diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 600d0bb133eff..5378a2c798d87 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -27,8 +27,10 @@ macro_rules! rtdebug ( macro_rules! rtassert ( ( $arg:expr ) => ( { - if !$arg { - rtabort!("assertion failed: %s", stringify!($arg)); + if ::rt::util::ENFORCE_SANITY { + if !$arg { + rtabort!("assertion failed: %s", stringify!($arg)); + } } } ) ) diff --git a/src/libstd/rt/util.rs b/src/libstd/rt/util.rs index f2ede8872c298..c3f5b11a93066 100644 --- a/src/libstd/rt/util.rs +++ b/src/libstd/rt/util.rs @@ -19,6 +19,9 @@ use unstable::atomics::{AtomicInt, INIT_ATOMIC_INT, SeqCst}; #[cfg(target_os="macos")] use unstable::running_on_valgrind; +// Indicates whether we should perform expensive sanity checks, including rtassert! +pub static ENFORCE_SANITY: bool = !cfg!(rtopt) || cfg!(rtdebug) || cfg!(rtassert); + /// Get the number of cores available pub fn num_cpus() -> uint { #[fixed_stack_segment]; #[inline(never)]; From 468d023fe94aadbdc7b7dc0a83d264ee2ae0d68a Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 01:12:08 -0700 Subject: [PATCH 03/14] std: Convert some assert!s to rtassert! --- src/libstd/rt/comm.rs | 9 +++++---- src/libstd/rt/sched.rs | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libstd/rt/comm.rs b/src/libstd/rt/comm.rs index 8ef9c1332f9e9..5cd0d7b8b54bd 100644 --- a/src/libstd/rt/comm.rs +++ b/src/libstd/rt/comm.rs @@ -125,7 +125,7 @@ impl ChanOne { unsafe { // Install the payload - assert!((*packet).payload.is_none()); + rtassert!((*packet).payload.is_none()); (*packet).payload = Some(val); // Atomically swap out the old state to figure out what @@ -307,7 +307,7 @@ impl SelectInner for PortOne { STATE_ONE => true, // Lost the race. Data available. same_ptr => { // We successfully unblocked our task pointer. - assert!(task_as_state == same_ptr); + rtassert!(task_as_state == same_ptr); let handle = BlockedTask::cast_from_uint(task_as_state); // Because we are already awake, the handle we // gave to this port shall already be empty. @@ -341,7 +341,8 @@ impl SelectPortInner for PortOne { unsafe { // See corresponding store() above in block_on for rationale. // FIXME(#8130) This can happen only in test builds. - assert!((*packet).state.load(Relaxed) == STATE_ONE); + // This load is not required for correctness and may be compiled out. + rtassert!((*packet).state.load(Relaxed) == STATE_ONE); let payload = (*packet).payload.take(); @@ -387,7 +388,7 @@ impl Drop for ChanOne { }, task_as_state => { // The port is blocked waiting for a message we will never send. Wake it. - assert!((*this.packet()).payload.is_none()); + rtassert!((*this.packet()).payload.is_none()); let recvr = BlockedTask::cast_from_uint(task_as_state); do recvr.wake().map_move |woken_task| { Scheduler::run_task(woken_task); diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index a8f3d01351b28..111e60ccb2f10 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -186,7 +186,7 @@ impl Scheduler { // Should not have any messages let message = stask.sched.get_mut_ref().message_queue.pop(); - assert!(message.is_none()); + rtassert!(message.is_none()); stask.destroyed = true; } From b0a072c4b58a53ceacb326ee7cbebc34bf5d04af Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 01:24:29 -0700 Subject: [PATCH 04/14] std: More TLS micro-optimization --- src/libstd/rt/local.rs | 29 ++++++++++++++++++++--------- src/libstd/rt/local_ptr.rs | 14 +++++++++++--- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/libstd/rt/local.rs b/src/libstd/rt/local.rs index 1faad913b5025..80beb5a2835a0 100644 --- a/src/libstd/rt/local.rs +++ b/src/libstd/rt/local.rs @@ -26,7 +26,9 @@ pub trait Local { } impl Local for Task { + #[inline] fn put(value: ~Task) { unsafe { local_ptr::put(value) } } + #[inline] fn take() -> ~Task { unsafe { local_ptr::take() } } fn exists() -> bool { local_ptr::exists() } fn borrow(f: &fn(&mut Task) -> T) -> T { @@ -43,7 +45,9 @@ impl Local for Task { None => { rtabort!("function failed in local_borrow") } } } + #[inline] unsafe fn unsafe_borrow() -> *mut Task { local_ptr::unsafe_borrow() } + #[inline] unsafe fn try_unsafe_borrow() -> Option<*mut Task> { local_ptr::try_unsafe_borrow() } @@ -57,12 +61,12 @@ impl Local for Scheduler { task.sched = Some(value.take()); }; } + #[inline] fn take() -> ~Scheduler { - do Local::borrow:: |task| { - let sched = task.sched.take_unwrap(); - let task = task; - task.sched = None; - sched + unsafe { + // XXX: Unsafe for speed + let task = Local::unsafe_borrow::(); + (*task).sched.take_unwrap() } } fn exists() -> bool { @@ -97,10 +101,17 @@ impl Local for Scheduler { } } unsafe fn try_unsafe_borrow() -> Option<*mut Scheduler> { - if Local::exists::() { - Some(Local::unsafe_borrow()) - } else { - None + match Local::try_unsafe_borrow::() { + Some(task) => { + match (*task).sched { + Some(~ref mut sched) => { + let s: *mut Scheduler = &mut *sched; + Some(s) + } + None => None + } + } + None => None } } } diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index 77303cb8c06cf..cef833e56b860 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -40,6 +40,7 @@ pub fn init_tls_key() { /// # Safety note /// /// Does not validate the pointer type. +#[inline] pub unsafe fn put(sched: ~T) { let key = tls_key(); let void_ptr: *mut c_void = cast::transmute(sched); @@ -51,6 +52,7 @@ pub unsafe fn put(sched: ~T) { /// # Safety note /// /// Does not validate the pointer type. +#[inline] pub unsafe fn take() -> ~T { let key = tls_key(); let void_ptr: *mut c_void = tls::get(key); @@ -99,10 +101,15 @@ pub unsafe fn borrow(f: &fn(&mut T)) { /// Because this leaves the value in thread-local storage it is possible /// For the Scheduler pointer to be aliased pub unsafe fn unsafe_borrow() -> *mut T { - match try_unsafe_borrow() { - Some(p) => p, - None => rtabort!("thread-local pointer is null. bogus!") + let key = tls_key(); + let mut void_ptr: *mut c_void = tls::get(key); + if void_ptr.is_null() { + rtabort!("thread-local pointer is null. bogus!") } + let ptr: *mut *mut c_void = &mut void_ptr; + let ptr: *mut ~T = ptr as *mut ~T; + let ptr: *mut T = &mut **ptr; + return ptr; } pub unsafe fn try_unsafe_borrow() -> Option<*mut T> { @@ -119,6 +126,7 @@ pub unsafe fn try_unsafe_borrow() -> Option<*mut T> { } } +#[inline] fn tls_key() -> tls::Key { match maybe_tls_key() { Some(key) => key, From d8691b940ff457ebaa4abe3b8034570c22c71417 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 17:40:38 -0700 Subject: [PATCH 05/14] std::rt: Optimize TLS use in change_task_context --- src/libstd/rt/local.rs | 6 ++++++ src/libstd/rt/local_ptr.rs | 17 +++++++++++++++++ src/libstd/rt/sched.rs | 4 +++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/libstd/rt/local.rs b/src/libstd/rt/local.rs index 80beb5a2835a0..c60d284672c9f 100644 --- a/src/libstd/rt/local.rs +++ b/src/libstd/rt/local.rs @@ -21,6 +21,7 @@ pub trait Local { fn take() -> ~Self; fn exists() -> bool; fn borrow(f: &fn(&mut Self) -> T) -> T; + unsafe fn unsafe_take() -> ~Self; unsafe fn unsafe_borrow() -> *mut Self; unsafe fn try_unsafe_borrow() -> Option<*mut Self>; } @@ -46,6 +47,8 @@ impl Local for Task { } } #[inline] + unsafe fn unsafe_take() -> ~Task { local_ptr::unsafe_take() } + #[inline] unsafe fn unsafe_borrow() -> *mut Task { local_ptr::unsafe_borrow() } #[inline] unsafe fn try_unsafe_borrow() -> Option<*mut Task> { @@ -89,6 +92,7 @@ impl Local for Scheduler { } } } + unsafe fn unsafe_take() -> ~Scheduler { rtabort!("unimpl") } unsafe fn unsafe_borrow() -> *mut Scheduler { match (*Local::unsafe_borrow::()).sched { Some(~ref mut sched) => { @@ -122,6 +126,8 @@ impl Local for IoFactoryObject { fn take() -> ~IoFactoryObject { rtabort!("unimpl") } fn exists() -> bool { rtabort!("unimpl") } fn borrow(_f: &fn(&mut IoFactoryObject) -> T) -> T { rtabort!("unimpl") } + + unsafe fn unsafe_take() -> ~IoFactoryObject { rtabort!("unimpl") } unsafe fn unsafe_borrow() -> *mut IoFactoryObject { let sched = Local::unsafe_borrow::(); let io: *mut IoFactoryObject = (*sched).event_loop.io().unwrap(); diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index cef833e56b860..5d529a0a0bdb7 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -64,6 +64,23 @@ pub unsafe fn take() -> ~T { return ptr; } +/// Take ownership of a pointer from thread-local storage. +/// +/// # Safety note +/// +/// Does not validate the pointer type. +/// Leaves the old pointer in TLS for speed. +#[inline] +pub unsafe fn unsafe_take() -> ~T { + let key = tls_key(); + let void_ptr: *mut c_void = tls::get(key); + if void_ptr.is_null() { + rtabort!("thread-local pointer is null. bogus!"); + } + let ptr: ~T = cast::transmute(void_ptr); + return ptr; +} + /// Check whether there is a thread-local pointer installed. pub fn exists() -> bool { unsafe { diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index 111e60ccb2f10..b161864a74fdf 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -505,7 +505,9 @@ impl Scheduler { let mut this = self; // The current task is grabbed from TLS, not taken as an input. - let current_task: ~Task = Local::take::(); + // Doing an unsafe_take to avoid writing back a null pointer - + // We're going to call `put` later to do that. + let current_task: ~Task = unsafe { Local::unsafe_take::() }; // Check that the task is not in an atomically() section (e.g., // holding a pthread mutex, which could deadlock the scheduler). From 509d5c0f3e631a22dcb3c747708636fa38238ef5 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 17:58:00 -0700 Subject: [PATCH 06/14] std::rt: Remove extra boxes from MessageQueue and SleeperList --- src/libstd/rt/message_queue.rs | 7 +++---- src/libstd/rt/sleeper_list.rs | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libstd/rt/message_queue.rs b/src/libstd/rt/message_queue.rs index 8518ddaeae15f..d58c012080360 100644 --- a/src/libstd/rt/message_queue.rs +++ b/src/libstd/rt/message_queue.rs @@ -20,14 +20,13 @@ use unstable::sync::Exclusive; use clone::Clone; pub struct MessageQueue { - // XXX: Another mystery bug fixed by boxing this lock - priv queue: ~Exclusive<~[T]> + priv queue: Exclusive<~[T]> } impl MessageQueue { pub fn new() -> MessageQueue { MessageQueue { - queue: ~Exclusive::new(~[]) + queue: Exclusive::new(~[]) } } @@ -51,7 +50,7 @@ impl MessageQueue { } } -impl Clone for MessageQueue { +impl Clone for MessageQueue { fn clone(&self) -> MessageQueue { MessageQueue { queue: self.queue.clone() diff --git a/src/libstd/rt/sleeper_list.rs b/src/libstd/rt/sleeper_list.rs index d327023de978a..967fde6f37102 100644 --- a/src/libstd/rt/sleeper_list.rs +++ b/src/libstd/rt/sleeper_list.rs @@ -20,13 +20,13 @@ use rt::sched::SchedHandle; use clone::Clone; pub struct SleeperList { - priv stack: ~Exclusive<~[SchedHandle]> + priv stack: Exclusive<~[SchedHandle]> } impl SleeperList { pub fn new() -> SleeperList { SleeperList { - stack: ~Exclusive::new(~[]) + stack: Exclusive::new(~[]) } } @@ -56,4 +56,4 @@ impl Clone for SleeperList { stack: self.stack.clone() } } -} \ No newline at end of file +} From 129f3fb3a15f5b2aee43dda738e887a137b9a882 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 18:16:30 -0700 Subject: [PATCH 07/14] std::rt: Reduce SleeperList contention This makes the lock much less contended. In the test I'm running the number of times it's contended goes from ~100000 down to ~1000. --- src/libstd/rt/sched.rs | 5 +--- src/libstd/rt/sleeper_list.rs | 51 +++++++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index b161864a74fdf..91ab87268f324 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -469,10 +469,7 @@ impl Scheduler { // We've made work available. Notify a // sleeping scheduler. - // XXX: perf. Check for a sleeper without - // synchronizing memory. It's not critical - // that we always find it. - match this.sleeper_list.pop() { + match this.sleeper_list.casual_pop() { Some(handle) => { let mut handle = handle; handle.send(Wake) diff --git a/src/libstd/rt/sleeper_list.rs b/src/libstd/rt/sleeper_list.rs index 967fde6f37102..7232afd6594b5 100644 --- a/src/libstd/rt/sleeper_list.rs +++ b/src/libstd/rt/sleeper_list.rs @@ -15,33 +15,68 @@ use container::Container; use vec::OwnedVector; use option::{Option, Some, None}; use cell::Cell; -use unstable::sync::Exclusive; +use unstable::sync::{UnsafeAtomicRcBox, LittleLock}; use rt::sched::SchedHandle; use clone::Clone; pub struct SleeperList { - priv stack: Exclusive<~[SchedHandle]> + priv state: UnsafeAtomicRcBox +} + +struct State { + count: uint, + stack: ~[SchedHandle], + lock: LittleLock } impl SleeperList { pub fn new() -> SleeperList { SleeperList { - stack: Exclusive::new(~[]) + state: UnsafeAtomicRcBox::new(State { + count: 0, + stack: ~[], + lock: LittleLock::new() + }) } } pub fn push(&mut self, handle: SchedHandle) { let handle = Cell::new(handle); unsafe { - self.stack.with(|s| s.push(handle.take())); + let state = self.state.get(); + do (*state).lock.lock { + (*state).count += 1; + (*state).stack.push(handle.take()); + } } } pub fn pop(&mut self) -> Option { unsafe { - do self.stack.with |s| { - if !s.is_empty() { - Some(s.pop()) + let state = self.state.get(); + do (*state).lock.lock { + if !(*state).stack.is_empty() { + (*state).count -= 1; + Some((*state).stack.pop()) + } else { + None + } + } + } + } + + /// A pop that may sometimes miss enqueued elements, but is much faster + /// to give up without doing any synchronization + pub fn casual_pop(&mut self) -> Option { + unsafe { + let state = self.state.get(); + // NB: Unsynchronized check + if (*state).count == 0 { return None; } + do (*state).lock.lock { + if !(*state).stack.is_empty() { + // NB: count is also protected by the lock + (*state).count -= 1; + Some((*state).stack.pop()) } else { None } @@ -53,7 +88,7 @@ impl SleeperList { impl Clone for SleeperList { fn clone(&self) -> SleeperList { SleeperList { - stack: self.stack.clone() + state: self.state.clone() } } } From 865091a4e0b31d6137f8cbd3206b128e27fa4389 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 19:55:22 -0700 Subject: [PATCH 08/14] std::rt: Reduce MessageQueue contention It's not a huge win but it does reduce the amount of time spent contesting the message queue when the schedulers are under load --- src/libstd/rt/message_queue.rs | 50 ++++++++++++++++++++++++++++------ src/libstd/rt/sched.rs | 31 ++++++++++++++++++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/libstd/rt/message_queue.rs b/src/libstd/rt/message_queue.rs index d58c012080360..2bbcaff6d28d6 100644 --- a/src/libstd/rt/message_queue.rs +++ b/src/libstd/rt/message_queue.rs @@ -16,32 +16,66 @@ use kinds::Send; use vec::OwnedVector; use cell::Cell; use option::*; -use unstable::sync::Exclusive; +use unstable::sync::{UnsafeAtomicRcBox, LittleLock}; use clone::Clone; pub struct MessageQueue { - priv queue: Exclusive<~[T]> + priv state: UnsafeAtomicRcBox> +} + +struct State { + count: uint, + queue: ~[T], + lock: LittleLock } impl MessageQueue { pub fn new() -> MessageQueue { MessageQueue { - queue: Exclusive::new(~[]) + state: UnsafeAtomicRcBox::new(State { + count: 0, + queue: ~[], + lock: LittleLock::new() + }) } } pub fn push(&mut self, value: T) { unsafe { let value = Cell::new(value); - self.queue.with(|q| q.push(value.take()) ); + let state = self.state.get(); + do (*state).lock.lock { + (*state).count += 1; + (*state).queue.push(value.take()); + } } } pub fn pop(&mut self) -> Option { unsafe { - do self.queue.with |q| { - if !q.is_empty() { - Some(q.shift()) + let state = self.state.get(); + do (*state).lock.lock { + if !(*state).queue.is_empty() { + (*state).count += 1; + Some((*state).queue.shift()) + } else { + None + } + } + } + } + + /// A pop that may sometimes miss enqueued elements, but is much faster + /// to give up without doing any synchronization + pub fn casual_pop(&mut self) -> Option { + unsafe { + let state = self.state.get(); + // NB: Unsynchronized check + if (*state).count == 0 { return None; } + do (*state).lock.lock { + if !(*state).queue.is_empty() { + (*state).count += 1; + Some((*state).queue.shift()) } else { None } @@ -53,7 +87,7 @@ impl MessageQueue { impl Clone for MessageQueue { fn clone(&self) -> MessageQueue { MessageQueue { - queue: self.queue.clone() + state: self.state.clone() } } } diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index 91ab87268f324..158a5e3cfe3f5 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -83,6 +83,14 @@ pub struct Scheduler { idle_callback: Option<~PausibleIdleCallback> } +/// An indication of how hard to work on a given operation, the difference +/// mainly being whether memory is synchronized or not +#[deriving(Eq)] +enum EffortLevel { + DontTryTooHard, + GiveItYourBest +} + impl Scheduler { // * Initialization Functions @@ -237,14 +245,21 @@ impl Scheduler { // First we check for scheduler messages, these are higher // priority than regular tasks. - let sched = match sched.interpret_message_queue() { + let sched = match sched.interpret_message_queue(DontTryTooHard) { Some(sched) => sched, None => return }; // This helper will use a randomized work-stealing algorithm // to find work. - let mut sched = match sched.do_work() { + let sched = match sched.do_work() { + Some(sched) => sched, + None => return + }; + + // Now, before sleeping we need to find out if there really + // were any messages. Give it your best! + let mut sched = match sched.interpret_message_queue(GiveItYourBest) { Some(sched) => sched, None => return }; @@ -277,10 +292,18 @@ impl Scheduler { // returns the still-available scheduler. At this point all // message-handling will count as a turn of work, and as a result // return None. - fn interpret_message_queue(~self) -> Option<~Scheduler> { + fn interpret_message_queue(~self, effort: EffortLevel) -> Option<~Scheduler> { let mut this = self; - match this.message_queue.pop() { + + let msg = if effort == DontTryTooHard { + // Do a cheap check that may miss messages + this.message_queue.casual_pop() + } else { + this.message_queue.pop() + }; + + match msg { Some(PinnedTask(task)) => { let mut task = task; task.give_home(Sched(this.make_handle())); From 083db54a32b700d16097e184409b3a7230afe224 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 21:50:37 -0700 Subject: [PATCH 09/14] std::rt: Remove metrics for perf These aren't used for anything at the moment and cause some TLS hits on some perf-critical code paths. Will need to put better thought into it in the future. --- src/libstd/rt/comm.rs | 10 ---- src/libstd/rt/metrics.rs | 98 ---------------------------------------- src/libstd/rt/mod.rs | 2 - src/libstd/rt/sched.rs | 5 -- 4 files changed, 115 deletions(-) delete mode 100644 src/libstd/rt/metrics.rs diff --git a/src/libstd/rt/comm.rs b/src/libstd/rt/comm.rs index 5cd0d7b8b54bd..bd83e2861567d 100644 --- a/src/libstd/rt/comm.rs +++ b/src/libstd/rt/comm.rs @@ -144,16 +144,8 @@ impl ChanOne { match oldstate { STATE_BOTH => { // Port is not waiting yet. Nothing to do - do Local::borrow:: |sched| { - rtdebug!("non-rendezvous send"); - sched.metrics.non_rendezvous_sends += 1; - } } STATE_ONE => { - do Local::borrow:: |sched| { - rtdebug!("rendezvous send"); - sched.metrics.rendezvous_sends += 1; - } // Port has closed. Need to clean up. let _packet: ~Packet = cast::transmute(this.void_packet); recvr_active = false; @@ -251,7 +243,6 @@ impl SelectInner for PortOne { STATE_BOTH => { // Data has not been sent. Now we're blocked. rtdebug!("non-rendezvous recv"); - sched.metrics.non_rendezvous_recvs += 1; false } STATE_ONE => { @@ -267,7 +258,6 @@ impl SelectInner for PortOne { (*self.packet()).state.store(STATE_ONE, Relaxed); rtdebug!("rendezvous recv"); - sched.metrics.rendezvous_recvs += 1; // Channel is closed. Switch back and check the data. // NB: We have to drop back into the scheduler event loop here diff --git a/src/libstd/rt/metrics.rs b/src/libstd/rt/metrics.rs deleted file mode 100644 index b0c0fa5d70862..0000000000000 --- a/src/libstd/rt/metrics.rs +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright 2013 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use to_str::ToStr; - -pub struct SchedMetrics { - // The number of times executing `run_sched_once`. - turns: uint, - // The number of turns that received a message. - messages_received: uint, - // The number of turns that ran a task from the queue. - tasks_resumed_from_queue: uint, - // The number of turns that found no work to perform. - wasted_turns: uint, - // The number of times the scheduler went to sleep. - sleepy_times: uint, - // Context switches from the scheduler into a task. - context_switches_sched_to_task: uint, - // Context switches from a task into the scheduler. - context_switches_task_to_sched: uint, - // Context switches from a task to a task. - context_switches_task_to_task: uint, - // Message sends that unblock the receiver - rendezvous_sends: uint, - // Message sends that do not unblock the receiver - non_rendezvous_sends: uint, - // Message receives that do not block the receiver - rendezvous_recvs: uint, - // Message receives that block the receiver - non_rendezvous_recvs: uint, - // JoinLatch releases that create tombstones - release_tombstone: uint, - // JoinLatch releases that do not create tombstones - release_no_tombstone: uint, -} - -impl SchedMetrics { - pub fn new() -> SchedMetrics { - SchedMetrics { - turns: 0, - messages_received: 0, - tasks_resumed_from_queue: 0, - wasted_turns: 0, - sleepy_times: 0, - context_switches_sched_to_task: 0, - context_switches_task_to_sched: 0, - context_switches_task_to_task: 0, - rendezvous_sends: 0, - non_rendezvous_sends: 0, - rendezvous_recvs: 0, - non_rendezvous_recvs: 0, - release_tombstone: 0, - release_no_tombstone: 0 - } - } -} - -impl ToStr for SchedMetrics { - fn to_str(&self) -> ~str { - fmt!("turns: %u\n\ - messages_received: %u\n\ - tasks_resumed_from_queue: %u\n\ - wasted_turns: %u\n\ - sleepy_times: %u\n\ - context_switches_sched_to_task: %u\n\ - context_switches_task_to_sched: %u\n\ - context_switches_task_to_task: %u\n\ - rendezvous_sends: %u\n\ - non_rendezvous_sends: %u\n\ - rendezvous_recvs: %u\n\ - non_rendezvous_recvs: %u\n\ - release_tombstone: %u\n\ - release_no_tombstone: %u\n\ - ", - self.turns, - self.messages_received, - self.tasks_resumed_from_queue, - self.wasted_turns, - self.sleepy_times, - self.context_switches_sched_to_task, - self.context_switches_task_to_sched, - self.context_switches_task_to_task, - self.rendezvous_sends, - self.non_rendezvous_sends, - self.rendezvous_recvs, - self.non_rendezvous_recvs, - self.release_tombstone, - self.release_no_tombstone - ) - } -} \ No newline at end of file diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs index ead0fb63793ce..0d59d5780cc87 100644 --- a/src/libstd/rt/mod.rs +++ b/src/libstd/rt/mod.rs @@ -152,8 +152,6 @@ pub mod local_ptr; /// Bindings to pthread/windows thread-local storage. pub mod thread_local_storage; -pub mod metrics; - // FIXME #5248 shouldn't be pub /// Just stuff pub mod util; diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index 158a5e3cfe3f5..7d59627ba399b 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -24,7 +24,6 @@ use rt::kill::BlockedTask; use rt::local_ptr; use rt::local::Local; use rt::rtio::{RemoteCallback, PausibleIdleCallback}; -use rt::metrics::SchedMetrics; use borrow::{to_uint}; use cell::Cell; use rand::{XorShiftRng, RngUtil}; @@ -71,7 +70,6 @@ pub struct Scheduler { /// An action performed after a context switch on behalf of the /// code running before the context switch cleanup_job: Option, - metrics: SchedMetrics, /// Should this scheduler run any task, or only pinned tasks? run_anything: bool, /// If the scheduler shouldn't run some tasks, a friend to send @@ -126,7 +124,6 @@ impl Scheduler { stack_pool: StackPool::new(), sched_task: None, cleanup_job: None, - metrics: SchedMetrics::new(), run_anything: run_anything, friend_handle: friend, rng: XorShiftRng::new(), @@ -267,10 +264,8 @@ impl Scheduler { // If we got here then there was no work to do. // Generate a SchedHandle and push it to the sleeper list so // somebody can wake us up later. - sched.metrics.wasted_turns += 1; if !sched.sleepy && !sched.no_sleep { rtdebug!("scheduler has no work to do, going to sleep"); - sched.metrics.sleepy_times += 1; sched.sleepy = true; let handle = sched.make_handle(); sched.sleeper_list.push(handle); From b0cae8a218131f74224997e3a60aafd77976bf97 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 02:12:08 -0700 Subject: [PATCH 10/14] std: Convert the runtime TLS key to a Rust global to avoid FFI --- src/libstd/rt/local_ptr.rs | 32 +++++++++++++++++--------------- src/rt/rust_builtin.cpp | 17 ++++++----------- src/rt/rustrt.def.in | 1 - 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index 5d529a0a0bdb7..10ff61d837590 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -23,14 +23,16 @@ use option::{Option, Some, None}; use unstable::finally::Finally; use tls = rt::thread_local_storage; +static mut RT_TLS_KEY: tls::Key = -1; + /// Initialize the TLS key. Other ops will fail if this isn't executed first. #[fixed_stack_segment] #[inline(never)] pub fn init_tls_key() { unsafe { - rust_initialize_rt_tls_key(); + rust_initialize_rt_tls_key(&mut RT_TLS_KEY); extern { - fn rust_initialize_rt_tls_key(); + fn rust_initialize_rt_tls_key(key: *mut tls::Key); } } } @@ -151,15 +153,10 @@ fn tls_key() -> tls::Key { } } -#[fixed_stack_segment] -#[inline(never)] -fn maybe_tls_key() -> Option { +#[inline] +#[cfg(not(test))] +pub fn maybe_tls_key() -> Option { unsafe { - let key: *mut c_void = rust_get_rt_tls_key(); - let key: &mut tls::Key = cast::transmute(key); - let key = *key; - // Check that the key has been initialized. - // NB: This is a little racy because, while the key is // initalized under a mutex and it's assumed to be initalized // in the Scheduler ctor by any thread that needs to use it, @@ -170,14 +167,19 @@ fn maybe_tls_key() -> Option { // another thread. I think this is fine since the only action // they could take if it was initialized would be to check the // thread-local value and see that it's not set. - if key != -1 { - return Some(key); + if RT_TLS_KEY != -1 { + return Some(RT_TLS_KEY); } else { return None; } } +} - extern { - fn rust_get_rt_tls_key() -> *mut c_void; - } +// XXX: The boundary between the running runtime and the testing runtime +// seems to be fuzzy at the moment, and trying to use two different keys +// results in disaster. This should not be necessary. +#[inline] +#[cfg(test)] +pub fn maybe_tls_key() -> Option { + unsafe { ::cast::transmute(::realstd::rt::local_ptr::maybe_tls_key()) } } diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp index 6f3a3bd36865f..27cc486c39efc 100644 --- a/src/rt/rust_builtin.cpp +++ b/src/rt/rust_builtin.cpp @@ -447,19 +447,14 @@ rust_readdir() { #endif #ifndef _WIN32 -pthread_key_t rt_key = -1; +typedef pthread_key_t tls_key; #else -DWORD rt_key = -1; +typedef DWORD tls_key; #endif -extern "C" void* -rust_get_rt_tls_key() { - return &rt_key; -} - // Initialize the TLS key used by the new scheduler extern "C" CDECL void -rust_initialize_rt_tls_key() { +rust_initialize_rt_tls_key(tls_key *key) { static lock_and_signal init_lock; static bool initialized = false; @@ -469,10 +464,10 @@ rust_initialize_rt_tls_key() { if (!initialized) { #ifndef _WIN32 - assert(!pthread_key_create(&rt_key, NULL)); + assert(!pthread_key_create(key, NULL)); #else - rt_key = TlsAlloc(); - assert(rt_key != TLS_OUT_OF_INDEXES); + *key = TlsAlloc(); + assert(*key != TLS_OUT_OF_INDEXES); #endif initialized = true; diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in index 6e95d96012bbf..b668d39440662 100644 --- a/src/rt/rustrt.def.in +++ b/src/rt/rustrt.def.in @@ -143,7 +143,6 @@ linenoiseHistoryLoad rust_raw_thread_start rust_raw_thread_join rust_raw_thread_delete -rust_get_rt_tls_key swap_registers rust_readdir rust_opendir From e866277e0afe14ed73b6b42545130ee42db07373 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 22:59:46 -0700 Subject: [PATCH 11/14] std::rt: Remove an unnecessary allocation from the main sched loop --- src/libstd/rt/task.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index 12ba39a6dcd67..9c2a6e646d2ce 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -59,7 +59,7 @@ pub struct Task { } pub enum TaskType { - GreenTask(Option<~SchedHome>), + GreenTask(Option), SchedTask } @@ -173,7 +173,7 @@ impl Task { name: None, coroutine: Some(Coroutine::new(stack_pool, stack_size, start)), sched: None, - task_type: GreenTask(Some(~home)), + task_type: GreenTask(Some(home)), borrow_list: None } } @@ -196,7 +196,7 @@ impl Task { name: None, coroutine: Some(Coroutine::new(stack_pool, stack_size, start)), sched: None, - task_type: GreenTask(Some(~home)), + task_type: GreenTask(Some(home)), borrow_list: None } } @@ -204,7 +204,7 @@ impl Task { pub fn give_home(&mut self, new_home: SchedHome) { match self.task_type { GreenTask(ref mut home) => { - *home = Some(~new_home); + *home = Some(new_home); } SchedTask => { rtabort!("type error: used SchedTask as GreenTask"); @@ -216,7 +216,7 @@ impl Task { match self.task_type { GreenTask(ref mut home) => { let out = home.take_unwrap(); - return *out; + return out; } SchedTask => { rtabort!("type error: used SchedTask as GreenTask"); @@ -275,8 +275,8 @@ impl Task { pub fn is_home_no_tls(&self, sched: &~Scheduler) -> bool { match self.task_type { - GreenTask(Some(~AnySched)) => { false } - GreenTask(Some(~Sched(SchedHandle { sched_id: ref id, _}))) => { + GreenTask(Some(AnySched)) => { false } + GreenTask(Some(Sched(SchedHandle { sched_id: ref id, _}))) => { *id == sched.sched_id() } GreenTask(None) => { @@ -291,8 +291,8 @@ impl Task { pub fn homed(&self) -> bool { match self.task_type { - GreenTask(Some(~AnySched)) => { false } - GreenTask(Some(~Sched(SchedHandle { _ }))) => { true } + GreenTask(Some(AnySched)) => { false } + GreenTask(Some(Sched(SchedHandle { _ }))) => { true } GreenTask(None) => { rtabort!("task without home"); } @@ -309,11 +309,11 @@ impl Task { let sched_id = task.sched.get_ref().sched_id(); let sched_run_anything = task.sched.get_ref().run_anything; match task.task_type { - GreenTask(Some(~AnySched)) => { + GreenTask(Some(AnySched)) => { rtdebug!("anysched task in sched check ****"); sched_run_anything } - GreenTask(Some(~Sched(SchedHandle { sched_id: ref id, _ }))) => { + GreenTask(Some(Sched(SchedHandle { sched_id: ref id, _ }))) => { rtdebug!("homed task in sched check ****"); *id == sched_id } From 30dc02c12d00e540b50806d492d460160ca9493f Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 17 Aug 2013 23:11:46 -0700 Subject: [PATCH 12/14] std: Make vec::push_all_move call reserve_at_least vec::unshift uses this to add elements, scheduler queues use unshift, and this was causing a lot of reallocation --- src/libstd/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/vec.rs b/src/libstd/vec.rs index f196cf423c130..10dc1b5409e57 100644 --- a/src/libstd/vec.rs +++ b/src/libstd/vec.rs @@ -1401,7 +1401,7 @@ impl OwnedVector for ~[T] { let self_len = self.len(); let rhs_len = rhs.len(); let new_len = self_len + rhs_len; - self.reserve(new_len); + self.reserve_at_least(new_len); unsafe { // Note: infallible. let self_p = vec::raw::to_mut_ptr(*self); let rhs_p = vec::raw::to_ptr(rhs); From 2e86230bd53b79da5e5f4abceff7bf1aafbc01bc Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 19 Aug 2013 10:10:27 -0700 Subject: [PATCH 13/14] std::rt: Enforce sanity a while longer I'm not comfortable turning off rtassert! yet --- src/libstd/rt/util.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstd/rt/util.rs b/src/libstd/rt/util.rs index c3f5b11a93066..9113f03ffee1c 100644 --- a/src/libstd/rt/util.rs +++ b/src/libstd/rt/util.rs @@ -20,7 +20,8 @@ use unstable::atomics::{AtomicInt, INIT_ATOMIC_INT, SeqCst}; use unstable::running_on_valgrind; // Indicates whether we should perform expensive sanity checks, including rtassert! -pub static ENFORCE_SANITY: bool = !cfg!(rtopt) || cfg!(rtdebug) || cfg!(rtassert); +// XXX: Once the runtime matures remove the `true` below to turn off rtassert, etc. +pub static ENFORCE_SANITY: bool = true || !cfg!(rtopt) || cfg!(rtdebug) || cfg!(rtassert); /// Get the number of cores available pub fn num_cpus() -> uint { From 092739956e55d9f262a895ea60e355ba0632b92e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 22 Aug 2013 13:24:21 -0700 Subject: [PATCH 14/14] Whitespace --- src/libstd/rt/local.rs | 2 +- src/libstd/rt/local_ptr.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/rt/local.rs b/src/libstd/rt/local.rs index c60d284672c9f..48c8c34702858 100644 --- a/src/libstd/rt/local.rs +++ b/src/libstd/rt/local.rs @@ -126,7 +126,7 @@ impl Local for IoFactoryObject { fn take() -> ~IoFactoryObject { rtabort!("unimpl") } fn exists() -> bool { rtabort!("unimpl") } fn borrow(_f: &fn(&mut IoFactoryObject) -> T) -> T { rtabort!("unimpl") } - + unsafe fn unsafe_take() -> ~IoFactoryObject { rtabort!("unimpl") } unsafe fn unsafe_borrow() -> *mut IoFactoryObject { let sched = Local::unsafe_borrow::(); diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index 10ff61d837590..d269e63cff29c 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -123,7 +123,7 @@ pub unsafe fn unsafe_borrow() -> *mut T { let key = tls_key(); let mut void_ptr: *mut c_void = tls::get(key); if void_ptr.is_null() { - rtabort!("thread-local pointer is null. bogus!") + rtabort!("thread-local pointer is null. bogus!") } let ptr: *mut *mut c_void = &mut void_ptr; let ptr: *mut ~T = ptr as *mut ~T;