Skip to content

Commit

Permalink
Optionally support parking_lot within global allocators.
Browse files Browse the repository at this point in the history
This adds a feature to support using parking_lot locks in
`#[global_allocator]` implementations. The main hazard is parking_lot calling
into the global allocator, and then the global allocator calling back into
parking_lot, at a point where parking_lot is not prepared to be re-entered on
the same thread.

There are currently two places where this comes up:

 - When a new thread is created, parking_lot needs to allocate space in its
   hashtable for the new thread. It uses the global allocator to allocate this
   space, and if the global allocator uses parking lot, it doesn't work because
   the hashtable is not ready for the new thread yet.

 - If the new hashtable is allocated while bucket locks are held, and the
   global allocator attempts to acquire a parking lot lock, it deadlocks
   because all the buckets are locked.

For the first, this adds a new `reserve` function, defined when the "reserve"
feature is enabled. This allocates space in the hashtable, and if users call
it before each thread creation in the program, parking_lot won't ever need to
call `grow_hashtable` on new threads. This is admittedly awkward to guarantee
in general, however it's not a problem in my own use case. And this function
might also be useful in cases where users want to create a pool of threads all
at once, to reduce temporary allocations, leakage, and rehashing. However, I'm
open to other ideas here.

For the second, when "reserve" is enabled, `create_hashtable` will allocate the
new `HashTable` before locking all the bucket locks, to avoid deadlock if
allocating the `HashTable` attempts to take a lock.
  • Loading branch information
sunfishcode committed Dec 7, 2021
1 parent 7b12ce3 commit ca57261
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
5 changes: 5 additions & 0 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ winapi = { version = "0.3.9", features = ["winnt", "ntstatus", "minwindef",
[features]
nightly = []
deadlock_detection = ["petgraph", "thread-id", "backtrace"]

# Enable this to enable the `reserve` function, which, if called before every
# thread creation in the program, allows `parking_lot` locks to be used in
# `global_allocator` implementations.
reserve = []
2 changes: 2 additions & 0 deletions core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ mod util;
mod word_lock;

pub use self::parking_lot::deadlock;
#[cfg(feature = "reserve")]
pub use self::parking_lot::reserve;
pub use self::parking_lot::{park, unpark_all, unpark_filter, unpark_one, unpark_requeue};
pub use self::parking_lot::{
FilterOp, ParkResult, ParkToken, RequeueOp, UnparkResult, UnparkToken,
Expand Down
31 changes: 26 additions & 5 deletions core/src/parking_lot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ cfg_if::cfg_if! {

static NUM_THREADS: AtomicUsize = AtomicUsize::new(0);

#[cfg(feature = "reserve")]
static NUM_RESERVED: AtomicUsize = AtomicUsize::new(0);

/// Holds the pointer to the currently active `HashTable`.
///
/// # Safety
Expand Down Expand Up @@ -192,6 +195,17 @@ impl ThreadData {
}
}

/// Reserve memory for `num_additional` additional new threads.
///
/// If all threads in a program have space reserved for them before they're
/// created, then the parking lot will never do a dynamic allocation on a
/// thread before the hashtable is ready for that thread.
#[cfg(feature = "reserve")]
pub fn reserve(num_additional: usize) {
let num_reserved = NUM_RESERVED.fetch_add(num_additional, Ordering::Relaxed) + num_additional;
grow_hashtable(num_reserved);
}

// Invokes the given closure with a reference to the current thread `ThreadData`.
#[inline(always)]
fn with_thread_data<T>(f: impl FnOnce(&ThreadData) -> T) -> T {
Expand Down Expand Up @@ -263,14 +277,19 @@ fn create_hashtable() -> &'static HashTable {
// created, which only happens once per thread.
fn grow_hashtable(num_threads: usize) {
// Lock all buckets in the existing table and get a reference to it
let old_table = loop {
let (old_table, mut new_table) = loop {
let table = get_hashtable();

// Check if we need to resize the existing table
if table.entries.len() >= LOAD_FACTOR * num_threads {
return;
}

// For feature = "reserve", create the new table here, before we lock
// all the buckets, in case the allocator needs to acquire a lock.
#[cfg(feature = "reserve")]
let new_table = HashTable::new(num_threads, table);

// Lock all buckets in the old table
for bucket in &table.entries[..] {
bucket.mutex.lock();
Expand All @@ -280,7 +299,12 @@ fn grow_hashtable(num_threads: usize) {
// have grown the hash table between us reading HASHTABLE and locking
// the buckets.
if HASHTABLE.load(Ordering::Relaxed) == table as *const _ as *mut _ {
break table;
// For not(feature = "reserve"), create the new table here, after
// we lock all the buckets, to minimize temporary allocations.
#[cfg(not(feature = "reserve"))]
let new_table = HashTable::new(num_threads, table);

break (table, new_table);
}

// Unlock buckets and try again
Expand All @@ -290,9 +314,6 @@ fn grow_hashtable(num_threads: usize) {
}
};

// Create the new table
let mut new_table = HashTable::new(num_threads, old_table);

// Move the entries from the old table to the new one
for bucket in &old_table.entries[..] {
// SAFETY: The park, unpark* and check_wait_graph_fast functions create only correct linked
Expand Down

0 comments on commit ca57261

Please sign in to comment.