Skip to content

Commit

Permalink
Merge pull request #140 from faern/fix-feedback-from-libstd-1
Browse files Browse the repository at this point in the history
Fix feedback from libstd PR, part 1
  • Loading branch information
Amanieu authored Jun 2, 2019
2 parents 6087747 + 6607fd3 commit 3a395e1
Show file tree
Hide file tree
Showing 17 changed files with 296 additions and 389 deletions.
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ backtrace = { version = "0.3.2", optional = true }
rustc_version = "0.2"

[target.'cfg(unix)'.dependencies]
libc = "0.2.27"
libc = "0.2.55"

[target.'cfg(target_os = "redox")'.dependencies]
redox_syscall = "0.1"
Expand Down
35 changes: 1 addition & 34 deletions core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,9 @@
feature(thread_local, checked_duration_since)
)]

use cfg_if::cfg_if;

cfg_if! {
if #[cfg(all(has_sized_atomics, target_os = "linux"))] {
#[path = "thread_parker/linux.rs"]
mod thread_parker;
} else if #[cfg(unix)] {
#[path = "thread_parker/unix.rs"]
mod thread_parker;
} else if #[cfg(windows)] {
#[path = "thread_parker/windows/mod.rs"]
mod thread_parker;
} else if #[cfg(all(has_sized_atomics, target_os = "redox"))] {
#[path = "thread_parker/redox.rs"]
mod thread_parker;
} else if #[cfg(all(target_env = "sgx", target_vendor = "fortanix"))] {
#[path = "thread_parker/sgx.rs"]
mod thread_parker;
} else if #[cfg(all(
feature = "nightly",
target_arch = "wasm32",
target_feature = "atomics"
))] {
#[path = "thread_parker/wasm.rs"]
mod thread_parker;
} else if #[cfg(all(feature = "nightly", target_os = "cloudabi"))] {
#[path = "thread_parker/cloudabi.rs"]
mod thread_parker;
} else {
#[path = "thread_parker/generic.rs"]
mod thread_parker;
}
}

mod parking_lot;
mod spinwait;
mod thread_parker;
mod util;
mod word_lock;

Expand Down
69 changes: 26 additions & 43 deletions core/src/parking_lot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// http://opensource.org/licenses/MIT>, at your option. This file may not be
// copied, modified, or distributed except according to those terms.

use crate::thread_parker::ThreadParker;
use crate::thread_parker::{ThreadParker, ThreadParkerT, UnparkHandleT};
use crate::util::UncheckedOptionExt;
use crate::word_lock::WordLock;
use core::{
Expand Down Expand Up @@ -161,10 +161,7 @@ impl ThreadData {

// Invokes the given closure with a reference to the current thread `ThreadData`.
#[inline(always)]
fn with_thread_data<F, T>(f: F) -> T
where
F: FnOnce(&ThreadData) -> T,
{
fn with_thread_data<T>(f: impl FnOnce(&ThreadData) -> T) -> T {
// Unlike word_lock::ThreadData, parking_lot::ThreadData is always expensive
// to construct. Try to use a thread-local version if possible. Otherwise just
// create a ThreadData on the stack
Expand Down Expand Up @@ -194,7 +191,6 @@ fn get_hashtable() -> *mut HashTable {

// Get a pointer to the latest hash table, creating one if it doesn't exist yet.
#[cold]
#[inline(never)]
fn create_hashtable() -> *mut HashTable {
let new_table = Box::into_raw(HashTable::new(LOAD_FACTOR, ptr::null()));

Expand Down Expand Up @@ -405,13 +401,8 @@ unsafe fn lock_bucket_pair<'a>(key1: usize, key2: usize) -> (&'a Bucket, &'a Buc
// Unlock a pair of buckets
#[inline]
unsafe fn unlock_bucket_pair(bucket1: &Bucket, bucket2: &Bucket) {
if bucket1 as *const _ == bucket2 as *const _ {
bucket1.mutex.unlock();
} else if bucket1 as *const _ < bucket2 as *const _ {
bucket2.mutex.unlock();
bucket1.mutex.unlock();
} else {
bucket1.mutex.unlock();
bucket1.mutex.unlock();
if !ptr::eq(bucket1, bucket2) {
bucket2.mutex.unlock();
}
}
Expand Down Expand Up @@ -538,19 +529,14 @@ pub const DEFAULT_PARK_TOKEN: ParkToken = ParkToken(0);
/// to call `unpark_one`, `unpark_all`, `unpark_requeue` or `unpark_filter`, but
/// it is not allowed to call `park` or panic.
#[inline]
pub unsafe fn park<V, B, T>(
pub unsafe fn park(
key: usize,
validate: V,
before_sleep: B,
timed_out: T,
validate: impl FnOnce() -> bool,
before_sleep: impl FnOnce(),
timed_out: impl FnOnce(usize, bool),
park_token: ParkToken,
timeout: Option<Instant>,
) -> ParkResult
where
V: FnOnce() -> bool,
B: FnOnce(),
T: FnOnce(usize, bool),
{
) -> ParkResult {
// Grab our thread data, this also ensures that the hash table exists
with_thread_data(|thread_data| {
// Lock the bucket for the given key
Expand Down Expand Up @@ -579,9 +565,9 @@ where
// Invoke the pre-sleep callback
before_sleep();

// Park our thread and determine whether we were woken up by an unpark or by
// our timeout. Note that this isn't precise: we can still be unparked since
// we are still in the queue.
// Park our thread and determine whether we were woken up by an unpark
// or by our timeout. Note that this isn't precise: we can still be
// unparked since we are still in the queue.
let unparked = match timeout {
Some(timeout) => thread_data.parker.park_until(timeout),
None => {
Expand Down Expand Up @@ -676,10 +662,10 @@ where
/// The `callback` function is called while the queue is locked and must not
/// panic or call into any function in `parking_lot`.
#[inline]
pub unsafe fn unpark_one<C>(key: usize, callback: C) -> UnparkResult
where
C: FnOnce(UnparkResult) -> UnparkToken,
{
pub unsafe fn unpark_one(
key: usize,
callback: impl FnOnce(UnparkResult) -> UnparkToken,
) -> UnparkResult {
// Lock the bucket for the given key
let bucket = lock_bucket(key);

Expand Down Expand Up @@ -825,16 +811,12 @@ pub unsafe fn unpark_all(key: usize, unpark_token: UnparkToken) -> usize {
/// The `validate` and `callback` functions are called while the queue is locked
/// and must not panic or call into any function in `parking_lot`.
#[inline]
pub unsafe fn unpark_requeue<V, C>(
pub unsafe fn unpark_requeue(
key_from: usize,
key_to: usize,
validate: V,
callback: C,
) -> UnparkResult
where
V: FnOnce() -> RequeueOp,
C: FnOnce(RequeueOp, UnparkResult) -> UnparkToken,
{
validate: impl FnOnce() -> RequeueOp,
callback: impl FnOnce(RequeueOp, UnparkResult) -> UnparkToken,
) -> UnparkResult {
// Lock the two buckets for the given key
let (bucket_from, bucket_to) = lock_bucket_pair(key_from, key_to);

Expand Down Expand Up @@ -956,11 +938,11 @@ where
/// The `filter` and `callback` functions are called while the queue is locked
/// and must not panic or call into any function in `parking_lot`.
#[inline]
pub unsafe fn unpark_filter<F, C>(key: usize, mut filter: F, callback: C) -> UnparkResult
where
F: FnMut(ParkToken) -> FilterOp,
C: FnOnce(UnparkResult) -> UnparkToken,
{
pub unsafe fn unpark_filter(
key: usize,
mut filter: impl FnMut(ParkToken) -> FilterOp,
callback: impl FnOnce(UnparkResult) -> UnparkToken,
) -> UnparkResult {
// Lock the bucket for the given key
let bucket = lock_bucket(key);

Expand Down Expand Up @@ -1078,6 +1060,7 @@ pub mod deadlock {
#[cfg(feature = "deadlock_detection")]
mod deadlock_impl {
use super::{get_hashtable, lock_bucket, with_thread_data, ThreadData, NUM_THREADS};
use crate::thread_parker::{ThreadParkerT, UnparkHandleT};
use crate::word_lock::WordLock;
use backtrace::Backtrace;
use petgraph;
Expand Down
Loading

0 comments on commit 3a395e1

Please sign in to comment.