Skip to content

Commit

Permalink
Auto merge of #75105 - ssomers:btree_respect_min_len_hard, r=Mark-Sim…
Browse files Browse the repository at this point in the history
…ulacrum

Hard way to respect BTreeMap's minimum node length

Resolves #74834 the hard way (though not the hardest imaginable).

Benchmarks (which are all biased/realistic, inserting keys in ascending order) say:
```
benchcmp r0 r1 --threshold 10
 name                                        r0 ns/iter  r1 ns/iter  diff ns/iter   diff %  speedup
 btree::map::clone_slim_100_and_clear        2,183       2,723                540   24.74%   x 0.80
 btree::map::clone_slim_100_and_drain_all    3,652       4,173                521   14.27%   x 0.88
 btree::map::clone_slim_100_and_drain_half   3,320       3,940                620   18.67%   x 0.84
 btree::map::clone_slim_100_and_into_iter    2,154       2,717                563   26.14%   x 0.79
 btree::map::clone_slim_100_and_pop_all      3,372       3,870                498   14.77%   x 0.87
 btree::map::clone_slim_100_and_remove_all   5,111       5,647                536   10.49%   x 0.91
 btree::map::clone_slim_100_and_remove_half  3,259       3,821                562   17.24%   x 0.85
 btree::map::iter_0                          1,733       1,509               -224  -12.93%   x 1.15
 btree::map::iter_100                        2,714       3,739              1,025   37.77%   x 0.73
 btree::map::iter_10k                        3,728       4,269                541   14.51%   x 0.87
 btree::map::range_unbounded_unbounded       28,426      36,631             8,205   28.86%   x 0.78
 btree::map::range_unbounded_vs_iter         28,808      34,056             5,248   18.22%   x 0.85
```
This difference is not caused by the `debug_assert`-related code in the function `splitpoint`, it's the same without.
  • Loading branch information
bors committed Aug 13, 2020
2 parents 5e3f1b1 + 17ab457 commit 81dc88f
Showing 1 changed file with 65 additions and 16 deletions.
81 changes: 65 additions & 16 deletions library/alloc/src/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,53 @@ impl<BorrowType, K, V, NodeType> Handle<NodeRef<BorrowType, K, V, NodeType>, mar
}
}

enum InsertionPlace {
Left(usize),
Right(usize),
}

/// Given an edge index where we want to insert into a node filled to capacity,
/// computes a sensible KV index of a split point and where to perform the insertion.
/// The goal of the split point is for its key and value to end up in a parent node;
/// the keys, values and edges to the left of the split point become the left child;
/// the keys, values and edges to the right of the split point become the right child.
fn splitpoint(edge_idx: usize) -> (usize, InsertionPlace) {
debug_assert!(edge_idx <= CAPACITY);
// Rust issue #74834 tries to explain these symmetric rules.
let middle_kv_idx;
let insertion;
if edge_idx <= B - 2 {
middle_kv_idx = B - 2;
insertion = InsertionPlace::Left(edge_idx);
} else if edge_idx == B - 1 {
middle_kv_idx = B - 1;
insertion = InsertionPlace::Left(edge_idx);
} else if edge_idx == B {
middle_kv_idx = B - 1;
insertion = InsertionPlace::Right(0);
} else {
middle_kv_idx = B;
let new_edge_idx = edge_idx - (B + 1);
insertion = InsertionPlace::Right(new_edge_idx);
}
let mut left_len = middle_kv_idx;
let mut right_len = CAPACITY - middle_kv_idx - 1;
match insertion {
InsertionPlace::Left(edge_idx) => {
debug_assert!(edge_idx <= left_len);
left_len += 1;
}
InsertionPlace::Right(edge_idx) => {
debug_assert!(edge_idx <= right_len);
right_len += 1;
}
}
debug_assert!(left_len >= MIN_LEN);
debug_assert!(right_len >= MIN_LEN);
debug_assert!(left_len + right_len == CAPACITY);
(middle_kv_idx, insertion)
}

impl<'a, K, V, NodeType> Handle<NodeRef<marker::Mut<'a>, K, V, NodeType>, marker::Edge> {
/// Helps implementations of `insert_fit` for a particular `NodeType`,
/// by taking care of leaf data.
Expand Down Expand Up @@ -863,18 +910,20 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge
let kv = unsafe { Handle::new_kv(self.node, self.idx) };
(InsertResult::Fit(kv), ptr)
} else {
let middle = unsafe { Handle::new_kv(self.node, B) };
let (middle_kv_idx, insertion) = splitpoint(self.idx);
let middle = unsafe { Handle::new_kv(self.node, middle_kv_idx) };
let (mut left, k, v, mut right) = middle.split();
let ptr = if self.idx <= B {
unsafe { Handle::new_edge(left.reborrow_mut(), self.idx).insert_fit(key, val) }
} else {
unsafe {
let ptr = match insertion {
InsertionPlace::Left(insert_idx) => unsafe {
Handle::new_edge(left.reborrow_mut(), insert_idx).insert_fit(key, val)
},
InsertionPlace::Right(insert_idx) => unsafe {
Handle::new_edge(
right.node_as_mut().cast_unchecked::<marker::Leaf>(),
self.idx - (B + 1),
insert_idx,
)
.insert_fit(key, val)
}
},
};
(InsertResult::Split(SplitResult { left: left.forget_type(), k, v, right }), ptr)
}
Expand Down Expand Up @@ -936,20 +985,20 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
let kv = unsafe { Handle::new_kv(self.node, self.idx) };
InsertResult::Fit(kv)
} else {
let middle = unsafe { Handle::new_kv(self.node, B) };
let (middle_kv_idx, insertion) = splitpoint(self.idx);
let middle = unsafe { Handle::new_kv(self.node, middle_kv_idx) };
let (mut left, k, v, mut right) = middle.split();
if self.idx <= B {
unsafe {
Handle::new_edge(left.reborrow_mut(), self.idx).insert_fit(key, val, edge);
}
} else {
unsafe {
match insertion {
InsertionPlace::Left(insert_idx) => unsafe {
Handle::new_edge(left.reborrow_mut(), insert_idx).insert_fit(key, val, edge);
},
InsertionPlace::Right(insert_idx) => unsafe {
Handle::new_edge(
right.node_as_mut().cast_unchecked::<marker::Internal>(),
self.idx - (B + 1),
insert_idx,
)
.insert_fit(key, val, edge);
}
},
}
InsertResult::Split(SplitResult { left: left.forget_type(), k, v, right })
}
Expand Down

0 comments on commit 81dc88f

Please sign in to comment.