From aeaa2b553fe0288c07ef2c7bd68d5826ab9ae76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20Tomi=C4=87?= Date: Wed, 18 Mar 2026 18:48:49 +0100 Subject: [PATCH 1/2] perf: single-pass pop_first/pop_last and remove cases 2a/2b Replace the double-traversal pattern in pop_first/pop_last with single-pass remove_leftmost/remove_rightmost methods. Previously, pop_last would walk the rightmost spine to find the max key (loading ~4 nodes), then walk the same spine again via remove_helper to delete it (loading ~4 more nodes). The single-pass approach descends once, handling rebalancing (rotation/merge) during descent. Also fix the EXC-1034 TODO in remove_helper cases 2a/2b: replace get_max + remove_helper with remove_rightmost, and get_min + remove_helper with remove_leftmost. Expected improvement: ~50% fewer load_node calls for pop operations, plus eliminates redundant binary searches on the descent path. --- src/btreemap.rs | 172 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 158 insertions(+), 14 deletions(-) diff --git a/src/btreemap.rs b/src/btreemap.rs index c6f4ae92..e655cf48 100644 --- a/src/btreemap.rs +++ b/src/btreemap.rs @@ -765,9 +765,9 @@ where } let root = self.load_node(self.root_addr); - let (max_key, _) = root.get_max(self.memory()); - self.remove_helper(root, &max_key) - .map(|v| (max_key, V::from_bytes(Cow::Owned(v)))) + self.remove_rightmost(root).map(|(k, v)| { + (k, V::from_bytes(Cow::Owned(v))) + }) } /// Removes and returns the first element in the map. The key of this element is the minimum key that was in the map @@ -777,9 +777,9 @@ where } let root = self.load_node(self.root_addr); - let (min_key, _) = root.get_min(self.memory()); - self.remove_helper(root, &min_key) - .map(|v| (min_key, V::from_bytes(Cow::Owned(v)))) + self.remove_leftmost(root).map(|(k, v)| { + (k, V::from_bytes(Cow::Owned(v))) + }) } /// A helper method for recursively removing a key from the B-tree. @@ -847,10 +847,8 @@ where // / \ // [...] [...] - // Recursively delete the predecessor. - // TODO(EXC-1034): Do this in a single pass. - let predecessor = left_child.get_max(self.memory()); - self.remove_helper(left_child, &predecessor.0)?; + // Remove the predecessor in a single pass (no double traversal). + let predecessor = self.remove_rightmost(left_child)?; // Replace the `key` with its predecessor. let (_, old_value) = node.swap_entry(idx, predecessor, self.memory()); @@ -882,10 +880,8 @@ where // / \ // [...] [...] - // Recursively delete the successor. - // TODO(EXC-1034): Do this in a single pass. - let successor = right_child.get_min(self.memory()); - self.remove_helper(right_child, &successor.0)?; + // Remove the successor in a single pass (no double traversal). + let successor = self.remove_leftmost(right_child)?; // Replace the `key` with its successor. let (_, old_value) = node.swap_entry(idx, successor, self.memory()); @@ -1150,6 +1146,154 @@ where } } + /// Removes and returns the rightmost (maximum) entry in the subtree rooted + /// at `node`, in a single top-down pass. This avoids the double traversal + /// of the previous approach (get_max + remove_helper). + fn remove_rightmost(&mut self, mut node: Node) -> Option> { + match node.node_type() { + NodeType::Leaf => { + let entry = node.pop_entry(self.memory())?; + self.length -= 1; + + if node.entries_len() == 0 { + assert_eq!(node.address(), self.root_addr); + self.deallocate_node(node); + self.root_addr = NULL; + } else { + self.save_node(&mut node); + } + self.save_header(); + Some(entry) + } + NodeType::Internal => { + let last_idx = node.children_len() - 1; + let mut child = self.load_node(node.child(last_idx)); + + if child.can_remove_entry_without_merging() { + return self.remove_rightmost(child); + } + + // The rightmost child is at minimum. Steal from its left sibling or merge. + let left_sibling_idx = last_idx - 1; + let mut left_sibling = self.load_node(node.child(left_sibling_idx)); + + if left_sibling.can_remove_entry_without_merging() { + // Rotate right: left_sibling -> parent -> child + let (left_key, left_value) = + left_sibling.pop_entry(self.memory()).unwrap(); + let (parent_key, parent_value) = node.swap_entry( + last_idx - 1, + (left_key, left_value), + self.memory(), + ); + child.insert_entry(0, (parent_key, parent_value)); + + if let Some(last_child) = left_sibling.pop_child() { + child.insert_child(0, last_child); + } + + self.save_node(&mut left_sibling); + self.save_node(&mut child); + self.save_node(&mut node); + return self.remove_rightmost(child); + } + + // Both at minimum: merge child into left sibling. + let merged = self.merge( + child, + left_sibling, + node.remove_entry(last_idx - 1, self.memory()), + ); + node.remove_child(last_idx); + + if node.entries_len() == 0 { + assert_eq!(node.address(), self.root_addr); + self.root_addr = merged.address(); + self.deallocate_node(node); + self.save_header(); + } else { + self.save_node(&mut node); + } + + self.remove_rightmost(merged) + } + } + } + + /// Removes and returns the leftmost (minimum) entry in the subtree rooted + /// at `node`, in a single top-down pass. + fn remove_leftmost(&mut self, mut node: Node) -> Option> { + match node.node_type() { + NodeType::Leaf => { + if node.entries_len() == 0 { + return None; + } + let entry = node.remove_entry(0, self.memory()); + self.length -= 1; + + if node.entries_len() == 0 { + assert_eq!(node.address(), self.root_addr); + self.deallocate_node(node); + self.root_addr = NULL; + } else { + self.save_node(&mut node); + } + self.save_header(); + Some(entry) + } + NodeType::Internal => { + let mut child = self.load_node(node.child(0)); + + if child.can_remove_entry_without_merging() { + return self.remove_leftmost(child); + } + + // The leftmost child is at minimum. Steal from its right sibling or merge. + let mut right_sibling = self.load_node(node.child(1)); + + if right_sibling.can_remove_entry_without_merging() { + // Rotate left: right_sibling -> parent -> child + let (right_key, right_value) = + right_sibling.remove_entry(0, self.memory()); + let parent_entry = node.swap_entry( + 0, + (right_key, right_value), + self.memory(), + ); + child.push_entry(parent_entry); + + if right_sibling.node_type() == NodeType::Internal { + child.push_child(right_sibling.remove_child(0)); + } + + self.save_node(&mut right_sibling); + self.save_node(&mut child); + self.save_node(&mut node); + return self.remove_leftmost(child); + } + + // Both at minimum: merge child into right sibling. + let merged = self.merge( + child, + right_sibling, + node.remove_entry(0, self.memory()), + ); + node.remove_child(0); + + if node.entries_len() == 0 { + assert_eq!(node.address(), self.root_addr); + self.root_addr = merged.address(); + self.deallocate_node(node); + self.save_header(); + } else { + self.save_node(&mut node); + } + + self.remove_leftmost(merged) + } + } + } + /// Returns an iterator over the entries of the map, sorted by key. /// /// # Example From ed49896a084ece56dc7b743f14562770d4649eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20Tomi=C4=87?= Date: Thu, 19 Mar 2026 15:54:59 +0100 Subject: [PATCH 2/2] fmt --- src/btreemap.rs | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/src/btreemap.rs b/src/btreemap.rs index e655cf48..3d2cd1b3 100644 --- a/src/btreemap.rs +++ b/src/btreemap.rs @@ -765,9 +765,8 @@ where } let root = self.load_node(self.root_addr); - self.remove_rightmost(root).map(|(k, v)| { - (k, V::from_bytes(Cow::Owned(v))) - }) + self.remove_rightmost(root) + .map(|(k, v)| (k, V::from_bytes(Cow::Owned(v)))) } /// Removes and returns the first element in the map. The key of this element is the minimum key that was in the map @@ -777,9 +776,8 @@ where } let root = self.load_node(self.root_addr); - self.remove_leftmost(root).map(|(k, v)| { - (k, V::from_bytes(Cow::Owned(v))) - }) + self.remove_leftmost(root) + .map(|(k, v)| (k, V::from_bytes(Cow::Owned(v)))) } /// A helper method for recursively removing a key from the B-tree. @@ -1179,13 +1177,9 @@ where if left_sibling.can_remove_entry_without_merging() { // Rotate right: left_sibling -> parent -> child - let (left_key, left_value) = - left_sibling.pop_entry(self.memory()).unwrap(); - let (parent_key, parent_value) = node.swap_entry( - last_idx - 1, - (left_key, left_value), - self.memory(), - ); + let (left_key, left_value) = left_sibling.pop_entry(self.memory()).unwrap(); + let (parent_key, parent_value) = + node.swap_entry(last_idx - 1, (left_key, left_value), self.memory()); child.insert_entry(0, (parent_key, parent_value)); if let Some(last_child) = left_sibling.pop_child() { @@ -1253,13 +1247,8 @@ where if right_sibling.can_remove_entry_without_merging() { // Rotate left: right_sibling -> parent -> child - let (right_key, right_value) = - right_sibling.remove_entry(0, self.memory()); - let parent_entry = node.swap_entry( - 0, - (right_key, right_value), - self.memory(), - ); + let (right_key, right_value) = right_sibling.remove_entry(0, self.memory()); + let parent_entry = node.swap_entry(0, (right_key, right_value), self.memory()); child.push_entry(parent_entry); if right_sibling.node_type() == NodeType::Internal { @@ -1273,11 +1262,7 @@ where } // Both at minimum: merge child into right sibling. - let merged = self.merge( - child, - right_sibling, - node.remove_entry(0, self.memory()), - ); + let merged = self.merge(child, right_sibling, node.remove_entry(0, self.memory())); node.remove_child(0); if node.entries_len() == 0 {