Skip to content

Commit 6af6c40

Browse files
Rollup merge of #81115 - ssomers:btree_drainy_refactor_4, r=Mark-Simulacrum
BTreeMap: prefer bulk_steal functions over specialized ones The `steal_` functions (apart from their return value) are basically specializations of the more general `bulk_steal_` functions. This PR removes the specializations. The library/alloc benchmarks say this is never slower and up to 6% faster. r? ``@Mark-Simulacrum``
2 parents de02bf3 + 4775334 commit 6af6c40

File tree

2 files changed

+8
-121
lines changed

2 files changed

+8
-121
lines changed

library/alloc/src/collections/btree/node.rs

+4-117
Original file line numberDiff line numberDiff line change
@@ -592,17 +592,6 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Leaf> {
592592
self.val_area_mut(idx).write(val);
593593
}
594594
}
595-
596-
/// Adds a key-value pair to the beginning of the node.
597-
fn push_front(&mut self, key: K, val: V) {
598-
let new_len = self.len() + 1;
599-
assert!(new_len <= CAPACITY);
600-
unsafe {
601-
slice_insert(self.key_area_mut(..new_len), 0, key);
602-
slice_insert(self.val_area_mut(..new_len), 0, val);
603-
*self.len_mut() = new_len as u16;
604-
}
605-
}
606595
}
607596

608597
impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
@@ -638,88 +627,6 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
638627
Handle::new_edge(self.reborrow_mut(), idx + 1).correct_parent_link();
639628
}
640629
}
641-
642-
/// Adds a key-value pair, and an edge to go to the left of that pair,
643-
/// to the beginning of the node.
644-
fn push_front(&mut self, key: K, val: V, edge: Root<K, V>) {
645-
let new_len = self.len() + 1;
646-
assert!(edge.height == self.height - 1);
647-
assert!(new_len <= CAPACITY);
648-
649-
unsafe {
650-
slice_insert(self.key_area_mut(..new_len), 0, key);
651-
slice_insert(self.val_area_mut(..new_len), 0, val);
652-
slice_insert(self.edge_area_mut(..new_len + 1), 0, edge.node);
653-
*self.len_mut() = new_len as u16;
654-
}
655-
656-
self.correct_all_childrens_parent_links();
657-
}
658-
}
659-
660-
impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
661-
/// Removes a key-value pair from the end of the node and returns the pair.
662-
/// Also removes the edge that was to the right of that pair and, if the node
663-
/// is internal, returns the orphaned subtree that this edge owned.
664-
///
665-
/// # Safety
666-
/// The node must not be empty.
667-
unsafe fn pop(&mut self) -> (K, V, Option<Root<K, V>>) {
668-
debug_assert!(self.len() > 0);
669-
670-
let idx = self.len() - 1;
671-
672-
unsafe {
673-
let key = self.key_area_mut(idx).assume_init_read();
674-
let val = self.val_area_mut(idx).assume_init_read();
675-
let edge = match self.reborrow_mut().force() {
676-
ForceResult::Leaf(_) => None,
677-
ForceResult::Internal(mut internal) => {
678-
let node = internal.edge_area_mut(idx + 1).assume_init_read();
679-
let mut edge = Root { node, height: internal.height - 1, _marker: PhantomData };
680-
// Currently, clearing the parent link is superfluous, because we will
681-
// insert the node elsewhere and set its parent link again.
682-
edge.clear_parent_link();
683-
Some(edge)
684-
}
685-
};
686-
687-
*self.len_mut() -= 1;
688-
(key, val, edge)
689-
}
690-
}
691-
692-
/// Removes a key-value pair from the beginning of the node and returns the pair.
693-
/// Also removes the edge that was to the left of that pair and, if the node is
694-
/// internal, returns the orphaned subtree that this edge owned.
695-
fn pop_front(&mut self) -> (K, V, Option<Root<K, V>>) {
696-
debug_assert!(self.len() > 0);
697-
698-
let old_len = self.len();
699-
700-
unsafe {
701-
let key = slice_remove(self.key_area_mut(..old_len), 0);
702-
let val = slice_remove(self.val_area_mut(..old_len), 0);
703-
let edge = match self.reborrow_mut().force() {
704-
ForceResult::Leaf(_) => None,
705-
ForceResult::Internal(mut internal) => {
706-
let node = slice_remove(internal.edge_area_mut(..old_len + 1), 0);
707-
let mut edge = Root { node, height: internal.height - 1, _marker: PhantomData };
708-
// Currently, clearing the parent link is superfluous, because we will
709-
// insert the node elsewhere and set its parent link again.
710-
edge.clear_parent_link();
711-
712-
internal.correct_childrens_parent_links(0..old_len);
713-
714-
Some(edge)
715-
}
716-
};
717-
718-
*self.len_mut() -= 1;
719-
720-
(key, val, edge)
721-
}
722-
}
723630
}
724631

725632
impl<BorrowType, K, V> NodeRef<BorrowType, K, V, marker::LeafOrInternal> {
@@ -1399,18 +1306,8 @@ impl<'a, K: 'a, V: 'a> BalancingContext<'a, K, V> {
13991306
mut self,
14001307
track_right_edge_idx: usize,
14011308
) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal>, marker::Edge> {
1402-
unsafe {
1403-
let (k, v, edge) = self.left_child.pop();
1404-
1405-
let (k, v) = self.parent.replace_kv(k, v);
1406-
1407-
match self.right_child.reborrow_mut().force() {
1408-
ForceResult::Leaf(mut leaf) => leaf.push_front(k, v),
1409-
ForceResult::Internal(mut internal) => internal.push_front(k, v, edge.unwrap()),
1410-
}
1411-
1412-
Handle::new_edge(self.right_child, 1 + track_right_edge_idx)
1413-
}
1309+
self.bulk_steal_left(1);
1310+
unsafe { Handle::new_edge(self.right_child, 1 + track_right_edge_idx) }
14141311
}
14151312

14161313
/// Removes a key-value pair from the right child and places it in the key-value storage
@@ -1421,18 +1318,8 @@ impl<'a, K: 'a, V: 'a> BalancingContext<'a, K, V> {
14211318
mut self,
14221319
track_left_edge_idx: usize,
14231320
) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal>, marker::Edge> {
1424-
unsafe {
1425-
let (k, v, edge) = self.right_child.pop_front();
1426-
1427-
let (k, v) = self.parent.replace_kv(k, v);
1428-
1429-
match self.left_child.reborrow_mut().force() {
1430-
ForceResult::Leaf(mut leaf) => leaf.push(k, v),
1431-
ForceResult::Internal(mut internal) => internal.push(k, v, edge.unwrap()),
1432-
}
1433-
1434-
Handle::new_edge(self.left_child, track_left_edge_idx)
1435-
}
1321+
self.bulk_steal_right(1);
1322+
unsafe { Handle::new_edge(self.left_child, track_left_edge_idx) }
14361323
}
14371324

14381325
/// This does stealing similar to `steal_left` but steals multiple elements at once.

library/alloc/src/collections/btree/remove.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -121,25 +121,25 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
121121
self,
122122
) -> Option<NodeRef<marker::Mut<'a>, K, V, marker::Internal>> {
123123
match self.forget_type().choose_parent_kv() {
124-
Ok(Left(left_parent_kv)) => {
124+
Ok(Left(mut left_parent_kv)) => {
125125
debug_assert_eq!(left_parent_kv.right_child_len(), MIN_LEN - 1);
126126
if left_parent_kv.can_merge() {
127127
let parent = left_parent_kv.merge_tracking_parent();
128128
Some(parent)
129129
} else {
130130
debug_assert!(left_parent_kv.left_child_len() > MIN_LEN);
131-
left_parent_kv.steal_left(0);
131+
left_parent_kv.bulk_steal_left(1);
132132
None
133133
}
134134
}
135-
Ok(Right(right_parent_kv)) => {
135+
Ok(Right(mut right_parent_kv)) => {
136136
debug_assert_eq!(right_parent_kv.left_child_len(), MIN_LEN - 1);
137137
if right_parent_kv.can_merge() {
138138
let parent = right_parent_kv.merge_tracking_parent();
139139
Some(parent)
140140
} else {
141141
debug_assert!(right_parent_kv.right_child_len() > MIN_LEN);
142-
right_parent_kv.steal_right(0);
142+
right_parent_kv.bulk_steal_right(1);
143143
None
144144
}
145145
}

0 commit comments

Comments
 (0)