BTreeMap: clarify comments and panics surrounding choose_parent_kv

This commit is contained in:
Stein Somers 2020-11-24 12:27:38 +01:00
parent 4c5c4aa002
commit 50576420f5
2 changed files with 14 additions and 12 deletions

View File

@ -1282,10 +1282,12 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
/// Chooses a balancing context involving the node as a child, thus between /// Chooses a balancing context involving the node as a child, thus between
/// the KV immediately to the left or to the right in the parent node. /// the KV immediately to the left or to the right in the parent node.
/// Returns an `Err` if there is no parent. /// Returns an `Err` if there is no parent.
/// Panics if the parent is empty.
/// ///
/// This method optimizes for a node that has fewer elements than its left /// Prefers the left side, to be optimal if the given node is somehow
/// and right siblings, if they exist, by preferring the left parent KV. /// underfull, meaning here only that it has fewer elements than its left
/// Merging with the left sibling is faster, since we only need to move /// sibling and than its right sibling, if they exist. In that case,
/// merging with the left sibling is faster, since we only need to move
/// the node's N elements, instead of shifting them to the right and moving /// the node's N elements, instead of shifting them to the right and moving
/// more than N elements in front. Stealing from the left sibling is also /// more than N elements in front. Stealing from the left sibling is also
/// typically faster, since we only need to shift the node's N elements to /// typically faster, since we only need to shift the node's N elements to
@ -1293,19 +1295,19 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
/// the left. /// the left.
pub fn choose_parent_kv(self) -> Result<LeftOrRight<BalancingContext<'a, K, V>>, Self> { pub fn choose_parent_kv(self) -> Result<LeftOrRight<BalancingContext<'a, K, V>>, Self> {
match unsafe { ptr::read(&self) }.ascend() { match unsafe { ptr::read(&self) }.ascend() {
Ok(parent) => match parent.left_kv() { Ok(parent_edge) => match parent_edge.left_kv() {
Ok(left_parent_kv) => Ok(LeftOrRight::Left(BalancingContext { Ok(left_parent_kv) => Ok(LeftOrRight::Left(BalancingContext {
parent: unsafe { ptr::read(&left_parent_kv) }, parent: unsafe { ptr::read(&left_parent_kv) },
left_child: left_parent_kv.left_edge().descend(), left_child: left_parent_kv.left_edge().descend(),
right_child: self, right_child: self,
})), })),
Err(parent) => match parent.right_kv() { Err(parent_edge) => match parent_edge.right_kv() {
Ok(right_parent_kv) => Ok(LeftOrRight::Right(BalancingContext { Ok(right_parent_kv) => Ok(LeftOrRight::Right(BalancingContext {
parent: unsafe { ptr::read(&right_parent_kv) }, parent: unsafe { ptr::read(&right_parent_kv) },
left_child: self, left_child: self,
right_child: right_parent_kv.right_edge().descend(), right_child: right_parent_kv.right_edge().descend(),
})), })),
Err(_) => unreachable!("empty non-root node"), Err(_) => unreachable!("empty internal node"),
}, },
}, },
Err(root) => Err(root), Err(root) => Err(root),

View File

@ -55,12 +55,12 @@ impl<'a, K: 'a, V: 'a> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, mark
pos = unsafe { new_pos.cast_to_leaf_unchecked() }; pos = unsafe { new_pos.cast_to_leaf_unchecked() };
// Only if we merged, the parent (if any) has shrunk, but skipping // Only if we merged, the parent (if any) has shrunk, but skipping
// the following step does not pay off in benchmarks. // the following step otherwise does not pay off in benchmarks.
// //
// SAFETY: We won't destroy or rearrange the leaf where `pos` is at // SAFETY: We won't destroy or rearrange the leaf where `pos` is at
// by handling its parent recursively; at worst we will destroy or // by handling its parent recursively; at worst we will destroy or
// rearrange the parent through the grandparent, thus change the // rearrange the parent through the grandparent, thus change the
// leaf's parent pointer. // link to the parent inside the leaf.
if let Ok(parent) = unsafe { pos.reborrow_mut() }.into_node().ascend() { if let Ok(parent) = unsafe { pos.reborrow_mut() }.into_node().ascend() {
parent.into_node().handle_shrunk_node_recursively(handle_emptied_internal_root); parent.into_node().handle_shrunk_node_recursively(handle_emptied_internal_root);
} }
@ -92,8 +92,8 @@ impl<'a, K: 'a, V: 'a> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>,
} }
impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> { impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
/// Stocks up a possibly underfull internal node, recursively. /// Stocks up a possibly underfull internal node and its ancestors,
/// Climbs up until it reaches an ancestor that has elements to spare or the root. /// until it reaches an ancestor that has elements to spare or is the root.
fn handle_shrunk_node_recursively<F: FnOnce()>(mut self, handle_emptied_internal_root: F) { fn handle_shrunk_node_recursively<F: FnOnce()>(mut self, handle_emptied_internal_root: F) {
loop { loop {
self = match self.len() { self = match self.len() {
@ -124,7 +124,7 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
) -> Option<NodeRef<marker::Mut<'a>, K, V, marker::Internal>> { ) -> Option<NodeRef<marker::Mut<'a>, K, V, marker::Internal>> {
match self.forget_type().choose_parent_kv() { match self.forget_type().choose_parent_kv() {
Ok(Left(left_parent_kv)) => { Ok(Left(left_parent_kv)) => {
debug_assert!(left_parent_kv.right_child_len() == MIN_LEN - 1); debug_assert_eq!(left_parent_kv.right_child_len(), MIN_LEN - 1);
if left_parent_kv.can_merge() { if left_parent_kv.can_merge() {
let pos = left_parent_kv.merge(None); let pos = left_parent_kv.merge(None);
let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) }; let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) };
@ -136,7 +136,7 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
} }
} }
Ok(Right(right_parent_kv)) => { Ok(Right(right_parent_kv)) => {
debug_assert!(right_parent_kv.left_child_len() == MIN_LEN - 1); debug_assert_eq!(right_parent_kv.left_child_len(), MIN_LEN - 1);
if right_parent_kv.can_merge() { if right_parent_kv.can_merge() {
let pos = right_parent_kv.merge(None); let pos = right_parent_kv.merge(None);
let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) }; let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) };