Do not unnecessarily compute `find_max_ordering` in bounds tree insertion

Lukas Wirth created 1 week ago

Change summary

crates/gpui/src/bounds_tree.rs  | 29 +++++++++++------------------
crates/gpui/src/key_dispatch.rs |  8 ++++----
2 files changed, 15 insertions(+), 22 deletions(-)

Detailed changes

crates/gpui/src/bounds_tree.rs

@@ -41,8 +41,9 @@ where
         }
 
         // Search for the best place to add the new leaf based on heuristics.
-        let mut max_intersecting_ordering = 0;
         let mut index = self.root.unwrap();
+        let mut max_intersecting_ordering = self.find_max_ordering(index, &new_bounds, 0);
+
         while let Node::Internal {
             left,
             right,
@@ -63,15 +64,7 @@ where
             let right_cost = new_bounds
                 .union(self.nodes[right].bounds())
                 .half_perimeter();
-            if left_cost < right_cost {
-                max_intersecting_ordering =
-                    self.find_max_ordering(right, &new_bounds, max_intersecting_ordering);
-                index = left;
-            } else {
-                max_intersecting_ordering =
-                    self.find_max_ordering(left, &new_bounds, max_intersecting_ordering);
-                index = right;
-            }
+            index = if left_cost < right_cost { left } else { right };
         }
 
         // We've found a leaf ('index' now refers to a leaf node).
@@ -201,14 +194,14 @@ where
     U: Clone + Debug + Default + PartialEq,
 {
     Leaf {
-        bounds: Bounds<U>,
         order: u32,
+        bounds: Bounds<U>,
     },
     Internal {
+        max_order: u32,
+        bounds: Bounds<U>,
         left: usize,
         right: usize,
-        bounds: Bounds<U>,
-        max_order: u32,
     },
 }
 
@@ -225,13 +218,13 @@ where
 
     fn max_ordering(&self) -> u32 {
         match self {
-            Node::Leaf {
+            &Node::Leaf {
                 order: ordering, ..
-            } => *ordering,
-            Node::Internal {
-                max_order: max_ordering,
+            }
+            | &Node::Internal {
+                max_order: ordering,
                 ..
-            } => *max_ordering,
+            } => ordering,
         }
     }
 }

crates/gpui/src/key_dispatch.rs

@@ -447,7 +447,7 @@ impl DispatchTree {
     fn bindings_for_input(
         &self,
         input: &[Keystroke],
-        dispatch_path: &SmallVec<[DispatchNodeId; 32]>,
+        dispatch_path: &[DispatchNodeId],
     ) -> (SmallVec<[KeyBinding; 1]>, bool, Vec<KeyContext>) {
         let context_stack: Vec<KeyContext> = dispatch_path
             .iter()
@@ -472,7 +472,7 @@ impl DispatchTree {
         &mut self,
         mut input: SmallVec<[Keystroke; 1]>,
         keystroke: Keystroke,
-        dispatch_path: &SmallVec<[DispatchNodeId; 32]>,
+        dispatch_path: &[DispatchNodeId],
     ) -> DispatchResult {
         input.push(keystroke.clone());
         let (bindings, pending, context_stack) = self.bindings_for_input(&input, dispatch_path);
@@ -510,7 +510,7 @@ impl DispatchTree {
     pub fn flush_dispatch(
         &mut self,
         input: SmallVec<[Keystroke; 1]>,
-        dispatch_path: &SmallVec<[DispatchNodeId; 32]>,
+        dispatch_path: &[DispatchNodeId],
     ) -> SmallVec<[Replay; 1]> {
         let (suffix, mut to_replay) = self.replay_prefix(input, dispatch_path);
 
@@ -525,7 +525,7 @@ impl DispatchTree {
     fn replay_prefix(
         &self,
         mut input: SmallVec<[Keystroke; 1]>,
-        dispatch_path: &SmallVec<[DispatchNodeId; 32]>,
+        dispatch_path: &[DispatchNodeId],
     ) -> (SmallVec<[Keystroke; 1]>, SmallVec<[Replay; 1]>) {
         let mut to_replay: SmallVec<[Replay; 1]> = Default::default();
         for last in (0..input.len()).rev() {