From dcb5a1ce543548ec9d7cffa8d01c4a3c81946a4c Mon Sep 17 00:00:00 2001 From: DQ Date: Wed, 8 Apr 2020 01:12:35 +0200 Subject: [PATCH] Implement the tree bin optimization (#72) --- .gitignore | 1 + src/iter/traverser.rs | 48 +- src/lib.rs | 97 +- src/map.rs | 1233 +++++++++++++++++++++---- src/map_ref.rs | 32 +- src/node.rs | 1389 ++++++++++++++++++++++++++++- src/raw/mod.rs | 29 +- src/serde_impls.rs | 12 +- src/set.rs | 52 +- src/set_ref.rs | 22 +- tests/jdk/concurrent_associate.rs | 2 +- tests/jdk/map_check.rs | 14 +- 12 files changed, 2636 insertions(+), 295 deletions(-) diff --git a/.gitignore b/.gitignore index 69369904..ecb95b05 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /target **/*.rs.bk Cargo.lock +**/.vscode/* diff --git a/src/iter/traverser.rs b/src/iter/traverser.rs index ab0dcb43..80a58bb9 100644 --- a/src/iter/traverser.rs +++ b/src/iter/traverser.rs @@ -1,4 +1,4 @@ -use crate::node::{BinEntry, Node}; +use crate::node::{BinEntry, Node, TreeNode}; use crate::raw::Table; use crossbeam_epoch::{Guard, Shared}; use std::sync::atomic::Ordering; @@ -116,19 +116,28 @@ impl<'g, K, V> Iterator for NodeIter<'g, K, V> { if let Some(prev) = self.prev { let next = prev.next.load(Ordering::SeqCst, self.guard); if !next.is_null() { + // we have to check if we are iterating over a regular bin or a + // TreeBin. the Java code gets away without this due to + // inheritance (everything is a node), but we have to explicitly + // check // safety: flurry does not drop or move until after guard drop - e = Some( - unsafe { next.deref() } - .as_node() - .expect("only Nodes follow a Node"), - ) + match unsafe { next.deref() } { + BinEntry::Node(node) => { + e = Some(node); + } + BinEntry::TreeNode(tree_node) => { + e = Some(&tree_node.node); + } + BinEntry::Moved => unreachable!("Nodes can only point to Nodes or TreeNodes"), + BinEntry::Tree(_) => unreachable!("Nodes can only point to Nodes or TreeNodes"), + } } } loop { - if let Some(e) = e { - self.prev = Some(e); - return Some(e); + if e.is_some() { + self.prev = e; + return e; } // safety: flurry does not drop or move until after guard drop @@ -160,6 +169,27 @@ impl<'g, K, V> Iterator for NodeIter<'g, K, V> { BinEntry::Node(node) => { e = Some(node); } + BinEntry::Tree(tree_bin) => { + // since we want to iterate over all entries, TreeBins + // are also traversed via the `next` pointers of their + // contained node + e = Some( + // safety: `bin` was read under our guard, at which + // point the tree was valid. Since our guard pins + // the current epoch, the TreeNodes remain valid for + // at least as long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + &unsafe { + TreeNode::get_tree_node( + tree_bin.first.load(Ordering::SeqCst, self.guard), + ) + } + .node, + ); + } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), } } diff --git a/src/lib.rs b/src/lib.rs index 5060ba24..528e2806 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,13 +55,20 @@ //! operation. When possible, it is a good idea to provide a size estimate by using the //! [`with_capacity`](HashMap::with_capacity) constructor. Note that using many keys with //! exactly the same [`Hash`](std::hash::Hash) value is a sure way to slow down performance of any -//! hash table. +//! hash table. To ameliorate impact, keys are required to be [`Ord`](std::cmp::Ord). This is used +//! by the map to more efficiently store bins that contain a large number of elements with +//! colliding hashes using the comparison order on their keys. //! /* //! TODO: dynamic load factor +//! */ +//! # Hash Sets //! -//! TODO: set projection +//! Flurry also supports concurrent hash sets, which may be created through [`HashSet`]. Hash sets +//! offer the same instantiation options as [`HashMap`], such as [`new`](HashSet::new) and +//! [`with_capacity`](HashSet::with_capacity). //! +/* //! TODO: frequency map through computeIfAbsent //! //! TODO: bulk operations like forEach, search, and reduce @@ -76,16 +83,18 @@ //! file](http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/main/java/util/concurrent/ConcurrentHashMap.java?view=markup). //! //! The primary design goal of this hash table is to maintain concurrent readability (typically -//! method `get()`, but also iterators and related methods) while minimizing update contention. +//! method [`get`](HashMap::get), but also iterators and related methods) while minimizing update contention. //! Secondary goals are to keep space consumption about the same or better than java.util.HashMap, //! and to support high initial insertion rates on an empty table by many threads. //! //! This map usually acts as a binned (bucketed) hash table. Each key-value mapping is held in a -//! `BinEntry`. Most nodes are of type `BinEntry::Node` with hash, key, value, and a `next` field. -//! However, some nodes are of type `BinEntry::Moved`; these "forwarding nodes" are placed at the -//! heads of bins during resizing. The Java version also has other special node types, but these -//! have not yet been implemented in this port. These special nodes are all either uncommon or -//! transient. +//! `BinEntry`. Most nodes are of type `BinEntry::Node` with hash, key, value, and a `next` field. +//! However, some other types of nodes exist: `BinEntry::TreeNode`s are arranged in balanced trees +//! instead of linear lists. Bins of type `BinEntry::Tree` hold the roots of sets of `BinEntry::TreeNode`s. +//! Some nodes are of type `BinEntry::Moved`; these "forwarding nodes" are placed at the +//! heads of bins during resizing. The Java version also has other special node types, but these +//! have not yet been implemented in this port. These special nodes are all either uncommon or +//! transient. //! /* //! TODO: TreeNodes, ReservationNodes @@ -95,8 +104,8 @@ //! `BinEntry`). Table accesses require atomic reads, writes, and CASes. //! //! Insertion (via `put`) of the first node in an empty bin is performed by just CASing it to the -//! bin. This is by far the most common case for put operations under most key/hash distributions. -//! Other update operations (insert, delete, and replace) require locks. We do not want to waste +//! bin. This is by far the most common case for put operations under most key/hash distributions. +//! Other update operations (insert, delete, and replace) require locks. We do not want to waste //! the space required to associate a distinct lock object with each bin, so we instead embed a //! lock inside each node, and use the lock in the the first node of a bin list as the lock for the //! bin. @@ -109,7 +118,7 @@ //! The main disadvantage of per-bin locks is that other update operations on other nodes in a bin //! list protected by the same lock can stall, for example when user `Eq` implementations or //! mapping functions take a long time. However, statistically, under random hash codes, this is -//! not a common problem. Ideally, the frequency of nodes in bins follows a Poisson distribution +//! not a common problem. Ideally, the frequency of nodes in bins follows a Poisson distribution //! (http://en.wikipedia.org/wiki/Poisson_distribution) with a parameter of about 0.5 on average, //! given the resizing threshold of 0.75, although with a large variance because of resizing //! granularity. Ignoring variance, the expected occurrences of list size `k` are `exp(-0.5) * @@ -132,30 +141,33 @@ //! #elements)` under random hashes. //! //! Actual hash code distributions encountered in practice sometimes deviate significantly from -//! uniform randomness. This includes the case when `N > (1<<30)`, so some keys MUST collide. +//! uniform randomness. This includes the case when `N > (1<<30)`, so some keys MUST collide. //! Similarly for dumb or hostile usages in which multiple keys are designed to have identical hash -//! codes or ones that differs only in masked-out high bits. Here, the Java implementation uses an -//! optimization where a bin is turned into a binary tree, but this has not yet been ported over to -//! the Rust version. -/* TODO */ +//! codes or ones that differs only in masked-out high bits. So we use secondary strategy that +//! applies when the number of nodes in a bin exceeds a threshold. These `BinEntry::Tree` bins use +//! a balanced tree to hold nodes (a specialized form of red-black trees), bounding search time to +//! `O(log N)`. Each search step in such a bin is at least twice as slow as in a regular list, but +//! given that N cannot exceed `(1<<64)` (before running out of adresses) this bounds search steps, +//! lock hold times, etc, to reasonable constants (roughly 100 nodes inspected per operation worst +//! case). `BinEntry::Tree` nodes (`BinEntry::TreeNode`s) also maintain the same `next` traversal +//! pointers as regular nodes, so can be traversed in iterators in a similar way. //! //! The table is resized when occupancy exceeds a percentage threshold (nominally, 0.75, but see -//! below). Any thread noticing an overfull bin may assist in resizing after the initiating thread +//! below). Any thread noticing an overfull bin may assist in resizing after the initiating thread //! allocates and sets up the replacement array. However, rather than stalling, these other threads -//! may proceed with insertions etc. Resizing proceeds by transferring bins, one by one, from the -//! table to the next table. However, threads claim small blocks of indices to transfer (via the -//! field `transfer_index`) before doing so, reducing contention. A generation stamp in the field -//! `size_ctl` ensures that resizings do not overlap. Because we are using power-of-two expansion, -//! the elements from each bin must either stay at same index, or move with a power of two offset. -//! We eliminate unnecessary node creation by catching cases where old nodes can be reused because -//! their next fields won't change. On average, only about one-sixth of them need cloning when a -//! table doubles. The nodes they replace will be garbage collectible as soon as they are no longer -//! referenced by any reader thread that may be in the midst of concurrently traversing table. -//! Upon transfer, the old table bin contains only a special forwarding node (`BinEntry::Moved`) -//! that contains the next table as its key. On encountering a forwarding node, access and update -//! operations restart, using the new table. -//! -/* TODO: note on TreeBins */ +//! may proceed with insertions etc. The use of `BinEntry::Tree` bins shields us from the worst case +//! effects of overfilling while resizes are in progress. Resizing proceeds by transferring bins, +//! one by one, from the table to the next table. However, threads claim small blocks of indices to +//! transfer (via the field `transfer_index`) before doing so, reducing contention. A generation +//! stamp in the field `size_ctl` ensures that resizings do not overlap. Because we are using +//! power-of-two expansion, the elements from each bin must either stay at same index, or move with +//! a power of two offset. We eliminate unnecessary node creation by catching cases where old nodes +//! can be reused because their next fields won't change. On average, only about one-sixth of them +//! need cloning when a table doubles. The nodes they replace will be garbage collectible as soon +//! as they are no longer referenced by any reader thread that may be in the midst of concurrently +//! traversing table. Upon transfer, the old table bin contains only a special forwarding node +//! (`BinEntry::Moved`) that contains the next table as its key. On encountering a forwarding node, +//! access and update operations restart, using the new table. //! //! Each bin transfer requires its bin lock, which can stall waiting for locks while resizing. //! However, because other threads can join in and help resize rather than contend for locks, @@ -190,10 +202,26 @@ //! occurring at threshold is around 13%, meaning that only about 1 in 8 puts check threshold (and //! after resizing, many fewer do so). //! */ -/* TODO: //! -//! TreeBins comparisons and locking -*/ +/* NOTE that we don't actually use most of the Java Code's complicated comparisons and tiebreakers + * since we require total ordering among the keys via `Ord` as opposed to a runtime check against + * Java's `Comparable` interface. */ +//! `BinEntry::Tree` bins use a special form of comparison for search and related operations (which +//! is the main reason we cannot use existing collections such as tree maps). The contained tree +//! is primarily ordered by hash value, then by [`cmp`](std::cmp::Ord::cmp) order on keys. The +//! red-black balancing code is updated from pre-jdk colelctions (http://gee.cs.oswego.edu/dl/classes/collections/RBCell.java) +//! based in turn on Cormen, Leiserson, and Rivest "Introduction to Algorithms" (CLR). +//! +//! `BinEntry::Tree` bins also require an additional locking mechanism. While list traversal is +//! always possible by readers even during updates, tree traversal is not, mainly because of +//! tree-rotations that may change the root node and/or its linkages. Tree bins include a simple +//! read-write lock mechanism parasitic on the main bin-synchronization strategy: Structural +//! adjustments associated with an insertion or removal are already bin-locked (and so cannot +//! conflict with other writers) but must wait for ongoing readers to finish. Since there can be +//! only one such waiter, we use a simple scheme using a single `waiter` field to block writers. +//! However, readers need never block. If the root lock is held, they proceed along the slow +//! traversal path (via next-pointers) until the lock becomes available or the list is exhausted, +//! whichever comes first. These cases are not fast, but maximize aggregate expected throughput. //! //! ## Garbage collection //! @@ -213,6 +241,7 @@ intra_doc_link_resolution_failure )] #![warn(rust_2018_idioms)] +#![allow(clippy::cognitive_complexity)] use crossbeam_epoch::Guard; use std::ops::Deref; diff --git a/src/map.rs b/src/map.rs index 72698538..fc0bebf7 100644 --- a/src/map.rs +++ b/src/map.rs @@ -25,6 +25,24 @@ const MAXIMUM_CAPACITY: usize = 1 << 30; // TODO: use ISIZE_BITS /// (i.e., at least 1) and at most `MAXIMUM_CAPACITY`. const DEFAULT_CAPACITY: usize = 16; +/// The bin count threshold for using a tree rather than list for a bin. Bins are +/// converted to trees when adding an element to a bin with at least this many +/// nodes. The value must be greater than 2, and should be at least 8 to mesh +/// with assumptions in tree removal about conversion back to plain bins upon +/// shrinkage. +const TREEIFY_THRESHOLD: usize = 8; + +/// The bin count threshold for untreeifying a (split) bin during a resize +/// operation. Should be less than TREEIFY_THRESHOLD, and at most 6 to mesh with +/// shrinkage detection under removal. +const UNTREEIFY_THRESHOLD: usize = 6; + +/// The smallest table capacity for which bins may be treeified. (Otherwise the +/// table is resized if too many nodes in a bin.) The value should be at least 4 +/// * TREEIFY_THRESHOLD to avoid conflicts between resizing and treeification +/// thresholds. +const MIN_TREEIFY_CAPACITY: usize = 64; + /// Minimum number of rebinnings per transfer step. Ranges are /// subdivided to allow multiple resizer threads. This value /// serves as a lower bound to avoid resizers encountering @@ -56,7 +74,7 @@ macro_rules! load_factor { /// A concurrent hash table. /// -/// Flurry uses an [`Guards`] to control the lifetime of the resources that get stored and +/// Flurry uses [`Guards`] to control the lifetime of the resources that get stored and /// extracted from the map. [`Guards`] are acquired through the [`epoch::pin`], [`HashMap::pin`] /// and [`HashMap::guard`] functions. For more information, see the [notes in the crate-level /// documentation]. @@ -526,15 +544,16 @@ impl HashMap { } // === -// the following methods require Clone, since they ultimately call `transfer`, which needs to be -// able to clone keys. however, they do _not_ need to require thread-safety bounds (Send + Sync + -// 'static) since if the bounds do not hold, the map is empty, so no keys or values will be -// transfered anyway. +// the following methods require Clone and Ord, since they ultimately call +// `transfer`, which needs to be able to clone keys and work with tree bins. +// however, they do _not_ need to require thread-safety bounds (Send + Sync + +// 'static) since if the bounds do not hold, the map is empty, so no keys or +// values will be transfered anyway. // === impl HashMap where - K: Clone, + K: Clone + Ord, { /// Tries to presize table to accommodate the given number of elements. fn try_presize(&self, size: usize, guard: &Guard) { @@ -791,15 +810,18 @@ where // safety: bin is a valid pointer. // - // there are two cases when a bin pointer is invalidated: + // there are three cases when a bin pointer is invalidated: // // 1. if the table was resized, bin is a move entry, and the resize has completed. in // that case, the table (and all its heads) will be dropped in the next epoch // following that. // 2. if the table is being resized, bin may be swapped with a move entry. the old bin // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. // - // in both cases, we held the guard when we got the reference to the bin. if any such + // in all cases, we held the guard when we got the reference to the bin. if any such // swap happened, it must have happened _after_ we read. since we did the read while // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we // are holding up by holding on to our guard). @@ -822,7 +844,7 @@ where // yes, it is still the head, so we can now "own" the bin // note that there can still be readers in the bin! - // TODO: TreeBin & ReservationNode + // TODO: ReservationNode let mut run_bit = head.hash & n as u64; let mut last_run = bin; @@ -881,13 +903,12 @@ where &mut high_bin }; - *link = Owned::new(BinEntry::Node(Node { - hash: node.hash, - key: node.key.clone(), - lock: parking_lot::Mutex::new(()), - value: node.value.clone(), - next: Atomic::from(*link), - })) + *link = Owned::new(BinEntry::Node(Node::with_next( + node.hash, + node.key.clone(), + node.value.clone(), + Atomic::from(*link), + ))) .into_shared(guard); p = node.next.load(Ordering::SeqCst, guard); @@ -928,6 +949,161 @@ where drop(head_lock); } + BinEntry::Tree(ref tree_bin) => { + let bin_lock = tree_bin.lock.lock(); + + // need to check that this is _still_ the correct bin + let current_head = table.bin(i, guard); + if current_head != bin { + // nope -- try again from the start + continue; + } + + let mut low = Shared::null(); + let mut low_tail = Shared::null(); + let mut high = Shared::null(); + let mut high_tail = Shared::null(); + let mut low_count = 0; + let mut high_count = 0; + let mut e = tree_bin.first.load(Ordering::Relaxed, guard); + while !e.is_null() { + // safety: the TreeBin was read under our guard, at + // which point the tree structure was valid. Since our + // guard pins the current epoch, the TreeNodes remain + // valid for at least as long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let tree_node = unsafe { TreeNode::get_tree_node(e) }; + let hash = tree_node.node.hash; + let new_node = TreeNode::new( + hash, + tree_node.node.key.clone(), + tree_node.node.value.clone(), + Atomic::null(), + Atomic::null(), + ); + let run_bit = hash & n as u64; + if run_bit == 0 { + new_node.prev.store(low_tail, Ordering::Relaxed); + let new_node = + Owned::new(BinEntry::TreeNode(new_node)).into_shared(guard); + if low_tail.is_null() { + // this is the first element inserted into the low bin + low = new_node; + } else { + // safety: `low_tail` was just created by us and not shared. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + unsafe { TreeNode::get_tree_node(low_tail) } + .node + .next + .store(new_node, Ordering::Relaxed); + } + low_tail = new_node; + low_count += 1; + } else { + new_node.prev.store(high_tail, Ordering::Relaxed); + let new_node = + Owned::new(BinEntry::TreeNode(new_node)).into_shared(guard); + if high_tail.is_null() { + // this is the first element inserted into the high bin + high = new_node; + } else { + // safety: `high_tail` was just created by us and not shared. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + unsafe { TreeNode::get_tree_node(high_tail) } + .node + .next + .store(new_node, Ordering::Relaxed); + } + high_tail = new_node; + high_count += 1; + } + e = tree_node.node.next.load(Ordering::Relaxed, guard); + } + + let mut reused_bin = false; + let low_bin = if low_count <= UNTREEIFY_THRESHOLD { + // use a regular bin instead of a tree bin since the + // bin is too small. since the tree nodes are + // already behind shared references, we have to + // clean them up manually. + let low_linear = Self::untreeify(low, guard); + // safety: we have just created `low` and its `next` + // nodes and have never shared them + unsafe { TreeBin::drop_tree_nodes(low, false, guard) }; + low_linear + } else if high_count != 0 { + // the new bin will also be a tree bin. if both the high + // bin and the low bin are non-empty, we have to + // allocate a new TreeBin. + Owned::new(BinEntry::Tree(TreeBin::new( + // safety: we have just created `low` and its `next` + // nodes and have never shared them + unsafe { low.into_owned() }, + guard, + ))) + .into_shared(guard) + } else { + // if not, we can re-use the old bin here, since it will + // be swapped for a Moved entry while we are still + // behind the bin lock. + reused_bin = true; + // since we also don't use the created low nodes here, + // we need to clean them up. + // safety: we have just created `low` and its `next` + // nodes and have never shared them + unsafe { TreeBin::drop_tree_nodes(low, false, guard) }; + bin + }; + let high_bin = if high_count <= UNTREEIFY_THRESHOLD { + let high_linear = Self::untreeify(high, guard); + // safety: we have just created `high` and its `next` + // nodes and have never shared them + unsafe { TreeBin::drop_tree_nodes(high, false, guard) }; + high_linear + } else if low_count != 0 { + Owned::new(BinEntry::Tree(TreeBin::new( + // safety: we have just created `high` and its `next` + // nodes and have never shared them + unsafe { high.into_owned() }, + guard, + ))) + .into_shared(guard) + } else { + reused_bin = true; + // since we also don't use the created low nodes here, + // we need to clean them up. + // safety: we have just created `high` and its `next` + // nodes and have never shared them + unsafe { TreeBin::drop_tree_nodes(high, false, guard) }; + bin + }; + + next_table.store_bin(i, low_bin); + next_table.store_bin(i + n, high_bin); + table.store_bin( + i, + table.get_moved(Shared::from(next_table as *const _), guard), + ); + + // if we did not re-use the old bin, it is now garbage, + // since all of its nodes have been reallocated. However, + // we always re-use the stored values, so we can't drop those. + if !reused_bin { + // safety: the entry for this bin in the old table was + // swapped for a Moved entry, so no thread can obtain a + // new reference to `bin` from there. we only defer + // dropping the old bin if it was not used in + // `next_table` so there is no other reference to it + // anyone could obtain. + unsafe { TreeBin::defer_drop_without_values(bin, guard) }; + } + + advance = true; + drop(bin_lock); + } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), } } } @@ -1084,7 +1260,7 @@ where impl HashMap where - K: Hash + Eq, + K: Hash + Ord, S: BuildHasher, { #[inline] @@ -1097,7 +1273,7 @@ where fn get_node<'g, Q>(&'g self, key: &Q, guard: &'g Guard) -> Option<&'g Node> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { let table = self.table.load(Ordering::SeqCst, guard); if table.is_null() { @@ -1120,15 +1296,18 @@ where // safety: bin is a valid pointer. // - // there are two cases when a bin pointer is invalidated: + // there are three cases when a bin pointer is invalidated: // // 1. if the table was resized, bin is a move entry, and the resize has completed. in // that case, the table (and all its heads) will be dropped in the next epoch // following that. // 2. if the table is being resized, bin may be swapped with a move entry. the old bin // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. // - // in both cases, we held the guard when we got the reference to the bin. if any such + // in all cases, we held the guard when we got the reference to the bin. if any such // swap happened, it must have happened _after_ we read. since we did the read while // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we // are holding up by holding on to our guard). @@ -1140,19 +1319,20 @@ where // next epoch after it is removed. since it wasn't removed, and the epoch was pinned, that // cannot be until after we drop our guard. let node = unsafe { node.deref() }; - Some( - node.as_node() - .expect("`BinEntry::find` should always return a Node"), - ) + Some(match node { + BinEntry::Node(ref n) => n, + BinEntry::TreeNode(ref tn) => &tn.node, + _ => panic!("`Table::find` should always return a Node"), + }) } /// Returns `true` if the map contains a value for the specified key. /// /// The key may be any borrowed form of the map's key type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the key type. /// - /// [`Eq`]: std::cmp::Eq + /// [`Ord`]: std::cmp::Ord /// [`Hash`]: std::hash::Hash /// /// # Examples @@ -1169,7 +1349,7 @@ where pub fn contains_key(&self, key: &Q, guard: &Guard) -> bool where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.check_guard(guard); self.get(key, &guard).is_some() @@ -1178,10 +1358,10 @@ where /// Returns a reference to the value corresponding to the key. /// /// The key may be any borrowed form of the map's key type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the key type. /// - /// [`Eq`]: std::cmp::Eq + /// [`Ord`]: std::cmp::Ord /// [`Hash`]: std::hash::Hash /// /// To obtain a `Guard`, use [`HashMap::guard`]. @@ -1201,7 +1381,7 @@ where pub fn get<'g, Q>(&'g self, key: &Q, guard: &'g Guard) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.check_guard(guard); let node = self.get_node(key, guard)?; @@ -1218,14 +1398,17 @@ where /// /// Returns `None` if this map contains no mapping for `key`. /// - /// The supplied `key` may be any borrowed form of the - /// map's key type, but `Hash` and `Eq` on the borrowed form - /// must match those for the key type. + /// The key may be any borrowed form of the map's key type, but + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for + /// the key type. + /// + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash #[inline] pub fn get_key_value<'g, Q>(&'g self, key: &Q, guard: &'g Guard) -> Option<(&'g K, &'g V)> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.check_guard(guard); let node = self.get_node(key, guard)?; @@ -1258,7 +1441,7 @@ where impl HashMap where - K: Clone, + K: Clone + Ord, { /// Clears the map, removing all key-value pairs. /// @@ -1305,7 +1488,13 @@ where } // we now own the bin // unlink it from the map to prevent others from entering it + // NOTE: The Java code stores the null bin _after_ the loop, and thus also has + // to hold the lock until that point. However, after the store happens new + // threads and threads waiting on the lock will read the new bin, so we can + // drop the lock early and do the counting and garbage collection outside the + // critical section. tab.store_bin(idx, Shared::null()); + drop(head_lock); // next, walk the nodes of the bin and free the nodes and their values as we go // note that we do not free the head node yet, since we're holding the lock it contains let mut p = node.next.load(Ordering::SeqCst, guard); @@ -1332,7 +1521,6 @@ where next }; } - drop(head_lock); // finally, we can drop the head node and its value let value = node.value.load(Ordering::SeqCst, guard); // NOTE: do not use the reference in `node` after this point! @@ -1342,6 +1530,49 @@ where delta -= 1; idx += 1; } + BinEntry::Tree(ref tree_bin) => { + let bin_lock = tree_bin.lock.lock(); + // need to check that this is _still_ the correct bin + let current_head = tab.bin(idx, guard); + if current_head != raw_node { + // nope -- try the bin again + continue; + } + // we now own the bin + // unlink it from the map to prevent others from entering it + // NOTE: The Java code stores the null bin _after_ the loop, and thus also has + // to hold the lock until that point. However, after the store happens new + // threads and threads waiting on the lock will read the new bin, so we can + // drop the lock early and do the counting and garbage collection outside the + // critical section. + tab.store_bin(idx, Shared::null()); + drop(bin_lock); + // next, walk the nodes of the bin and count how many values we remove + let mut p = tree_bin.first.load(Ordering::SeqCst, guard); + while !p.is_null() { + delta -= 1; + p = { + // safety: the TreeBin was read under our guard, at + // which point the tree was valid. Since our guard + // pins the current epoch, the TreeNodes remain + // valid for at least as long as we hold onto the + // guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let tree_node = unsafe { TreeNode::get_tree_node(p) }; + // NOTE: we do not drop the TreeNodes or their + // values here, since they will be dropped together + // with the containing TreeBin (`tree_bin`) in its + // `drop` + tree_node.node.next.load(Ordering::SeqCst, guard) + }; + } + // safety: same as in the BinEntry::Node case above + unsafe { guard.defer_destroy(raw_node) }; + idx += 1; + } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), }; } @@ -1358,7 +1589,7 @@ where impl HashMap where - K: 'static + Sync + Send + Clone + Hash + Eq, + K: 'static + Sync + Send + Clone + Hash + Ord, V: 'static + Sync + Send, S: BuildHasher, { @@ -1445,24 +1676,16 @@ where fn put<'g>( &'g self, - key: K, + mut key: K, value: V, no_replacement: bool, guard: &'g Guard, ) -> PutResult<'g, V> { - let h = self.hash(&key); - + let hash = self.hash(&key); let mut table = self.table.load(Ordering::SeqCst, guard); - + let mut bin_count; let value = Owned::new(value).into_shared(guard); - let mut node = Owned::new(BinEntry::Node(Node { - key, - value: Atomic::from(value), - hash: h, - next: Atomic::null(), - lock: parking_lot::Mutex::new(()), - })); - + let mut old_val = None; loop { // safety: see argument below for !is_null case if table.is_null() || unsafe { table.deref() }.is_empty() { @@ -1489,10 +1712,11 @@ where // still be valid, see the safety comment on Table.next_table. let t = unsafe { table.deref() }; - let bini = t.bini(h); + let bini = t.bini(hash); let mut bin = t.bin(bini, guard); if bin.is_null() { // fast path -- bin is empty so stick us at the front + let node = Owned::new(BinEntry::Node(Node::new(hash, key, value))); match t.cas_bin(bini, bin, node, guard) { Ok(_old_null_ptr) => { self.add_count(1, Some(0), guard); @@ -1509,44 +1733,50 @@ where } Err(changed) => { assert!(!changed.current.is_null()); - node = changed.new; bin = changed.current; + if let BinEntry::Node(node) = *changed.new.into_box() { + key = node.key; + } else { + unreachable!("we declared node and it is a BinEntry::Node"); + } } } } // slow path -- bin is non-empty + // safety: bin is a valid pointer. // - // there are two cases when a bin pointer is invalidated: + // there are three cases when a bin pointer is invalidated: // // 1. if the table was resized, bin is a move entry, and the resize has completed. in // that case, the table (and all its heads) will be dropped in the next epoch // following that. // 2. if the table is being resized, bin may be swapped with a move entry. the old bin // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. // - // in both cases, we held the guard when we got the reference to the bin. if any such + // in all cases, we held the guard when we got the reference to the bin. if any such // swap happened, it must have happened _after_ we read. since we did the read while // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we // are holding up by holding on to our guard). - let key = &node.as_node().unwrap().key; match *unsafe { bin.deref() } { BinEntry::Moved => { table = self.help_transfer(table, guard); + continue; } BinEntry::Node(ref head) - if no_replacement && head.hash == h && &head.key == key => + if no_replacement && head.hash == hash && head.key == key => { // fast path if replacement is disallowed and first bin matches let v = head.value.load(Ordering::SeqCst, guard); - drop(node); // safety (for v): since the value is present now, and we've held a guard from // the beginning of the search, the value cannot be dropped until the next // epoch, which won't arrive until after we drop our guard. - // safety (for value): since we never inserted the node or the value in the - // tree, and the node has been dropped, `value` is the last remaining pointer - // to the initial value. + // safety (for value): since we never inserted the value in the tree, `value` + // is the last remaining pointer to the initial value. return PutResult::Exists { current: unsafe { v.deref() }, not_inserted: unsafe { value.into_owned().into_box() }, @@ -1566,18 +1796,18 @@ where // yes, it is still the head, so we can now "own" the bin // note that there can still be readers in the bin! - // TODO: TreeBin & ReservationNode + // TODO: ReservationNode - let mut bin_count = 1; + bin_count = 1; let mut p = bin; - let old_val = loop { + old_val = loop { // safety: we read the bin while pinning the epoch. a bin will never be // dropped until the next epoch after it is removed. since it wasn't // removed, and the epoch was pinned, that cannot be until after we drop // our guard. let n = unsafe { p.deref() }.as_node().unwrap(); - if n.hash == h && &n.key == key { + if n.hash == hash && n.key == key { // the key already exists in the map! let current_value = n.value.load(Ordering::SeqCst, guard); @@ -1588,14 +1818,12 @@ where if no_replacement { // the key is not absent, so don't update + // because of `no_replacement`, we don't use the + // new value, so we need to clean it up + // safety: we own value and did not share it + let _ = unsafe { value.into_owned() }; } else { - // drop the node we made -- we won't need it - // TODO: we should avoid allocating the node if we won't need it. - let _ = node.into_box(); - // then update the value in the existing node - // NOTE: using `value` after dropping the `BinEntry` for `node` is - // fine here -- the value is behind an `Atomic`, which doesn't - // automatically drop its target when dropped. + // update the value in the existing node let now_garbage = n.value.swap(value, Ordering::SeqCst, guard); // NOTE: now_garbage == current_value @@ -1627,6 +1855,7 @@ where let next = n.next.load(Ordering::SeqCst, guard); if next.is_null() { // we're at the end of the bin -- stick the node here! + let node = Owned::new(BinEntry::Node(Node::new(hash, key, value))); n.next.store(node, Ordering::SeqCst); break None; } @@ -1635,32 +1864,119 @@ where bin_count += 1; }; drop(head_lock); + } + // NOTE: BinEntry::Tree(ref tree_bin) if no_replacement && head.hash == h && &head.key == key + // cannot occur as in the Java code, TreeBins have a special, indicator hash value + BinEntry::Tree(ref tree_bin) => { + // bin is non-empty, need to link into it, so we must take the lock + let head_lock = tree_bin.lock.lock(); - // TODO: TREEIFY_THRESHOLD - - if old_val.is_none() { - // increment count - self.add_count(1, Some(bin_count), guard); + // need to check that this is _still_ the correct bin + let current_head = t.bin(bini, guard); + if current_head != bin { + // nope -- try again from the start + continue; } - guard.flush(); + // yes, it is still the head, so we can now "own" the bin + // note that there can still be readers in the bin! + + // we don't actually count bins, just set this low enough + // that we don't try to treeify the bin later + bin_count = 2; + let p = tree_bin.find_or_put_tree_val(hash, key, value, guard); + if p.is_null() { + // no TreeNode was returned, so the key did not previously exist in the + // TreeBin. This means it was successfully put there by the call above + // and we are done. + break; + } + // safety: the TreeBin was read under our guard, at + // which point the tree structure was valid. Since our + // guard pins the current epoch, the TreeNodes remain + // valid for at least as long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let tree_node = unsafe { TreeNode::get_tree_node(p) }; + old_val = { + let current_value = tree_node.node.value.load(Ordering::SeqCst, guard); + // safety: since the value is present now, and we've held a guard from + // the beginning of the search, the value cannot be dropped until the + // next epoch, which won't arrive until after we drop our guard. + let current_value = unsafe { current_value.deref() }; + if no_replacement { + // the key is not absent, so don't update + // because of `no_replacement`, we don't use the + // new value, so we need to clean it up + // safety: we own value and did not share it + let _ = unsafe { value.into_owned() }; + } else { + let now_garbage = + tree_node.node.value.swap(value, Ordering::SeqCst, guard); + // NOTE: now_garbage == current_value + + // safety: need to guarantee that now_garbage is no longer + // reachable. more specifically, no thread that executes _after_ + // this line can ever get a reference to now_garbage. + // + // here are the possible cases: + // + // - another thread already has a reference to now_garbage. + // they must have read it before the call to swap. + // because of this, that thread must be pinned to an epoch <= + // the epoch of our guard. since the garbage is placed in our + // epoch, it won't be freed until the _next_ epoch, at which + // point, that thread must have dropped its guard, and with it, + // any reference to the value. + // - another thread is about to get a reference to this value. + // they execute _after_ the swap, and therefore do _not_ get a + // reference to now_garbage (they get `value` instead). there are + // no other ways to get to a value except through its Node's + // `value` field (which is what we swapped), so freeing + // now_garbage is fine. + unsafe { guard.defer_destroy(now_garbage) }; + } + Some(current_value) + }; + drop(head_lock); + } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), + } + // NOTE: the Java code checks `bin_count` here because they also + // reach this point if the bin changed while obtaining the lock. + // However, our code doesn't (it uses continue) and `bin_count` + // _cannot_ be 0 at this point. + debug_assert_ne!(bin_count, 0); + if bin_count >= TREEIFY_THRESHOLD { + self.treeify_bin(t, bini, guard); + } + if let Some(old_val) = old_val { + return PutResult::Replaced { + old: old_val, // safety: we have not moved the node's value since we placed it into its // `Atomic` in the very beginning of the method, so the ref is still valid. // since the value is not currently marked as garbage, we know it will not // collected until at least one epoch passes, and since `value` was produced // under a guard the pins the current epoch, the returned reference will remain // valid for the guard's lifetime. - return match old_val { - Some(old_val) => PutResult::Replaced { - old: old_val, - new: unsafe { value.deref() }, - }, - None => PutResult::Inserted { - new: unsafe { value.deref() }, - }, - }; - } + new: unsafe { value.deref() }, + }; } + break; + } + // increment count, since we only get here if we did not return an old (updated) value + debug_assert!(old_val.is_none()); + self.add_count(1, Some(bin_count), guard); + guard.flush(); + PutResult::Inserted { + // safety: we have not moved the node's value since we placed it into its + // `Atomic` in the very beginning of the method, so the ref is still valid. + // since the value is not currently marked as garbage, we know it will not + // collected until at least one epoch passes, and since `value` was produced + // under a guard the pins the current epoch, the returned reference will remain + // valid for the guard's lifetime. + new: unsafe { value.deref() }, } } @@ -1684,6 +2000,13 @@ where /// /// Returns the new value associated with the specified `key`, or `None` /// if no value for the specified `key` is present. + /// + /// The key may be any borrowed form of the map's key type, but + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for + /// the key type. + /// + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash pub fn compute_if_present<'g, Q, F>( &'g self, key: &Q, @@ -1692,14 +2015,16 @@ where ) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, F: FnOnce(&K, &V) -> Option, { self.check_guard(guard); - let h = self.hash(&key); + let hash = self.hash(&key); let mut table = self.table.load(Ordering::SeqCst, guard); - + let mut new_val = None; + let mut removed_node = false; + let mut bin_count; loop { // safety: see argument below for !is_null case if table.is_null() || unsafe { table.deref() }.is_empty() { @@ -1726,7 +2051,7 @@ where // still be valid, see the safety comment on Table.next_table. let t = unsafe { table.deref() }; - let bini = t.bini(h); + let bini = t.bini(hash); let bin = t.bin(bini, guard); if bin.is_null() { // fast path -- bin is empty so key is not present @@ -1736,21 +2061,25 @@ where // slow path -- bin is non-empty // safety: bin is a valid pointer. // - // there are two cases when a bin pointer is invalidated: + // there are three cases when a bin pointer is invalidated: // // 1. if the table was resized, bin is a move entry, and the resize has completed. in // that case, the table (and all its heads) will be dropped in the next epoch // following that. // 2. if the table is being resized, bin may be swapped with a move entry. the old bin // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. // - // in both cases, we held the guard when we got the reference to the bin. if any such + // in all cases, we held the guard when we got the reference to the bin. if any such // swap happened, it must have happened _after_ we read. since we did the read while // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we // are holding up by holding on to our guard). match *unsafe { bin.deref() } { BinEntry::Moved => { table = self.help_transfer(table, guard); + continue; } BinEntry::Node(ref head) => { // bin is non-empty, need to link into it, so we must take the lock @@ -1766,14 +2095,12 @@ where // yes, it is still the head, so we can now "own" the bin // note that there can still be readers in the bin! - // TODO: TreeBin & ReservationNode - - let mut removed_node = false; - let mut bin_count = 1; + // TODO: ReservationNode + bin_count = 1; let mut p = bin; let mut pred: Shared<'_, BinEntry> = Shared::null(); - let new_val = loop { + new_val = loop { // safety: we read the bin while pinning the epoch. a bin will never be // dropped until the next epoch after it is removed. since it wasn't // removed, and the epoch was pinned, that cannot be until after we drop @@ -1781,7 +2108,7 @@ where let n = unsafe { p.deref() }.as_node().unwrap(); // TODO: This Ordering can probably be relaxed due to the Mutex let next = n.next.load(Ordering::SeqCst, guard); - if n.hash == h && n.key.borrow() == key { + if n.hash == hash && n.key.borrow() == key { // the key already exists in the map! let current_value = n.value.load(Ordering::SeqCst, guard); @@ -1792,8 +2119,8 @@ where remapping_function(&n.key, unsafe { current_value.deref() }); if let Some(value) = new_value { - let now_garbage = - n.value.swap(Owned::new(value), Ordering::SeqCst, guard); + let value = Owned::new(value).into_shared(guard); + let now_garbage = n.value.swap(value, Ordering::SeqCst, guard); // NOTE: now_garbage == current_value // safety: need to guarantee that now_garbage is no longer @@ -1820,9 +2147,7 @@ where // safety: since the value is present now, and we've held a guard from // the beginning of the search, the value cannot be dropped until the // next epoch, which won't arrive until after we drop our guard. - break Some(unsafe { - n.value.load(Ordering::SeqCst, guard).deref() - }); + break Some(unsafe { value.deref() }); } else { removed_node = true; // remove the BinEntry containing the removed key value pair from the bucket @@ -1874,25 +2199,149 @@ where bin_count += 1; }; drop(head_lock); + } + BinEntry::Tree(ref tree_bin) => { + // bin is non-empty, need to link into it, so we must take the lock + let bin_lock = tree_bin.lock.lock(); - if removed_node { - // decrement count - self.add_count(-1, Some(bin_count), guard); + // need to check that this is _still_ the head + let current_head = t.bin(bini, guard); + if current_head != bin { + // nope -- try again from the start + continue; } - guard.flush(); - return new_val; + + // yes, it is still the head, so we can now "own" the bin + // note that there can still be readers in the bin! + bin_count = 2; + let root = tree_bin.root.load(Ordering::SeqCst, guard); + if root.is_null() { + // TODO: Is this even reachable? + // The Java code has `NULL` checks for this, but in theory it should not be possible to + // encounter a tree that has no root when we have its lock. It should always have at + // least `UNTREEIFY_THRESHOLD` nodes. If it is indeed impossible we should replace + // this with an assertion instead. + break; + } + new_val = { + let p = TreeNode::find_tree_node(root, hash, key, guard); + if p.is_null() { + // the given key is not present in the map + None + } else { + // a node for the given key exists, so we try to update it + // safety: the TreeBin was read under our guard, + // at which point the tree structure was valid. + // Since our guard pins the current epoch, the + // TreeNodes and `p` in particular remain valid + // for at least as long as we hold onto the + // guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let n = &unsafe { TreeNode::get_tree_node(p) }.node; + let current_value = n.value.load(Ordering::SeqCst, guard); + + // safety: since the value is present now, and we've held a guard from + // the beginning of the search, the value cannot be dropped until the + // next epoch, which won't arrive until after we drop our guard. + let new_value = + remapping_function(&n.key, unsafe { current_value.deref() }); + + if let Some(value) = new_value { + let value = Owned::new(value).into_shared(guard); + let now_garbage = n.value.swap(value, Ordering::SeqCst, guard); + // NOTE: now_garbage == current_value + + // safety: need to guarantee that now_garbage is no longer + // reachable. more specifically, no thread that executes _after_ + // this line can ever get a reference to now_garbage. + // + // here are the possible cases: + // + // - another thread already has a reference to now_garbage. + // they must have read it before the call to swap. + // because of this, that thread must be pinned to an epoch <= + // the epoch of our guard. since the garbage is placed in our + // epoch, it won't be freed until the _next_ epoch, at which + // point, that thread must have dropped its guard, and with it, + // any reference to the value. + // - another thread is about to get a reference to this value. + // they execute _after_ the swap, and therefore do _not_ get a + // reference to now_garbage (they get value instead). there are + // no other ways to get to a value except through its Node's + // `value` field (which is what we swapped), so freeing + // now_garbage is fine. + unsafe { guard.defer_destroy(now_garbage) }; + // safety: since the value is present now, and we've held a guard from + // the beginning of the search, the value cannot be dropped until the + // next epoch, which won't arrive until after we drop our guard. + Some(unsafe { value.deref() }) + } else { + removed_node = true; + // remove the BinEntry::TreeNode containing the removed key value pair from the bucket + // also drop the old value stored in the tree node, as it was removed from the map + // safety: `p` and its value are either marked for garbage collection in `remove_tree_node` + // directly, or we will `need_to_untreeify`. In the latter case, we `defer_destroy` + // both `p` and its value below, after storing the linear bin. Thus, everything is + // always marked for garbage collection _after_ it becomes unaccessible by other threads. + let need_to_untreeify = + unsafe { tree_bin.remove_tree_node(p, true, guard) }; + if need_to_untreeify { + let linear_bin = Self::untreeify( + tree_bin.first.load(Ordering::SeqCst, guard), + guard, + ); + t.store_bin(bini, linear_bin); + // the old bin is now garbage, but its values are not, + // since they are re-used in the linear bin. + // safety: in the same way as for `now_garbage` above, any existing + // references to `bin` must have been obtained before storing the + // linear bin. These references were obtained while pinning an epoch + // <= our epoch and have to be dropped before the epoch can advance + // past the destruction of the old bin. After the store, threads will + // always see the linear bin, so the cannot obtain new references either. + // + // The same holds for `p` and its value, which does not get dropped together + // with `bin` here since `remove_tree_node` indicated that the bin needs to + // be untreeified. + unsafe { + TreeBin::defer_drop_without_values(bin, guard); + guard.defer_destroy(p); + guard.defer_destroy(current_value); + } + } + None + } + } + }; + drop(bin_lock); } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), } + // NOTE: the Java code checks `bin_count` here because they also + // reach this point if the bin changed while obtaining the lock. + // However, our code doesn't (it uses continue) and making this + // break conditional confuses the compiler about how many times we + // use the `remapping_function`. + debug_assert_ne!(bin_count, 0); + break; } + if removed_node { + // decrement count + self.add_count(-1, Some(bin_count), guard); + } + guard.flush(); + new_val } /// Removes a key-value pair from the map, and returns the removed value (if any). /// /// The key may be any borrowed form of the map's key type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the key type. /// - /// [`Eq`]: std::cmp::Eq + /// [`Ord`]: std::cmp::Ord /// [`Hash`]: std::hash::Hash /// /// # Examples @@ -1908,7 +2357,7 @@ where pub fn remove<'g, Q>(&'g self, key: &Q, guard: &'g Guard) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { // NOTE: _technically_, this method shouldn't require the thread-safety bounds, but a) that // would require special-casing replace_node for when new_value.is_none(), and b) it's sort @@ -1921,11 +2370,11 @@ where /// key was previously in the map. /// /// The key may be any borrowed form of the map's key type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the key type. /// - /// [`Eq`]: ../../std/cmp/trait.Eq.html - /// [`Hash`]: ../../std/hash/trait.Hash.html + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash /// /// # Examples /// @@ -1941,13 +2390,16 @@ where pub fn remove_entry<'g, Q>(&'g self, key: &Q, guard: &'g Guard) -> Option<(&'g K, &'g V)> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.check_guard(guard); self.replace_node(key, None, None, guard) } - /// Replaces the node value for the given key with `v`, if is equal to `cv`. + /// Replaces node value with `new_value`. + /// + /// If an `observed_value` is provided, the replacement only happens if `observed_value` equals + /// the value that is currently associated with the given key. /// /// If `new_value` is `None`, it removes the key (and its corresponding value) from this map. /// @@ -1955,8 +2407,12 @@ where /// /// Returns the previous key and value associated with the given key. /// - /// The key may be any borrowed form of the map's key type, but `Hash` and `Eq` on the borrowed - /// form must match those for the key type. + /// The key may be any borrowed form of the map's key type, but + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for + /// the key type. + /// + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash fn replace_node<'g, Q>( &'g self, key: &Q, @@ -1966,12 +2422,13 @@ where ) -> Option<(&'g K, &'g V)> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { let hash = self.hash(key); + let is_remove = new_value.is_none(); + let mut old_val = None; let mut table = self.table.load(Ordering::SeqCst, guard); - loop { if table.is_null() { break; @@ -2001,34 +2458,35 @@ where // safety: bin is a valid pointer. // - // there are two cases when a bin pointer is invalidated: + // there are three cases when a bin pointer is invalidated: // // 1. if the table was resized, bin is a move entry, and the resize has completed. in // that case, the table (and all its heads) will be dropped in the next epoch // following that. // 2. if the table is being resized, bin may be swapped with a move entry. the old bin // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. // - // in both cases, we held the guard when we got the reference to the bin. if any such + // in all cases, we held the guard when we got the reference to the bin. if any such // swap happened, it must have happened _after_ we read. since we did the read while // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we // are holding up by holding on to our guard). match *unsafe { bin.deref() } { BinEntry::Moved => { table = self.help_transfer(table, guard); + continue; } BinEntry::Node(ref head) => { let head_lock = head.lock.lock(); - let mut old_val: Option<(&'g K, Shared<'g, V>)> = None; // need to check that this is _still_ the head if t.bin(bini, guard) != bin { continue; } - // TODO: tree nodes let mut e = bin; - let is_remove = new_value.is_none(); let mut pred: Shared<'_, BinEntry> = Shared::null(); loop { // safety: either e is bin, in which case it is valid due to the above, @@ -2044,7 +2502,7 @@ where if n.hash == hash && n.key.borrow() == key { let ev = n.value.load(Ordering::SeqCst, guard); - // just remove the node if the value is the one we expected at method call + // only replace the node if the value is the one we expected at method call if observed_value.map(|ov| ov == ev).unwrap_or(true) { // we remember the old value so that we can return it and mark it for deletion below old_val = Some((&n.key, ev)); @@ -2085,40 +2543,115 @@ where } } drop(head_lock); + } + BinEntry::Tree(ref tree_bin) => { + let bin_lock = tree_bin.lock.lock(); - if let Some((key, val)) = old_val { - if is_remove { - self.add_count(-1, None, guard); - } + // need to check that this is _still_ the head + if t.bin(bini, guard) != bin { + continue; + } - // safety: need to guarantee that the old value is no longer - // reachable. more specifically, no thread that executes _after_ - // this line can ever get a reference to val. - // - // here are the possible cases: - // - // - another thread already has a reference to the old value. - // they must have read it before the call to store_bin. - // because of this, that thread must be pinned to an epoch <= - // the epoch of our guard. since the garbage is placed in our - // epoch, it won't be freed until the _next_ epoch, at which - // point, that thread must have dropped its guard, and with it, - // any reference to the value. - // - another thread is about to get a reference to this value. - // they execute _after_ the store_bin, and therefore do _not_ get a - // reference to the old value. there are no other ways to get to a - // value except through its Node's `value` field (which is now gone - // together with the node), so freeing the old value is fine. - unsafe { guard.defer_destroy(val) }; - - // safety: the lifetime of the reference is bound to the guard - // supplied which means that the memory will not be freed - // until at least after the guard goes out of scope - return unsafe { val.as_ref() }.map(move |v| (key, v)); + let root = tree_bin.root.load(Ordering::SeqCst, guard); + if root.is_null() { + // we are in the correct bin for the given key's hash and the bin is not + // `Moved`, but it is empty. This means there is no node for the given + // key that could be replaced + // TODO: Is this even reachable? + // The Java code has `NULL` checks for this, but in theory it should not be possible to + // encounter a tree that has no root when we have its lock. It should always have at + // least `UNTREEIFY_THRESHOLD` nodes. If it is indeed impossible we should replace + // this with an assertion instead. + break; } - break; + let p = TreeNode::find_tree_node(root, hash, key, guard); + if p.is_null() { + // similarly to the above, there now are entries in this bin, but none of + // them matches the given key + break; + } + // safety: the TreeBin was read under our guard, + // at which point the tree structure was valid. + // Since our guard pins the current epoch, the + // TreeNodes and `p` in particular remain valid + // for at least as long as we hold onto the + // guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let n = &unsafe { TreeNode::get_tree_node(p) }.node; + let pv = n.value.load(Ordering::SeqCst, guard); + + // only replace the node if the value is the one we expected at method call + if observed_value.map(|ov| ov == pv).unwrap_or(true) { + // we remember the old value so that we can return it and mark it for deletion below + old_val = Some((&n.key, pv)); + + if let Some(nv) = new_value { + // found the node but we have a new value to replace the old one + n.value.store(Owned::new(nv), Ordering::SeqCst); + } else { + // drop `p` without its value, since the old value is dropped + // in the check on `old_val` below + // safety: `p` is either marked for garbage collection in `remove_tree_node` directly, + // or we will `need_to_untreeify`. In the latter case, we `defer_destroy` `p` below, + // after storing the linear bin. The value stored in `p` is `defer_destroy`ed from within + // `old_val` at the end of the method. Thus, everything is always marked for garbage + // collection _after_ it becomes unaccessible by other threads. + let need_to_untreeify = + unsafe { tree_bin.remove_tree_node(p, false, guard) }; + if need_to_untreeify { + let linear_bin = Self::untreeify( + tree_bin.first.load(Ordering::SeqCst, guard), + guard, + ); + t.store_bin(bini, linear_bin); + // the old bin is now garbage, but its values are not, + // since they get re-used in the linear bin + // safety: same as in put + unsafe { + TreeBin::defer_drop_without_values(bin, guard); + guard.defer_destroy(p); + } + } + } + } + + drop(bin_lock); } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), } + if let Some((key, val)) = old_val { + if is_remove { + self.add_count(-1, None, guard); + } + + // safety: need to guarantee that the old value is no longer + // reachable. more specifically, no thread that executes _after_ + // this line can ever get a reference to val. + // + // here are the possible cases: + // + // - another thread already has a reference to the old value. + // they must have read it before the call to store_bin. + // because of this, that thread must be pinned to an epoch <= + // the epoch of our guard. since the garbage is placed in our + // epoch, it won't be freed until the _next_ epoch, at which + // point, that thread must have dropped its guard, and with it, + // any reference to the value. + // - another thread is about to get a reference to this value. + // they execute _after_ the store_bin, and therefore do _not_ get a + // reference to the old value. there are no other ways to get to a + // value except through its Node's `value` field (which is now gone + // together with the node), so freeing the old value is fine. + unsafe { guard.defer_destroy(val) }; + + // safety: the lifetime of the reference is bound to the guard + // supplied which means that the memory will not be freed + // until at least after the guard goes out of scope + return unsafe { val.as_ref() }.map(move |v| (key, v)); + } + break; } None } @@ -2194,9 +2727,170 @@ where } } +impl HashMap +where + K: Clone + Ord, +{ + /// Replaces all linked nodes in the bin at the given index unless the table + /// is too small, in which case a resize is initiated instead. + fn treeify_bin<'g>(&'g self, tab: &Table, index: usize, guard: &'g Guard) { + let n = tab.len(); + if n < MIN_TREEIFY_CAPACITY { + self.try_presize(n << 1, guard); + } else { + let bin = tab.bin(index, guard); + if bin.is_null() { + return; + } + // safety: we loaded `bin` while the epoch was pinned by our + // guard. if the bin was replaced since then, the old bin still + // won't be dropped until after we release our guard. + if let BinEntry::Node(ref node) = unsafe { bin.deref() } { + let lock = node.lock.lock(); + // check if `bin` is still the head + if tab.bin(index, guard) != bin { + return; + } + let mut e = bin; + let mut head = Shared::null(); + let mut tail = Shared::null(); + while !e.is_null() { + // safety: either `e` is `bin`, in which case it is + // valid due to the above, or `e` was obtained from + // a next pointer. Any next pointer obtained from + // bin is valid at the time we look up bin in the + // table, at which point the epoch is pinned by our + // guard. Since we found the next pointer in a valid + // map and it is not null (as checked above), the + // node it points to was present (i.e. not removed) + // from the map in the current epoch. Thus, because + // the epoch cannot advance until we release our + // guard, `e` is also valid if it was obtained from + // a next pointer. + let e_deref = unsafe { e.deref() }.as_node().unwrap(); + // NOTE: cloning the value uses a load with Ordering::Relaxed, but + // write access is synchronized through the bin lock + let new_tree_node = TreeNode::new( + e_deref.hash, + e_deref.key.clone(), + e_deref.value.clone(), + Atomic::null(), + Atomic::null(), + ); + new_tree_node.prev.store(tail, Ordering::Relaxed); + let new_tree_node = + Owned::new(BinEntry::TreeNode(new_tree_node)).into_shared(guard); + if tail.is_null() { + // this was the first TreeNode, so it becomes the head + head = new_tree_node; + } else { + // safety: if `tail` is not `null`, we have just created + // it in the last iteration, thus the pointer is valid + unsafe { tail.deref() } + .as_tree_node() + .unwrap() + .node + .next + .store(new_tree_node, Ordering::Relaxed); + } + tail = new_tree_node; + e = e_deref.next.load(Ordering::SeqCst, guard); + } + tab.store_bin( + index, + Owned::new(BinEntry::Tree(TreeBin::new( + // safety: we have just created `head` and its `next` + // nodes and have never shared them + unsafe { head.into_owned() }, + guard, + ))), + ); + drop(lock); + // make sure the old bin entries get dropped + e = bin; + while !e.is_null() { + // safety: we just replaced the bin containing this + // BinEntry, making it unreachable for other threads + // since subsequent loads will see the new bin. + // Threads with existing references to `e` must have + // obtained them in this or an earlier epoch, and + // this epoch is pinned by our guard. Thus, `e` will + // only be dropped after these threads release their + // guard, at which point they can no longer hold + // their reference to `e`. + // The BinEntry pointers are valid to deref for the + // same reason as above. + // + // NOTE: we do not drop the value, since it gets + // moved to the new TreeNode + unsafe { + guard.defer_destroy(e); + e = e + .deref() + .as_node() + .unwrap() + .next + .load(Ordering::SeqCst, guard); + } + } + } else { + match unsafe { bin.deref() } { + BinEntry::Node(_) => unreachable!("Node is the regular `if` case"), + BinEntry::Moved => unreachable!("treeifying a Moved entry"), + BinEntry::Tree(_) => unreachable!("treeifying a Tree"), + BinEntry::TreeNode(_) => unreachable!("treeifying a TreeBin"), + }; + } + } + } + + /// Returns a list of non-TreeNodes replacing those in the given list. Does + /// _not_ clean up old TreeNodes, as they may still be reachable. + fn untreeify<'g>( + bin: Shared<'g, BinEntry>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + let mut head = Shared::null(); + let mut tail: Shared<'_, BinEntry> = Shared::null(); + let mut q = bin; + while !q.is_null() { + // safety: we only untreeify sequences of TreeNodes which either + // - were just created (e.g. in transfer) and are thus valid, or + // - are read from a TreeBin loaded from the map. In this case, + // the bin gets loaded under our guard and at that point all + // of its nodes (its `first` and all `next` nodes) are valid. + // As `q` is not `null` (checked above), this means that `q` + // remains a valid pointer at least until our guard is dropped. + let q_deref = unsafe { q.deref() }.as_tree_node().unwrap(); + // NOTE: cloning the value uses a load with Ordering::Relaxed, but + // write access is synchronized through the bin lock + let new_node = Owned::new(BinEntry::Node(Node::new( + q_deref.node.hash, + q_deref.node.key.clone(), + q_deref.node.value.clone(), + ))) + .into_shared(guard); + if tail.is_null() { + head = new_node; + } else { + // safety: if `tail` is not `null`, we have just created it + // in the last iteration, thus the pointer is valid + unsafe { tail.deref() } + .as_node() + .unwrap() + .next + .store(new_node, Ordering::Relaxed); + } + tail = new_node; + q = q_deref.node.next.load(Ordering::Relaxed, guard); + } + + head + } +} impl PartialEq for HashMap where - K: Eq + Hash, + K: Ord + Hash, V: PartialEq, S: BuildHasher, { @@ -2210,7 +2904,7 @@ where impl Eq for HashMap where - K: Eq + Hash, + K: Ord + Hash, V: Eq, S: BuildHasher, { @@ -2253,7 +2947,7 @@ impl Drop for HashMap { impl Extend<(K, V)> for &HashMap where - K: 'static + Sync + Send + Clone + Hash + Eq, + K: 'static + Sync + Send + Clone + Hash + Ord, V: 'static + Sync + Send, S: BuildHasher, { @@ -2278,7 +2972,7 @@ where impl<'a, K, V, S> Extend<(&'a K, &'a V)> for &HashMap where - K: 'static + Sync + Send + Copy + Hash + Eq, + K: 'static + Sync + Send + Copy + Hash + Ord, V: 'static + Sync + Send + Copy, S: BuildHasher, { @@ -2289,7 +2983,7 @@ where impl FromIterator<(K, V)> for HashMap where - K: 'static + Sync + Send + Clone + Hash + Eq, + K: 'static + Sync + Send + Clone + Hash + Ord, V: 'static + Sync + Send, S: BuildHasher + Default, { @@ -2315,7 +3009,7 @@ where impl<'a, K, V, S> FromIterator<(&'a K, &'a V)> for HashMap where - K: 'static + Sync + Send + Copy + Hash + Eq, + K: 'static + Sync + Send + Copy + Hash + Ord, V: 'static + Sync + Send + Copy, S: BuildHasher + Default, { @@ -2326,7 +3020,7 @@ where impl<'a, K, V, S> FromIterator<&'a (K, V)> for HashMap where - K: 'static + Sync + Send + Copy + Hash + Eq, + K: 'static + Sync + Send + Copy + Hash + Ord, V: 'static + Sync + Send + Copy, S: BuildHasher + Default, { @@ -2337,7 +3031,7 @@ where impl Clone for HashMap where - K: 'static + Sync + Send + Clone + Hash + Eq, + K: 'static + Sync + Send + Clone + Hash + Ord, V: 'static + Sync + Send + Clone, S: BuildHasher + Clone, { @@ -2639,22 +3333,199 @@ fn replace_twice() { } #[cfg(test)] -#[test] -#[should_panic] -fn disallow_evil() { - let map: HashMap<_, _> = HashMap::default(); - map.insert(42, String::from("hello"), &crossbeam_epoch::pin()); - - let evil = crossbeam_epoch::Collector::new(); - let evil = evil.register(); - let guard = evil.pin(); - let oops = map.get(&42, &guard); - - map.remove(&42, &crossbeam_epoch::pin()); - // at this point, the default collector is allowed to free `"hello"` - // since no-one has the global epoch pinned as far as it is aware. - // `oops` is tied to the lifetime of a Guard that is not a part of - // the same epoch group, and so can now be dangling. - // but we can still access it! - assert_eq!(oops.unwrap(), "hello"); +mod tree_bins { + use super::*; + + // Tests for the tree bin optimization. + // Includes testing that bins are actually treeified and untreeified, and that, when tree bins + // are untreeified, the associated values remain in the map. + + #[derive(Default)] + struct ZeroHasher; + struct ZeroHashBuilder; + impl Hasher for ZeroHasher { + fn finish(&self) -> u64 { + 0 + } + fn write(&mut self, _: &[u8]) {} + } + impl BuildHasher for ZeroHashBuilder { + type Hasher = ZeroHasher; + fn build_hasher(&self) -> ZeroHasher { + ZeroHasher + } + } + + #[test] + fn concurrent_tree_bin() { + let map = HashMap::::with_hasher(ZeroHashBuilder); + // first, ensure that we have a tree bin + { + let guard = &map.guard(); + // Force creation of a tree bin by inserting enough values that hash to 0 + for i in 0..10 { + map.insert(i, i, guard); + } + // Ensure the bin was correctly treeified + let t = map.table.load(Ordering::Relaxed, guard); + let t = unsafe { t.deref() }; + let bini = t.bini(0); + let bin = t.bin(bini, guard); + match unsafe { bin.deref() } { + BinEntry::Tree(_) => {} // pass + BinEntry::Moved => panic!("bin was not correctly treeified -- is Moved"), + BinEntry::Node(_) => panic!("bin was not correctly treeified -- is Node"), + BinEntry::TreeNode(_) => panic!("bin was not correctly treeified -- is TreeNode"), + } + + guard.flush(); + drop(guard); + } + // then, spin up lots of reading and writing threads on a range of keys + const NUM_WRITERS: usize = 5; + const NUM_READERS: usize = 20; + const NUM_REPEATS: usize = 1000; + const NUM_KEYS: usize = 1000; + use rand::{ + distributions::{Distribution, Uniform}, + thread_rng, + }; + let uniform = Uniform::new(0, NUM_KEYS); + let m = std::sync::Arc::new(map); + + let mut handles = Vec::with_capacity(2 * NUM_WRITERS + NUM_READERS); + for _ in 0..NUM_READERS { + // ...and a reading thread + let map = m.clone(); + handles.push(std::thread::spawn(move || { + let guard = &map.guard(); + let mut trng = thread_rng(); + for _ in 0..NUM_REPEATS { + let key = uniform.sample(&mut trng); + if let Some(v) = map.get(&key, guard) { + criterion::black_box(v); + } + } + })); + } + for i in 0..NUM_WRITERS { + // NUM_WRITERS times, create a writing thread... + let map = m.clone(); + handles.push(std::thread::spawn(move || { + let guard = &map.guard(); + let mut trng = thread_rng(); + for _ in 0..NUM_REPEATS { + let key = uniform.sample(&mut trng); + map.insert(key, i, guard); + } + })); + // ...a removing thread. + let map = m.clone(); + handles.push(std::thread::spawn(move || { + let guard = &map.guard(); + let mut trng = thread_rng(); + for _ in 0..NUM_REPEATS { + let key = uniform.sample(&mut trng); + if let Some(v) = map.remove(&key, guard) { + criterion::black_box(v); + } + } + })); + } + + // in the end, join all threads + for handle in handles { + handle.join().unwrap(); + } + } + + #[test] + fn untreeify_shared_values_remove() { + test_tree_bin_remove(|i, map, guard| { + assert_eq!(map.remove(&i, guard), Some(&i)); + }); + } + + #[test] + fn untreeify_shared_values_compute_if_present() { + test_tree_bin_remove(|i, map, guard| { + assert_eq!(map.compute_if_present(&i, |_, _| None, guard), None); + }); + } + + fn test_tree_bin_remove(f: F) + where + F: Fn(usize, &HashMap, &Guard), + { + let map = HashMap::::with_hasher(ZeroHashBuilder); + { + let guard = &map.guard(); + // Force creation of a tree bin by inserting enough values that hash to 0 + for i in 0..10 { + map.insert(i, i, guard); + } + // Ensure the bin was correctly treeified + let t = map.table.load(Ordering::Relaxed, guard); + let t = unsafe { t.deref() }; + let bini = t.bini(0); + let bin = t.bin(bini, guard); + match unsafe { bin.deref() } { + BinEntry::Tree(_) => {} // pass + BinEntry::Moved => panic!("bin was not correctly treeified -- is Moved"), + BinEntry::Node(_) => panic!("bin was not correctly treeified -- is Node"), + BinEntry::TreeNode(_) => panic!("bin was not correctly treeified -- is TreeNode"), + } + + // Delete keys to force untreeifying the bin + for i in 0..9 { + f(i, &map, guard); + } + guard.flush(); + drop(guard); + } + assert_eq!(map.len(), 1); + + { + // Ensure the bin was correctly untreeified + let guard = &map.guard(); + let t = map.table.load(Ordering::Relaxed, guard); + let t = unsafe { t.deref() }; + let bini = t.bini(0); + let bin = t.bin(bini, guard); + match unsafe { bin.deref() } { + BinEntry::Tree(_) => panic!("bin was not correctly untreeified -- is Tree"), + BinEntry::Moved => panic!("bin was not correctly untreeified -- is Moved"), + BinEntry::Node(_) => {} // pass + BinEntry::TreeNode(_) => panic!("bin was not correctly untreeified -- is TreeNode"), + } + } + + // Create some guards to more reliably trigger garbage collection + for _ in 0..10 { + let _ = map.guard(); + } + + // Access a value that should still be in the map + let guard = &map.guard(); + assert_eq!(map.get(&9, guard), Some(&9)); + } + #[test] + #[should_panic] + fn disallow_evil() { + let map: HashMap<_, _> = HashMap::default(); + map.insert(42, String::from("hello"), &crossbeam_epoch::pin()); + + let evil = crossbeam_epoch::Collector::new(); + let evil = evil.register(); + let guard = evil.pin(); + let oops = map.get(&42, &guard); + + map.remove(&42, &crossbeam_epoch::pin()); + // at this point, the default collector is allowed to free `"hello"` + // since no-one has the global epoch pinned as far as it is aware. + // `oops` is tied to the lifetime of a Guard that is not a part of + // the same epoch group, and so can now be dangling. + // but we can still access it! + assert_eq!(oops.unwrap(), "hello"); + } } diff --git a/src/map_ref.rs b/src/map_ref.rs index 31a9e3c0..673f35a6 100644 --- a/src/map_ref.rs +++ b/src/map_ref.rs @@ -81,7 +81,7 @@ impl HashMapRef<'_, K, V, S> { impl HashMapRef<'_, K, V, S> where - K: Clone, + K: Clone + Ord, { /// Tries to reserve capacity for at least `additional` more elements to be inserted in the /// `HashMap`. @@ -96,7 +96,7 @@ where impl HashMapRef<'_, K, V, S> where - K: Hash + Eq, + K: Hash + Ord, S: BuildHasher, { /// Returns `true` if the map contains a value for the specified key. @@ -105,7 +105,7 @@ where pub fn contains_key(&self, key: &Q) -> bool where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.contains_key(key, &self.guard) } @@ -117,7 +117,7 @@ where pub fn get<'g, Q>(&'g self, key: &Q) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.get(key, &self.guard) } @@ -129,7 +129,7 @@ where pub fn get_key_value<'g, Q>(&'g self, key: &Q) -> Option<(&'g K, &'g V)> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.get_key_value(key, &self.guard) } @@ -137,7 +137,7 @@ where impl HashMapRef<'_, K, V, S> where - K: Clone, + K: Clone + Ord, { /// Clears the map, removing all key-value pairs. /// @@ -149,7 +149,7 @@ where impl HashMapRef<'_, K, V, S> where - K: 'static + Sync + Send + Clone + Hash + Eq, + K: 'static + Sync + Send + Clone + Hash + Ord, V: 'static + Sync + Send, S: BuildHasher, { @@ -175,7 +175,7 @@ where pub fn compute_if_present<'g, Q, F>(&'g self, key: &Q, remapping_function: F) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, F: FnOnce(&K, &V) -> Option, { self.map @@ -188,7 +188,7 @@ where pub fn remove<'g, Q>(&'g self, key: &Q) -> Option<&'g V> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.remove(key, &self.guard) } @@ -200,7 +200,7 @@ where pub fn remove_entry<'g, Q>(&'g self, key: &Q) -> Option<(&'g K, &'g V)> where K: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.remove_entry(key, &self.guard) } @@ -253,7 +253,7 @@ impl Clone for HashMapRef<'_, K, V, S> { impl PartialEq for HashMapRef<'_, K, V, S> where - K: Hash + Eq, + K: Hash + Ord, V: PartialEq, S: BuildHasher, { @@ -264,7 +264,7 @@ where impl PartialEq> for HashMapRef<'_, K, V, S> where - K: Hash + Eq, + K: Hash + Ord, V: PartialEq, S: BuildHasher, { @@ -275,7 +275,7 @@ where impl PartialEq> for HashMap where - K: Hash + Eq, + K: Hash + Ord, V: PartialEq, S: BuildHasher, { @@ -286,7 +286,7 @@ where impl Eq for HashMapRef<'_, K, V, S> where - K: Hash + Eq, + K: Hash + Ord, V: Eq, S: BuildHasher, { @@ -294,8 +294,8 @@ where impl Index<&'_ Q> for HashMapRef<'_, K, V, S> where - K: Hash + Eq + Borrow, - Q: ?Sized + Hash + Eq, + K: Hash + Ord + Borrow, + Q: ?Sized + Hash + Ord, S: BuildHasher, { type Output = V; diff --git a/src/node.rs b/src/node.rs index d8715d11..76030a38 100644 --- a/src/node.rs +++ b/src/node.rs @@ -1,6 +1,9 @@ use crate::raw::Table; -use crossbeam_epoch::Atomic; +use core::sync::atomic::{spin_loop_hint, AtomicBool, AtomicI64, Ordering}; +use crossbeam_epoch::{Atomic, Guard, Owned, Shared}; use parking_lot::Mutex; +use std::borrow::Borrow; +use std::thread::{current, park, Thread}; /// Entry in a bin. /// @@ -8,6 +11,8 @@ use parking_lot::Mutex; #[derive(Debug)] pub(crate) enum BinEntry { Node(Node), + Tree(TreeBin), + TreeNode(TreeNode), Moved, } @@ -37,6 +42,21 @@ impl BinEntry { None } } + + pub(crate) fn as_tree_node(&self) -> Option<&TreeNode> { + if let BinEntry::TreeNode(ref n) = *self { + Some(n) + } else { + None + } + } + pub(crate) fn as_tree_bin(&self) -> Option<&TreeBin> { + if let BinEntry::Tree(ref bin) = *self { + Some(bin) + } else { + None + } + } } /// Key-value entry. @@ -49,6 +69,1373 @@ pub(crate) struct Node { pub(crate) lock: Mutex<()>, } +impl Node { + pub(crate) fn new(hash: u64, key: K, value: AV) -> Self + where + AV: Into>, + { + Node::with_next(hash, key, value, Atomic::null()) + } + pub(crate) fn with_next(hash: u64, key: K, value: AV, next: Atomic>) -> Self + where + AV: Into>, + { + Node { + hash, + key, + value: value.into(), + next, + lock: Mutex::new(()), + } + } +} + +/* ------------------------ TreeNodes ------------------------ */ + +/// Nodes for use in TreeBins. +#[derive(Debug)] +pub(crate) struct TreeNode { + // Node properties + pub(crate) node: Node, + + // red-black tree links + pub(crate) parent: Atomic>, + pub(crate) left: Atomic>, + pub(crate) right: Atomic>, + pub(crate) prev: Atomic>, // needed to unlink `next` upon deletion + pub(crate) red: AtomicBool, +} + +impl TreeNode { + /// Constructs a new TreeNode with the given attributes to be inserted into a TreeBin. + /// + /// This does yet not arrange this node and its `next` nodes into a tree, since the tree + /// structure is maintained globally by the TreeBin. + pub(crate) fn new( + hash: u64, + key: K, + value: Atomic, + next: Atomic>, + parent: Atomic>, + ) -> Self { + TreeNode { + node: Node::with_next(hash, key, value, next), + parent, + left: Atomic::null(), + right: Atomic::null(), + prev: Atomic::null(), + red: AtomicBool::new(false), + } + } + + /// Returns the `TreeNode` (or `Shared::null()` if not found) for the given + /// key, starting at the given node. + pub(crate) fn find_tree_node<'g, Q>( + from: Shared<'g, BinEntry>, + hash: u64, + key: &Q, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> + where + K: Borrow, + Q: ?Sized + Ord, + { + // NOTE: in the Java code, this method is implemented on the `TreeNode` + // instance directly, as they don't need to worry about shared pointers. + // The Java code then uses a do-while loop here, as `self`/`this` always + // exists so the condition below will always be satisfied on the first + // iteration. We however _do_ have shared pointers and _do not_ have + // do-while loops, so we end up with one extra check since we also need + // to introduce some `continue` due to the extraction of local + // assignments from checks. + let mut p = from; + while !p.is_null() { + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let p_deref = unsafe { Self::get_tree_node(p) }; + let p_hash = p_deref.node.hash; + + // first attempt to follow the tree order with the given hash + match p_hash.cmp(&hash) { + std::cmp::Ordering::Greater => { + p = p_deref.left.load(Ordering::SeqCst, guard); + continue; + } + std::cmp::Ordering::Less => { + p = p_deref.right.load(Ordering::SeqCst, guard); + continue; + } + _ => {} + } + + // if the hash matches, check if the given key also matches. If so, + // we have found the target node. + let p_key = &p_deref.node.key; + if p_key.borrow() == key { + return p; + } + + // If the key does not match, we need to descend down the tree. + let p_left = p_deref.left.load(Ordering::SeqCst, guard); + let p_right = p_deref.right.load(Ordering::SeqCst, guard); + // If one of the children is empty, there is only one child to check. + if p_left.is_null() { + p = p_right; + continue; + } else if p_right.is_null() { + p = p_left; + continue; + } + + // Otherwise, we compare keys to find the next child to look at. + p = match p_key.borrow().cmp(&key) { + std::cmp::Ordering::Greater => p_left, + std::cmp::Ordering::Less => p_right, + std::cmp::Ordering::Equal => { + unreachable!("Ord and Eq have to match and Eq is checked above") + } + }; + // NOTE: the Java code has some addional cases here in case the keys + // _are not_ equal (p_key != key and !key.equals(p_key)), but + // _compare_ equal (k.compareTo(p_key) == 0). In this case, both + // children are searched. Since `Eq` and `Ord` must match, these + // cases cannot occur here. + } + + Shared::null() + } +} + +const WRITER: i64 = 1; // set while holding write lock +const WAITER: i64 = 2; // set when waiting for write lock +const READER: i64 = 4; // increment value for setting read lock + +/// Private representation for movement direction along tree successors. +enum Dir { + Left, + Right, +} + +/// TreeNodes used at the heads of bins. TreeBins do not hold user keys or +/// values, but instead point to a list of TreeNodes and their root. They also +/// maintain a parasitic read-write lock forcing writers (who hold the bin lock) +/// to wait for readers (who do not) to complete before tree restructuring +/// operations. +#[derive(Debug)] +pub(crate) struct TreeBin { + pub(crate) root: Atomic>, + pub(crate) first: Atomic>, + pub(crate) waiter: Atomic, + pub(crate) lock: parking_lot::Mutex<()>, + pub(crate) lock_state: AtomicI64, +} + +impl TreeBin +where + K: Ord, +{ + /// Constructs a new bin from the given nodes. + /// + /// Nodes are arranged into an ordered red-black tree. + pub(crate) fn new(bin: Owned>, guard: &Guard) -> Self { + let mut root = Shared::null(); + let bin = bin.into_shared(guard); + + // safety: We own the nodes for creating this new TreeBin, so they are + // not shared with another thread and cannot get invalidated. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let mut x = bin; + while !x.is_null() { + let x_deref = unsafe { TreeNode::get_tree_node(x) }; + let next = x_deref.node.next.load(Ordering::Relaxed, guard); + x_deref.left.store(Shared::null(), Ordering::Relaxed); + x_deref.right.store(Shared::null(), Ordering::Relaxed); + + // if there is no root yet, make x the root + if root.is_null() { + x_deref.parent.store(Shared::null(), Ordering::Relaxed); + x_deref.red.store(false, Ordering::Relaxed); + root = x; + x = next; + continue; + } + + let key = &x_deref.node.key; + let hash = x_deref.node.hash; + + // Traverse the tree that was constructed so far from the root to + // find out where to insert x + let mut p = root; + loop { + let p_deref = unsafe { TreeNode::get_tree_node(p) }; + let p_key = &p_deref.node.key; + let p_hash = p_deref.node.hash; + + // Select successor of p in the correct direction. We will continue + // to descend the tree through this successor. + let xp = p; + let dir; + p = match p_hash.cmp(&hash).then(p_key.cmp(&key)) { + std::cmp::Ordering::Greater => { + dir = Dir::Left; + &p_deref.left + } + std::cmp::Ordering::Less => { + dir = Dir::Right; + &p_deref.right + } + std::cmp::Ordering::Equal => unreachable!("one key references two nodes"), + } + .load(Ordering::Relaxed, guard); + + if p.is_null() { + x_deref.parent.store(xp, Ordering::Relaxed); + match dir { + Dir::Left => { + unsafe { TreeNode::get_tree_node(xp) } + .left + .store(x, Ordering::Relaxed); + } + Dir::Right => { + unsafe { TreeNode::get_tree_node(xp) } + .right + .store(x, Ordering::Relaxed); + } + } + root = TreeNode::balance_insertion(root, x, guard); + break; + } + } + + x = next; + } + + if cfg!(debug_assertions) { + TreeNode::check_invariants(root, guard); + } + TreeBin { + root: Atomic::from(root), + first: Atomic::from(bin), + waiter: Atomic::null(), + lock: parking_lot::Mutex::new(()), + lock_state: AtomicI64::new(0), + } + } +} + +impl TreeBin { + /// Acquires write lock for tree restucturing. + fn lock_root(&self, guard: &Guard) { + if self + .lock_state + .compare_and_swap(0, WRITER, Ordering::SeqCst) + != 0 + { + // the current lock state is non-zero, which means the lock is contended + self.contended_lock(guard); + } + } + + /// Releases write lock for tree restructuring. + fn unlock_root(&self) { + self.lock_state.store(0, Ordering::Release); + } + + /// Possibly blocks awaiting root lock. + fn contended_lock(&self, guard: &Guard) { + let mut waiting = false; + let mut state: i64; + loop { + state = self.lock_state.load(Ordering::Acquire); + if state & !WAITER == 0 { + // there are no writing or reading threads + if self + .lock_state + .compare_and_swap(state, WRITER, Ordering::SeqCst) + == state + { + // we won the race for the lock and get to return from blocking + if waiting { + let waiter = self.waiter.swap(Shared::null(), Ordering::SeqCst, guard); + // safety: we are the only thread that modifies the + // `waiter` thread handle (reading threads only use it + // to notify us). Thus, having stored a valid value + // below, `waiter` is a valid pointer. + // The reading thread that notifies us does so as its + // last action in `find` and then lets go of the + // reference immediately. _New_ reading threads already + // take the slow path since we are `WAITING`, so they do + // not obtain new references to our thread handle. Also, + // we just swapped out that handle, so it is no longer + // reachable. + let _ = unsafe { waiter.into_owned() }; + } + return; + } + } else if state & WAITER == 0 { + // we have not indicated yet that we are waiting, so we need to + // do that now + if self + .lock_state + .compare_and_swap(state, state | WAITER, Ordering::SeqCst) + == state + { + waiting = true; + let current_thread = Owned::new(current()); + let waiter = self.waiter.swap(current_thread, Ordering::SeqCst, guard); + assert!(waiter.is_null()); + } + } else if waiting { + park(); + } + spin_loop_hint(); + } + } + + /// Returns matching node or `Shared::null()` if none. Tries to search using + /// tree comparisons from root, but continues linear search when lock not + /// available. + pub(crate) fn find<'g, Q>( + bin: Shared<'g, BinEntry>, + hash: u64, + key: &Q, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> + where + K: Borrow, + Q: ?Sized + Ord, + { + // safety: bin is a valid pointer. + // + // there are three cases when a bin pointer is invalidated: + // + // 1. if the table was resized, bin is a move entry, and the resize has completed. in + // that case, the table (and all its heads) will be dropped in the next epoch + // following that. + // 2. if the table is being resized, bin may be swapped with a move entry. the old bin + // will then be dropped in the following epoch after that happens. + // 3. when elements are inserted into or removed from the map, bin may be changed into + // or from a TreeBin from or into a regular, linear bin. the old bin will also be + // dropped in the following epoch if that happens. + // + // in all cases, we held the guard when we got the reference to the bin. if any such + // swap happened, it must have happened _after_ we read. since we did the read while + // pinning the epoch, the drop must happen in the _next_ epoch (i.e., the one that we + // are holding up by holding on to our guard). + let bin_deref = unsafe { bin.deref() }.as_tree_bin().unwrap(); + let mut element = bin_deref.first.load(Ordering::SeqCst, guard); + while !element.is_null() { + let s = bin_deref.lock_state.load(Ordering::SeqCst); + if s & (WAITER | WRITER) != 0 { + // another thread is modifying or wants to modify the tree + // (write). As long as that's the case, we follow the `next` + // pointers of the `TreeNode` linearly, as we cannot trust the + // tree's structure. + // + // safety: we were read under our guard, at which point the tree + // structure was valid. Since our guard pins the current epoch, + // the TreeNodes remain valid for at least as long as we hold + // onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let element_deref = unsafe { TreeNode::get_tree_node(element) }; + let element_key = &element_deref.node.key; + if element_deref.node.hash == hash && element_key.borrow() == key { + return element; + } + element = element_deref.node.next.load(Ordering::SeqCst, guard); + } else if bin_deref + .lock_state + .compare_and_swap(s, s + READER, Ordering::SeqCst) + == s + { + // the current lock state indicates no waiter or writer and we + // acquired a read lock + let root = bin_deref.root.load(Ordering::SeqCst, guard); + let p = if root.is_null() { + Shared::null() + } else { + TreeNode::find_tree_node(root, hash, key, guard) + }; + if bin_deref.lock_state.fetch_add(-READER, Ordering::SeqCst) == (READER | WAITER) { + // we were the last reader holding up a waiting writer, so + // we unpark the waiting writer by granting it a token + let waiter = &bin_deref.waiter.load(Ordering::SeqCst, guard); + if !waiter.is_null() { + // safety: thread handles are only dropped by the thread + // they represent _after_ it acquires the write lock. + // Since the thread behind the `waiter` handle is + // currently _waiting_ on said lock, the handle will not + // yet be dropped. + unsafe { waiter.deref() }.unpark(); + } + } + return p; + } + } + + Shared::null() + } + + /// Unlinks the given node, which must be present before this call. + /// + /// This is messier than typical red-black deletion code because we cannot + /// swap the contents of an interior node with a leaf successor that is + /// pinned by `next` pointers that are accessible independently of the bin + /// lock. So instead we swap the tree links. + /// + /// Returns `true` if the bin is now too small and should be untreeified. + /// + /// # Safety + /// The given node is only marked for garbage collection if the bin remains + /// large enough to be a `TreeBin`. If this method returns `true`, indicating + /// that the bin should be untreeified, the given node is only unlinked from + /// linear traversal, but not from the tree. This makes the node unreachable + /// through linear reads and excludes it from being dropped when the bin is + /// dropped. However, reading threads may still obtain a reference to until + /// the bin is swapped out for a linear bin. + /// + /// The caller of this method _must_ ensure that the given node is properly + /// marked for garbage collection _after_ this bin has been swapped out. If + /// the value of the given node was supposed to get dropped as well + /// (`drop_value` was true), the caller must do the same for the value. + pub(crate) unsafe fn remove_tree_node<'g>( + &'g self, + p: Shared<'g, BinEntry>, + drop_value: bool, + guard: &'g Guard, + ) -> bool { + // safety: we were read under our guard, at which point the tree + // structure was valid. Since our guard pins the current epoch, the + // TreeNodes remain valid for at least as long as we hold onto the + // guard. Additionally, this method assumes `p` to be non-null. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let p_deref = TreeNode::get_tree_node(p); + let next = p_deref.node.next.load(Ordering::SeqCst, guard); + let prev = p_deref.prev.load(Ordering::SeqCst, guard); + + // unlink traversal pointers + if prev.is_null() { + // the node to delete is the first node + self.first.store(next, Ordering::SeqCst); + } else { + TreeNode::get_tree_node(prev) + .node + .next + .store(next, Ordering::SeqCst); + } + if !next.is_null() { + TreeNode::get_tree_node(next) + .prev + .store(prev, Ordering::SeqCst); + } + + if self.first.load(Ordering::SeqCst, guard).is_null() { + // since the bin was not empty previously (it contained p), + // `self.first` is `null` only if we just stored `null` via `next`. + // In that case, we have removed the last node from this bin and + // don't have a tree anymore, so we reset `self.root`. + self.root.store(Shared::null(), Ordering::SeqCst); + return true; + } + + // if we are now too small to be a `TreeBin`, we don't need to worry + // about restructuring the tree since the bin will be untreeified + // anyway, so we check that + let mut root = self.root.load(Ordering::SeqCst, guard); + // TODO: Can `root` even be `null`? + // The Java code has `NULL` checks for this, but in theory it should not be possible to + // encounter a tree that has no root when we have its lock. It should always have at + // least `UNTREEIFY_THRESHOLD` nodes. If it is indeed impossible we should replace + // this with an assertion instead. + if root.is_null() + || TreeNode::get_tree_node(root) + .right + .load(Ordering::SeqCst, guard) + .is_null() + { + return true; + } else { + let root_left = TreeNode::get_tree_node(root) + .left + .load(Ordering::SeqCst, guard); + if root_left.is_null() + || TreeNode::get_tree_node(root_left) + .left + .load(Ordering::SeqCst, guard) + .is_null() + { + return true; + } + } + + // if we get here, we know that we will still be a tree and have + // unlinked the `next` and `prev` pointers, so it's time to restructure + // the tree + self.lock_root(guard); + // NOTE: since we have the write lock for the tree, we know that all + // readers will read along the linear `next` pointers until we release + // the lock (these pointers were adjusted above to exclude the removed + // node and are synchronized as `SeqCst`). This means that we can + // operate on the _other_ pointers of tree nodes that represent the tree + // structure using a `Relaxed` ordering. The release of the write lock + // will then synchronize with later readers who will see the new values. + let replacement; + let p_left = p_deref.left.load(Ordering::Relaxed, guard); + let p_right = p_deref.right.load(Ordering::Relaxed, guard); + if !p_left.is_null() && !p_right.is_null() { + // find the smalles successor of `p` + let mut successor = p_right; + let mut successor_deref = TreeNode::get_tree_node(successor); + let mut successor_left = successor_deref.left.load(Ordering::Relaxed, guard); + while !successor_left.is_null() { + successor = successor_left; + successor_deref = TreeNode::get_tree_node(successor); + successor_left = successor_deref.left.load(Ordering::Relaxed, guard); + } + // swap colors + let color = successor_deref.red.load(Ordering::Relaxed); + successor_deref + .red + .store(p_deref.red.load(Ordering::Relaxed), Ordering::Relaxed); + p_deref.red.store(color, Ordering::Relaxed); + + let successor_right = successor_deref.right.load(Ordering::Relaxed, guard); + let p_parent = p_deref.parent.load(Ordering::Relaxed, guard); + if successor == p_right { + // `p` was the direct parent of the smallest successor. + // the two nodes will be swapped + p_deref.parent.store(successor, Ordering::Relaxed); + successor_deref.right.store(p, Ordering::Relaxed); + } else { + let successor_parent = successor_deref.parent.load(Ordering::Relaxed, guard); + p_deref.parent.store(successor_parent, Ordering::Relaxed); + if !successor_parent.is_null() { + if successor + == TreeNode::get_tree_node(successor_parent) + .left + .load(Ordering::Relaxed, guard) + { + TreeNode::get_tree_node(successor_parent) + .left + .store(p, Ordering::Relaxed); + } else { + TreeNode::get_tree_node(successor_parent) + .right + .store(p, Ordering::Relaxed); + } + } + successor_deref.right.store(p_right, Ordering::Relaxed); + if !p_right.is_null() { + TreeNode::get_tree_node(p_right) + .parent + .store(successor, Ordering::Relaxed); + } + } + debug_assert!(successor_left.is_null()); + p_deref.left.store(Shared::null(), Ordering::Relaxed); + p_deref.right.store(successor_right, Ordering::Relaxed); + if !successor_right.is_null() { + TreeNode::get_tree_node(successor_right) + .parent + .store(p, Ordering::Relaxed); + } + successor_deref.left.store(p_left, Ordering::Relaxed); + if !p_left.is_null() { + TreeNode::get_tree_node(p_left) + .parent + .store(successor, Ordering::Relaxed); + } + successor_deref.parent.store(p_parent, Ordering::Relaxed); + if p_parent.is_null() { + // the successor was swapped to the root as `p` was previously the root + root = successor; + } else if p + == TreeNode::get_tree_node(p_parent) + .left + .load(Ordering::Relaxed, guard) + { + TreeNode::get_tree_node(p_parent) + .left + .store(successor, Ordering::Relaxed); + } else { + TreeNode::get_tree_node(p_parent) + .right + .store(successor, Ordering::Relaxed); + } + + // We have swapped `p` with `successor`, which is the next element + // after `p` in `(hash, key)` order (the smallest element larger + // than `p`). To actually remove `p`, we need to check if + // `successor` has a right child (it cannot have a left child, as + // otherwise _it_ would be the `successor`). If not, we can just + // directly unlink `p`, since it is now a leaf. Otherwise, we have + // to replace it with the right child of `successor` (which is now + // also its right child), which preserves the ordering. + if !successor_right.is_null() { + replacement = successor_right; + } else { + replacement = p; + } + } else if !p_left.is_null() { + // If `p` only has a left child, just replacing `p` with that child preserves the ordering. + replacement = p_left; + } else if !p_right.is_null() { + // Symmetrically, we can use its right child. + replacement = p_right; + } else { + // If `p` is _already_ a leaf, we can also just unlink it. + replacement = p; + } + + if replacement != p { + // `p` (at its potentially new position) has a child, so we need to do a replacement. + let p_parent = p_deref.parent.load(Ordering::Relaxed, guard); + TreeNode::get_tree_node(replacement) + .parent + .store(p_parent, Ordering::Relaxed); + if p_parent.is_null() { + root = replacement; + } else { + let p_parent_deref = TreeNode::get_tree_node(p_parent); + if p == p_parent_deref.left.load(Ordering::Relaxed, guard) { + p_parent_deref.left.store(replacement, Ordering::Relaxed); + } else { + p_parent_deref.right.store(replacement, Ordering::Relaxed); + } + } + p_deref.parent.store(Shared::null(), Ordering::Relaxed); + p_deref.right.store(Shared::null(), Ordering::Relaxed); + p_deref.left.store(Shared::null(), Ordering::Relaxed); + } + + self.root.store( + if p_deref.red.load(Ordering::Relaxed) { + root + } else { + TreeNode::balance_deletion(root, replacement, guard) + }, + Ordering::Relaxed, + ); + + if p == replacement { + // `p` does _not_ have children, so we unlink it as a leaf. + let p_parent = p_deref.parent.load(Ordering::Relaxed, guard); + if !p_parent.is_null() { + let p_parent_deref = TreeNode::get_tree_node(p_parent); + if p == p_parent_deref.left.load(Ordering::Relaxed, guard) { + TreeNode::get_tree_node(p_parent) + .left + .store(Shared::null(), Ordering::Relaxed); + } else if p == p_parent_deref.right.load(Ordering::Relaxed, guard) { + p_parent_deref + .right + .store(Shared::null(), Ordering::Relaxed); + } + p_deref.parent.store(Shared::null(), Ordering::Relaxed); + debug_assert!(p_deref.left.load(Ordering::Relaxed, guard).is_null()); + debug_assert!(p_deref.right.load(Ordering::Relaxed, guard).is_null()); + } + } + self.unlock_root(); + + // mark the old node and its value for garbage collection + // safety: we just completely unlinked `p` from both linear and tree + // traversal, making it and its value unreachable for any future thread. + // Any existing references to one of them were obtained under a guard + // that pins an epoch <= our epoch, and thus have to be released before + // `p` is actually dropped. + #[allow(unused_unsafe)] + unsafe { + if drop_value { + guard.defer_destroy(p_deref.node.value.load(Ordering::Relaxed, guard)); + } + guard.defer_destroy(p); + } + + if cfg!(debug_assertions) { + TreeNode::check_invariants(self.root.load(Ordering::SeqCst, guard), guard); + } + false + } +} + +impl TreeBin +where + K: Ord + Send + Sync, +{ + /// Finds or adds a node to the tree. + /// If a node for the given key already exists, it is returned. Otherwise, + /// returns `Shared::null()`. + pub(crate) fn find_or_put_tree_val<'g>( + &'g self, + hash: u64, + key: K, + value: Shared<'g, V>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + let mut p = self.root.load(Ordering::SeqCst, guard); + if p.is_null() { + // the current root is `null`, i.e. the tree is currently empty. + // This, we simply insert the new entry as the root. + let tree_node = Owned::new(BinEntry::TreeNode(TreeNode::new( + hash, + key, + Atomic::from(value), + Atomic::null(), + Atomic::null(), + ))) + .into_shared(guard); + self.root.store(tree_node, Ordering::Release); + self.first.store(tree_node, Ordering::Release); + return Shared::null(); + } + // safety: we were read under our guard, at which point the tree + // structure was valid. Since our guard pins the current epoch, the + // TreeNodes remain valid for at least as long as we hold onto the + // guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + loop { + let p_deref = unsafe { TreeNode::get_tree_node(p) }; + let p_hash = p_deref.node.hash; + let xp = p; + let dir; + p = match p_hash.cmp(&hash) { + std::cmp::Ordering::Greater => { + dir = Dir::Left; + &p_deref.left + } + std::cmp::Ordering::Less => { + dir = Dir::Right; + &p_deref.right + } + std::cmp::Ordering::Equal => { + let p_key = &p_deref.node.key; + if *p_key == key { + // a node with the given key already exists, so we return it + return p; + } + match p_key.cmp(&key) { + std::cmp::Ordering::Greater => { + dir = Dir::Left; + &p_deref.left + } + std::cmp::Ordering::Less => { + dir = Dir::Right; + &p_deref.right + } + std::cmp::Ordering::Equal => { + unreachable!("Ord and Eq have to match and Eq is checked above") + } + } + // NOTE: the Java code has some addional cases here in case the + // keys _are not_ equal (p_key != key and !key.equals(p_key)), + // but _compare_ equal (k.compareTo(p_key) == 0). In this case, + // both children are searched and if a matching node exists it + // is returned. Since `Eq` and `Ord` must match, these cases + // cannot occur here. + } + } + .load(Ordering::SeqCst, guard); + + if p.is_null() { + // we have reached a tree leaf, so the given key is not yet + // contained in the tree and we can add it at the correct + // position (which is here, since we arrived here by comparing + // hash and key of the new entry) + let first = self.first.load(Ordering::SeqCst, guard); + let x = Owned::new(BinEntry::TreeNode(TreeNode::new( + hash, + key, + Atomic::from(value), + Atomic::from(first), + Atomic::from(xp), + ))) + .into_shared(guard); + self.first.store(x, Ordering::SeqCst); + if !first.is_null() { + unsafe { TreeNode::get_tree_node(first) } + .prev + .store(x, Ordering::SeqCst); + } + match dir { + Dir::Left => { + unsafe { TreeNode::get_tree_node(xp) } + .left + .store(x, Ordering::SeqCst); + } + Dir::Right => { + unsafe { TreeNode::get_tree_node(xp) } + .right + .store(x, Ordering::SeqCst); + } + } + + if !unsafe { TreeNode::get_tree_node(xp) } + .red + .load(Ordering::SeqCst) + { + unsafe { TreeNode::get_tree_node(x) } + .red + .store(true, Ordering::SeqCst); + } else { + self.lock_root(guard); + self.root.store( + TreeNode::balance_insertion( + self.root.load(Ordering::Relaxed, guard), + x, + guard, + ), + Ordering::Relaxed, + ); + self.unlock_root(); + } + break; + } + } + + if cfg!(debug_assertions) { + TreeNode::check_invariants(self.root.load(Ordering::SeqCst, guard), guard); + } + Shared::null() + } +} + +impl Drop for TreeBin { + fn drop(&mut self) { + // safety: we have &mut self _and_ all references we have returned are bound to the + // lifetime of their borrow of self, so there cannot be any outstanding references to + // anything in the map. + unsafe { self.drop_fields(true) }; + } +} + +impl TreeBin { + /// Defers dropping the given tree bin without its nodes' values. + /// + /// # Safety + /// The given bin must be a valid, non-null BinEntry::TreeBin and the caller must ensure + /// that no references to the bin can be obtained by other threads after the call to this + /// method. + pub(crate) unsafe fn defer_drop_without_values<'g>( + bin: Shared<'g, BinEntry>, + guard: &'g Guard, + ) { + guard.defer_unchecked(move || { + if let BinEntry::Tree(mut tree_bin) = *bin.into_owned().into_box() { + tree_bin.drop_fields(false); + } else { + unreachable!("bin is a tree bin"); + } + }); + } + + /// Drops the given tree bin, but only drops its nodes' values when specified. + /// + /// # Safety + /// The pointer to the tree bin must be valid and the caller must be the single owner + /// of the tree bin. If the nodes' values are to be dropped, there must be no outstanding + /// references to these values in other threads and it must be impossible to obtain them. + pub(crate) unsafe fn drop_fields(&mut self, drop_values: bool) { + // assume ownership of atomically shared references. note that it is + // sufficient to follow the `next` pointers of the `first` element in + // the bin, since the tree pointers point to the same nodes. + + // swap out first pointer so nodes will not get dropped again when + // `tree_bin` is dropped + let guard = crossbeam_epoch::unprotected(); + let p = self.first.swap(Shared::null(), Ordering::Relaxed, guard); + Self::drop_tree_nodes(p, drop_values, guard); + } + + /// Drops the given list of tree nodes, but only drops their values when specified. + /// + /// # Safety + /// The pointers to the tree nodes must be valid and the caller must be the single owner + /// of the tree nodes. If the nodes' values are to be dropped, there must be no outstanding + /// references to these values in other threads and it must be impossible to obtain them. + pub(crate) unsafe fn drop_tree_nodes<'g>( + from: Shared<'g, BinEntry>, + drop_values: bool, + guard: &'g Guard, + ) { + let mut p = from; + while !p.is_null() { + if let BinEntry::TreeNode(tree_node) = *p.into_owned().into_box() { + // if specified, drop the value in this node + if drop_values { + let _ = tree_node.node.value.into_owned(); + } + // then we move to the next node + p = tree_node.node.next.load(Ordering::SeqCst, guard); + } else { + unreachable!("Trees can only ever contain TreeNodes"); + }; + } + } +} + +/* Helper impls to avoid code explosion */ +impl TreeNode { + /// Gets the `BinEntry::TreeNode(tree_node)` behind the given pointer and + /// returns its `tree_node`. + /// + /// # Safety + /// All safety concerns of [`deref`](Shared::deref) apply. In particular, the + /// supplied pointer must be non-null and must point to valid memory. + /// Additionally, it must point to an instance of BinEntry that is actually a + /// TreeNode. + #[inline] + pub(crate) unsafe fn get_tree_node<'g>(bin: Shared<'g, BinEntry>) -> &'g TreeNode { + bin.deref().as_tree_node().unwrap() + } +} + +/* ----------------------------------------------------------------- */ + +macro_rules! treenode { + ($pointer:ident) => { + unsafe { Self::get_tree_node($pointer) } + }; +} +// Red-black tree methods, all adapted from CLR +impl TreeNode { + // NOTE: these functions can be executed only when creating a new TreeBin or + // while holding the `write_lock`. Thus, we can use `Relaxed` memory + // operations everywhere. + fn rotate_left<'g>( + mut root: Shared<'g, BinEntry>, + p: Shared<'g, BinEntry>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + if p.is_null() { + return root; + } + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let p_deref = treenode!(p); + let right = p_deref.right.load(Ordering::Relaxed, guard); + if right.is_null() { + // there is no right successor to rotate left + return root; + } + let right_deref = treenode!(right); + let right_left = right_deref.left.load(Ordering::Relaxed, guard); + p_deref.right.store(right_left, Ordering::Relaxed); + if !right_left.is_null() { + treenode!(right_left).parent.store(p, Ordering::Relaxed); + } + + let p_parent = p_deref.parent.load(Ordering::Relaxed, guard); + right_deref.parent.store(p_parent, Ordering::Relaxed); + if p_parent.is_null() { + root = right; + right_deref.red.store(false, Ordering::Relaxed); + } else { + let p_parent_deref = treenode!(p_parent); + if p_parent_deref.left.load(Ordering::Relaxed, guard) == p { + p_parent_deref.left.store(right, Ordering::Relaxed); + } else { + p_parent_deref.right.store(right, Ordering::Relaxed); + } + } + right_deref.left.store(p, Ordering::Relaxed); + p_deref.parent.store(right, Ordering::Relaxed); + + root + } + + fn rotate_right<'g>( + mut root: Shared<'g, BinEntry>, + p: Shared<'g, BinEntry>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + if p.is_null() { + return root; + } + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let p_deref = treenode!(p); + let left = p_deref.left.load(Ordering::Relaxed, guard); + if left.is_null() { + // there is no left successor to rotate right + return root; + } + let left_deref = treenode!(left); + let left_right = left_deref.right.load(Ordering::Relaxed, guard); + p_deref.left.store(left_right, Ordering::Relaxed); + if !left_right.is_null() { + treenode!(left_right).parent.store(p, Ordering::Relaxed); + } + + let p_parent = p_deref.parent.load(Ordering::Relaxed, guard); + left_deref.parent.store(p_parent, Ordering::Relaxed); + if p_parent.is_null() { + root = left; + left_deref.red.store(false, Ordering::Relaxed); + } else { + let p_parent_deref = treenode!(p_parent); + if p_parent_deref.right.load(Ordering::Relaxed, guard) == p { + p_parent_deref.right.store(left, Ordering::Relaxed); + } else { + p_parent_deref.left.store(left, Ordering::Relaxed); + } + } + left_deref.right.store(p, Ordering::Relaxed); + p_deref.parent.store(left, Ordering::Relaxed); + + root + } + + fn balance_insertion<'g>( + mut root: Shared<'g, BinEntry>, + mut x: Shared<'g, BinEntry>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + treenode!(x).red.store(true, Ordering::Relaxed); + + let mut x_parent: Shared<'_, BinEntry>; + let mut x_parent_parent: Shared<'_, BinEntry>; + let mut x_parent_parent_left: Shared<'_, BinEntry>; + let mut x_parent_parent_right: Shared<'_, BinEntry>; + loop { + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + if x_parent.is_null() { + treenode!(x).red.store(false, Ordering::Relaxed); + return x; + } + x_parent_parent = treenode!(x_parent).parent.load(Ordering::Relaxed, guard); + if !treenode!(x_parent).red.load(Ordering::Relaxed) || x_parent_parent.is_null() { + return root; + } + x_parent_parent_left = treenode!(x_parent_parent) + .left + .load(Ordering::Relaxed, guard); + if x_parent == x_parent_parent_left { + x_parent_parent_right = treenode!(x_parent_parent) + .right + .load(Ordering::Relaxed, guard); + if !x_parent_parent_right.is_null() + && treenode!(x_parent_parent_right).red.load(Ordering::Relaxed) + { + treenode!(x_parent_parent_right) + .red + .store(false, Ordering::Relaxed); + treenode!(x_parent).red.store(false, Ordering::Relaxed); + treenode!(x_parent_parent) + .red + .store(true, Ordering::Relaxed); + x = x_parent_parent; + } else { + if x == treenode!(x_parent).right.load(Ordering::Relaxed, guard) { + x = x_parent; + root = Self::rotate_left(root, x, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_parent = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).parent.load(Ordering::Relaxed, guard) + }; + } + if !x_parent.is_null() { + treenode!(x_parent).red.store(false, Ordering::Relaxed); + if !x_parent_parent.is_null() { + treenode!(x_parent_parent) + .red + .store(true, Ordering::Relaxed); + root = Self::rotate_right(root, x_parent_parent, guard); + } + } + } + } else if !x_parent_parent_left.is_null() + && treenode!(x_parent_parent_left).red.load(Ordering::Relaxed) + { + treenode!(x_parent_parent_left) + .red + .store(false, Ordering::Relaxed); + treenode!(x_parent).red.store(false, Ordering::Relaxed); + treenode!(x_parent_parent) + .red + .store(true, Ordering::Relaxed); + x = x_parent_parent; + } else { + if x == treenode!(x_parent).left.load(Ordering::Relaxed, guard) { + x = x_parent; + root = Self::rotate_right(root, x, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_parent = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).parent.load(Ordering::Relaxed, guard) + }; + } + if !x_parent.is_null() { + treenode!(x_parent).red.store(false, Ordering::Relaxed); + if !x_parent_parent.is_null() { + treenode!(x_parent_parent) + .red + .store(true, Ordering::Relaxed); + root = Self::rotate_left(root, x_parent_parent, guard); + } + } + } + } + } + + fn balance_deletion<'g>( + mut root: Shared<'g, BinEntry>, + mut x: Shared<'g, BinEntry>, + guard: &'g Guard, + ) -> Shared<'g, BinEntry> { + let mut x_parent: Shared<'_, BinEntry>; + let mut x_parent_left: Shared<'_, BinEntry>; + let mut x_parent_right: Shared<'_, BinEntry>; + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + loop { + if x.is_null() || x == root { + return root; + } + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + if x_parent.is_null() { + treenode!(x).red.store(false, Ordering::Relaxed); + return x; + } else if treenode!(x).red.load(Ordering::Relaxed) { + treenode!(x).red.store(false, Ordering::Relaxed); + return root; + } + x_parent_left = treenode!(x_parent).left.load(Ordering::Relaxed, guard); + if x_parent_left == x { + x_parent_right = treenode!(x_parent).right.load(Ordering::Relaxed, guard); + if !x_parent_right.is_null() + && treenode!(x_parent_right).red.load(Ordering::Relaxed) + { + treenode!(x_parent_right) + .red + .store(false, Ordering::Relaxed); + treenode!(x_parent).red.store(true, Ordering::Relaxed); + root = Self::rotate_left(root, x_parent, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_right = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).right.load(Ordering::Relaxed, guard) + }; + } + if x_parent_right.is_null() { + x = x_parent; + continue; + } + let s_left = treenode!(x_parent_right) + .left + .load(Ordering::Relaxed, guard); + let mut s_right = treenode!(x_parent_right) + .right + .load(Ordering::Relaxed, guard); + + if (s_right.is_null() || !treenode!(s_right).red.load(Ordering::Relaxed)) + && (s_left.is_null() || !treenode!(s_left).red.load(Ordering::Relaxed)) + { + treenode!(x_parent_right).red.store(true, Ordering::Relaxed); + x = x_parent; + continue; + } + if s_right.is_null() || !treenode!(s_right).red.load(Ordering::Relaxed) { + if !s_left.is_null() { + treenode!(s_left).red.store(false, Ordering::Relaxed); + } + treenode!(x_parent_right).red.store(true, Ordering::Relaxed); + root = Self::rotate_right(root, x_parent_right, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_right = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).right.load(Ordering::Relaxed, guard) + }; + } + if !x_parent_right.is_null() { + treenode!(x_parent_right).red.store( + if x_parent.is_null() { + false + } else { + treenode!(x_parent).red.load(Ordering::Relaxed) + }, + Ordering::Relaxed, + ); + s_right = treenode!(x_parent_right) + .right + .load(Ordering::Relaxed, guard); + if !s_right.is_null() { + treenode!(s_right).red.store(false, Ordering::Relaxed); + } + } + if !x_parent.is_null() { + treenode!(x_parent).red.store(false, Ordering::Relaxed); + root = Self::rotate_left(root, x_parent, guard); + } + x = root; + } else { + // symmetric + if !x_parent_left.is_null() && treenode!(x_parent_left).red.load(Ordering::Relaxed) + { + treenode!(x_parent_left).red.store(false, Ordering::Relaxed); + treenode!(x_parent).red.store(true, Ordering::Relaxed); + root = Self::rotate_right(root, x_parent, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_left = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).left.load(Ordering::Relaxed, guard) + }; + } + if x_parent_left.is_null() { + x = x_parent; + continue; + } + let mut s_left = treenode!(x_parent_left).left.load(Ordering::Relaxed, guard); + let s_right = treenode!(x_parent_left) + .right + .load(Ordering::Relaxed, guard); + + if (s_left.is_null() || !treenode!(s_left).red.load(Ordering::Relaxed)) + && (s_right.is_null() || !treenode!(s_right).red.load(Ordering::Relaxed)) + { + treenode!(x_parent_left).red.store(true, Ordering::Relaxed); + x = x_parent; + continue; + } + if s_left.is_null() || !treenode!(s_left).red.load(Ordering::Relaxed) { + if !s_right.is_null() { + treenode!(s_right).red.store(false, Ordering::Relaxed); + } + treenode!(x_parent_left).red.store(true, Ordering::Relaxed); + root = Self::rotate_left(root, x_parent_left, guard); + x_parent = treenode!(x).parent.load(Ordering::Relaxed, guard); + x_parent_left = if x_parent.is_null() { + Shared::null() + } else { + treenode!(x_parent).left.load(Ordering::Relaxed, guard) + }; + } + if !x_parent_left.is_null() { + treenode!(x_parent_left).red.store( + if x_parent.is_null() { + false + } else { + treenode!(x_parent).red.load(Ordering::Relaxed) + }, + Ordering::Relaxed, + ); + s_left = treenode!(x_parent_left).left.load(Ordering::Relaxed, guard); + if !s_left.is_null() { + treenode!(s_left).red.store(false, Ordering::Relaxed); + } + } + if !x_parent.is_null() { + treenode!(x_parent).red.store(false, Ordering::Relaxed); + root = Self::rotate_right(root, x_parent, guard); + } + x = root; + } + } + } + /// Checks invariants recursively for the tree of Nodes rootet at t. + fn check_invariants<'g>(t: Shared<'g, BinEntry>, guard: &'g Guard) { + // safety: the containing TreeBin of all TreeNodes was read under our + // guard, at which point the tree structure was valid. Since our guard + // pins the current epoch, the TreeNodes remain valid for at least as + // long as we hold onto the guard. + // Structurally, TreeNodes always point to TreeNodes, so this is sound. + let t_deref = treenode!(t); + let t_parent = t_deref.parent.load(Ordering::Relaxed, guard); + let t_left = t_deref.left.load(Ordering::Relaxed, guard); + let t_right = t_deref.right.load(Ordering::Relaxed, guard); + let t_back = t_deref.prev.load(Ordering::Relaxed, guard); + let t_next = t_deref.node.next.load(Ordering::Relaxed, guard); + + if !t_back.is_null() { + let t_back_deref = treenode!(t_back); + assert_eq!( + t_back_deref.node.next.load(Ordering::Relaxed, guard), + t, + "A TreeNode's `prev` node did not point back to it as its `next` node" + ); + } + if !t_next.is_null() { + let t_next_deref = treenode!(t_next); + assert_eq!( + t_next_deref.prev.load(Ordering::Relaxed, guard), + t, + "A TreeNode's `next` node did not point back to it as its `prev` node" + ); + } + if !t_parent.is_null() { + let t_parent_deref = treenode!(t_parent); + assert!( + t_parent_deref.left.load(Ordering::Relaxed, guard) == t + || t_parent_deref.right.load(Ordering::Relaxed, guard) == t, + "A TreeNode's `parent` node did not point back to it as either its `left` or `right` child" + ); + } + if !t_left.is_null() { + let t_left_deref = treenode!(t_left); + assert_eq!( + t_left_deref.parent.load(Ordering::Relaxed, guard), + t, + "A TreeNode's `left` child did not point back to it as its `parent` node" + ); + assert!( + t_left_deref.node.hash <= t_deref.node.hash, + "A TreeNode's `left` child had a greater hash value than it" + ); + } + if !t_right.is_null() { + let t_right_deref = treenode!(t_right); + assert_eq!( + t_right_deref.parent.load(Ordering::Relaxed, guard), + t, + "A TreeNode's `right` child did not point back to it as its `parent` node" + ); + assert!( + t_right_deref.node.hash >= t_deref.node.hash, + "A TreeNode's `right` child had a smaller hash value than it" + ); + } + if t_deref.red.load(Ordering::Relaxed) && !t_left.is_null() && !t_right.is_null() { + // if we are red, at least one of our children must be black + let t_left_deref = treenode!(t_left); + let t_right_deref = treenode!(t_right); + assert!( + !(t_left_deref.red.load(Ordering::Relaxed) + && t_right_deref.red.load(Ordering::Relaxed)), + "A red TreeNode's two children were both also red" + ); + } + if !t_left.is_null() { + Self::check_invariants(t_left, guard) + } + if !t_right.is_null() { + Self::check_invariants(t_right, guard) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/raw/mod.rs b/src/raw/mod.rs index 58736e2c..bf4fd49b 100644 --- a/src/raw/mod.rs +++ b/src/raw/mod.rs @@ -111,7 +111,7 @@ impl Table { ) -> Shared<'g, BinEntry> where K: Borrow, - Q: ?Sized + Eq, + Q: ?Sized + Ord, { match *bin { BinEntry::Node(_) => { @@ -154,15 +154,24 @@ impl Table { let bin = unsafe { bin.deref() }; match *bin { - BinEntry::Node(_) => break table.find(bin, hash, key, guard), + BinEntry::Node(_) | BinEntry::Tree(_) => { + break table.find(bin, hash, key, guard) + } BinEntry::Moved => { // safety: same as above. table = unsafe { table.next_table(guard).deref() }; continue; } + BinEntry::TreeNode(_) => unreachable!("`find` was called on a Moved entry pointing to a TreeNode, which cannot be the first entry in a bin"), } } } + BinEntry::TreeNode(_) => { + unreachable!( + "`find` was called on a TreeNode, which cannot be the first entry in a bin" + ); + } + BinEntry::Tree(_) => TreeBin::find(Shared::from(bin as *const _), hash, key, guard), } } @@ -186,7 +195,7 @@ impl Table { match *bin_entry { BinEntry::Moved => {} BinEntry::Node(_) => { - // safety: same as above + we own the bin - Node are not shared across the table + // safety: same as above + we own the bin - Nodes are not shared across the table let mut p = unsafe { bin.into_owned() }; loop { // safety below: @@ -210,6 +219,20 @@ impl Table { p = unsafe { node.next.into_owned() }; } } + BinEntry::Tree(_) => { + // safety: same as for BinEntry::Node + let p = unsafe { bin.into_owned() }; + let bin = if let BinEntry::Tree(bin) = *p.into_box() { + bin + } else { + unreachable!(); + }; + // TreeBin::drop will take care of freeing the contained TreeNodes and their values + drop(bin); + } + BinEntry::TreeNode(_) => unreachable!( + "The head of a bin cannot be a TreeNode directly without BinEntry::Tree" + ), } } } diff --git a/src/serde_impls.rs b/src/serde_impls.rs index 42e5ebbd..f46a2e38 100644 --- a/src/serde_impls.rs +++ b/src/serde_impls.rs @@ -41,8 +41,8 @@ where impl<'de, K, V, S> Deserialize<'de> for HashMap where - K: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Eq, - V: 'static + Deserialize<'de> + Send + Sync + Eq, + K: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Ord, + V: 'static + Deserialize<'de> + Send + Sync + Ord, S: Default + BuildHasher, { fn deserialize(deserializer: D) -> Result @@ -65,8 +65,8 @@ impl HashMapVisitor { impl<'de, K, V, S> Visitor<'de> for HashMapVisitor where - K: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Eq, - V: 'static + Deserialize<'de> + Send + Sync + Eq, + K: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Ord, + V: 'static + Deserialize<'de> + Send + Sync + Ord, S: Default + BuildHasher, { type Value = HashMap; @@ -121,7 +121,7 @@ where impl<'de, T, S> Deserialize<'de> for HashSet where - T: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Eq, + T: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Ord, S: Default + BuildHasher, { fn deserialize(deserializer: D) -> Result @@ -148,7 +148,7 @@ impl HashSetVisitor { impl<'de, T, S> Visitor<'de> for HashSetVisitor where - T: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Eq, + T: 'static + Deserialize<'de> + Send + Sync + Hash + Clone + Ord, S: Default + BuildHasher, { type Value = HashSet; diff --git a/src/set.rs b/src/set.rs index cdd2c690..8468b584 100644 --- a/src/set.rs +++ b/src/set.rs @@ -216,16 +216,16 @@ impl HashSet { impl HashSet where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { /// Returns `true` if the given value is an element of this set. /// /// The value may be any borrowed form of the set's value type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the value type. /// - /// [`Eq`]: std::cmp::Eq + /// [`Ord`]: std::cmp::Ord /// [`Hash`]: std::hash::Hash /// /// # Examples @@ -244,7 +244,7 @@ where pub fn contains<'g, Q>(&self, value: &Q, guard: &'g Guard) -> bool where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.contains_key(value, guard) } @@ -252,9 +252,12 @@ where /// Returns a reference to the element in the set, if any, that is equal to the given value. /// /// The value may be any borrowed form of the set's value type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the value type. /// + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash + /// /// # Examples /// /// ``` @@ -265,13 +268,10 @@ where /// assert_eq!(set.get(&2, &guard), Some(&2)); /// assert_eq!(set.get(&4, &guard), None); /// ``` - /// - /// [`Eq`]: ../../std/cmp/trait.Eq.html - /// [`Hash`]: ../../std/hash/trait.Hash.html pub fn get<'g, Q>(&'g self, value: &Q, guard: &'g Guard) -> Option<&'g T> where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.get_key_value(value, guard).map(|(k, _)| k) } @@ -374,7 +374,7 @@ where impl HashSet where - T: 'static + Sync + Send + Clone + Hash + Eq, + T: 'static + Sync + Send + Clone + Hash + Ord, S: BuildHasher, { /// Adds a value to the set. @@ -407,10 +407,10 @@ where /// If the set did have this value present, `true` is returned. /// /// The value may be any borrowed form of the set's value type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the value type. /// - /// [`Eq`]: std::cmp::Eq + /// [`Ord`]: std::cmp::Ord /// [`Hash`]: std::hash::Hash /// /// # Examples @@ -429,7 +429,7 @@ where pub fn remove(&self, value: &Q, guard: &Guard) -> bool where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { let removed = self.map.remove(value, guard); removed.is_some() @@ -438,9 +438,12 @@ where /// Removes and returns the value in the set, if any, that is equal to the given one. /// /// The value may be any borrowed form of the set's value type, but - /// [`Hash`] and [`Eq`] on the borrowed form *must* match those for + /// [`Hash`] and [`Ord`] on the borrowed form *must* match those for /// the value type. /// + /// [`Ord`]: std::cmp::Ord + /// [`Hash`]: std::hash::Hash + /// /// # Examples /// /// ``` @@ -451,13 +454,10 @@ where /// assert_eq!(set.take(&2, &guard), Some(&2)); /// assert_eq!(set.take(&2, &guard), None); /// ``` - /// - /// [`Eq`]: ../../std/cmp/trait.Eq.html - /// [`Hash`]: ../../std/hash/trait.Hash.html pub fn take<'g, Q>(&'g self, value: &Q, guard: &'g Guard) -> Option<&'g T> where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.map.remove_entry(value, guard).map(|(k, _)| k) } @@ -489,7 +489,7 @@ where impl HashSet where - T: Clone, + T: Clone + Ord, { /// Clears the set, removing all elements. /// @@ -519,7 +519,7 @@ where impl PartialEq for HashSet where - T: Eq + Hash, + T: Ord + Hash, S: BuildHasher, { fn eq(&self, other: &Self) -> bool { @@ -529,7 +529,7 @@ where impl Eq for HashSet where - T: Eq + Hash, + T: Ord + Hash, S: BuildHasher, { } @@ -546,7 +546,7 @@ where impl Extend for &HashSet where - T: 'static + Sync + Send + Clone + Hash + Eq, + T: 'static + Sync + Send + Clone + Hash + Ord, S: BuildHasher, { fn extend>(&mut self, iter: I) { @@ -556,7 +556,7 @@ where impl<'a, T, S> Extend<&'a T> for &HashSet where - T: 'static + Sync + Send + Copy + Hash + Eq, + T: 'static + Sync + Send + Copy + Hash + Ord, S: BuildHasher, { fn extend>(&mut self, iter: I) { @@ -566,7 +566,7 @@ where impl FromIterator for HashSet where - T: 'static + Sync + Send + Clone + Hash + Eq, + T: 'static + Sync + Send + Clone + Hash + Ord, S: BuildHasher + Default, { fn from_iter>(iter: I) -> Self { @@ -578,7 +578,7 @@ where impl<'a, T, S> FromIterator<&'a T> for HashSet where - T: 'static + Sync + Send + Copy + Hash + Eq, + T: 'static + Sync + Send + Copy + Hash + Ord, S: BuildHasher + Default, { fn from_iter>(iter: I) -> Self { @@ -590,7 +590,7 @@ where impl Clone for HashSet where - T: 'static + Sync + Send + Clone + Hash + Eq, + T: 'static + Sync + Send + Clone + Hash + Ord, S: BuildHasher + Clone, { fn clone(&self) -> HashSet { diff --git a/src/set_ref.rs b/src/set_ref.rs index 80c9fe0a..985a9f49 100644 --- a/src/set_ref.rs +++ b/src/set_ref.rs @@ -62,7 +62,7 @@ impl HashSetRef<'_, T, S> { impl HashSetRef<'_, T, S> where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { /// Returns `true` if the given value is an element of this set. @@ -72,7 +72,7 @@ where pub fn contains(&self, value: &Q) -> bool where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.set.contains(value, &self.guard) } @@ -83,7 +83,7 @@ where pub fn get<'g, Q>(&'g self, value: &Q) -> Option<&'g T> where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.set.get(value, &self.guard) } @@ -112,7 +112,7 @@ where impl HashSetRef<'_, T, S> where - T: 'static + Sync + Send + Clone + Hash + Eq, + T: 'static + Sync + Send + Clone + Hash + Ord, S: BuildHasher, { /// Adds a value to the set. @@ -128,7 +128,7 @@ where pub fn remove(&self, value: &Q) -> bool where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.set.remove(value, &self.guard) } @@ -139,7 +139,7 @@ where pub fn take<'g, Q>(&'g self, value: &Q) -> Option<&'g T> where T: Borrow, - Q: ?Sized + Hash + Eq, + Q: ?Sized + Hash + Ord, { self.set.take(value, &self.guard) } @@ -157,7 +157,7 @@ where impl HashSetRef<'_, T, S> where - T: Clone, + T: Clone + Ord, { /// Clears the set, removing all elements. /// @@ -201,7 +201,7 @@ impl Clone for HashSetRef<'_, T, S> { impl PartialEq for HashSetRef<'_, T, S> where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { fn eq(&self, other: &Self) -> bool { @@ -211,7 +211,7 @@ where impl PartialEq> for HashSetRef<'_, T, S> where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { fn eq(&self, other: &HashSet) -> bool { @@ -221,7 +221,7 @@ where impl PartialEq> for HashSet where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { fn eq(&self, other: &HashSetRef<'_, T, S>) -> bool { @@ -231,7 +231,7 @@ where impl Eq for HashSetRef<'_, T, S> where - T: Hash + Eq, + T: Hash + Ord, S: BuildHasher, { } diff --git a/tests/jdk/concurrent_associate.rs b/tests/jdk/concurrent_associate.rs index f21dce5f..4f884dc8 100644 --- a/tests/jdk/concurrent_associate.rs +++ b/tests/jdk/concurrent_associate.rs @@ -8,7 +8,7 @@ const NUM_ENTRIES: usize = 128; /// Number of iterations for each test const ITERATIONS: usize = 64; -#[derive(Hash, PartialEq, Eq, Clone, Copy)] +#[derive(Hash, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] struct KeyVal { _data: usize, } diff --git a/tests/jdk/map_check.rs b/tests/jdk/map_check.rs index 2bc5ea1f..e376e3d4 100644 --- a/tests/jdk/map_check.rs +++ b/tests/jdk/map_check.rs @@ -9,7 +9,7 @@ const ABSENT_MASK: usize = ABSENT_SIZE - 1; fn t1(map: &HashMap, keys: &[K], expect: usize) where - K: Sync + Send + Clone + Hash + Eq, + K: Sync + Send + Clone + Hash + Ord, V: Sync + Send, { let mut sum = 0; @@ -27,7 +27,7 @@ where fn t2(map: &HashMap, keys: &[K], expect: usize) where - K: 'static + Sync + Send + Copy + Hash + Eq + std::fmt::Display, + K: 'static + Sync + Send + Copy + Hash + Ord + std::fmt::Display, { let mut sum = 0; let guard = epoch::pin(); @@ -41,7 +41,7 @@ where fn t3(map: &HashMap, keys: &[K], expect: usize) where - K: 'static + Sync + Send + Copy + Hash + Eq, + K: 'static + Sync + Send + Copy + Hash + Ord, { let mut sum = 0; let guard = epoch::pin(); @@ -55,7 +55,7 @@ where fn t4(map: &HashMap, keys: &[K], expect: usize) where - K: Sync + Send + Copy + Hash + Eq, + K: Sync + Send + Copy + Hash + Ord, { let mut sum = 0; let guard = epoch::pin(); @@ -69,7 +69,7 @@ where fn t5(map: &HashMap, keys: &[K], expect: usize) where - K: 'static + Sync + Send + Copy + Hash + Eq, + K: 'static + Sync + Send + Copy + Hash + Ord, { let mut sum = 0; let guard = epoch::pin(); @@ -85,7 +85,7 @@ where fn t6(map: &HashMap, keys1: &[K], keys2: &[K], expect: usize) where - K: Sync + Send + Clone + Hash + Eq, + K: Sync + Send + Clone + Hash + Ord, V: Sync + Send, { let mut sum = 0; @@ -103,7 +103,7 @@ where fn t7(map: &HashMap, k1: &[K], k2: &[K]) where - K: Sync + Send + Copy + Hash + Eq, + K: Sync + Send + Copy + Hash + Ord, { let mut sum = 0; let guard = epoch::pin();