From 0a78c676475d8e3ca3d569f22e8bce0d3291cff5 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Thu, 25 Jan 2024 12:56:12 +0100 Subject: [PATCH] Improve performance of select-all-matches This fixes #6440. The previous approach was calling select-next-match in a loop, which leaves optimizations on the table when you already know that you want to select all of the matches. So what we did here is to optimize the code for the "give me all matches" case: 1. Find all results in the current buffer 2. Build up *all* selections 3. Sort selections & throw away overlapping ones (keep oldest) 4. Unfold if necessary 5. Render selections On my M3 Max searching for ` --- crates/editor/src/actions.rs | 10 +---- crates/editor/src/editor.rs | 70 +++++++++++++++++++++++++++---- crates/editor/src/editor_tests.rs | 12 ++++++ 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 4edc1d12ea744c53f4e9878573684a95387c0ea5..9cfaeaaf4a274d5765c8c326a5f7880996302e2e 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -13,12 +13,6 @@ pub struct SelectPrevious { pub replace_newest: bool, } -#[derive(PartialEq, Clone, Deserialize, Default)] -pub struct SelectAllMatches { - #[serde(default)] - pub replace_newest: bool, -} - #[derive(PartialEq, Clone, Deserialize, Default)] pub struct SelectToBeginningOfLine { #[serde(default)] @@ -81,7 +75,6 @@ impl_actions!( [ SelectNext, SelectPrevious, - SelectAllMatches, SelectToBeginningOfLine, MovePageUp, MovePageDown, @@ -128,6 +121,7 @@ gpui::actions!( DeleteToNextWordEnd, DeleteToPreviousSubwordStart, DeleteToPreviousWordStart, + DisplayCursorNames, DuplicateLine, ExpandMacroRecursively, FindAllReferences, @@ -185,6 +179,7 @@ gpui::actions!( ScrollCursorCenter, ScrollCursorTop, SelectAll, + SelectAllMatches, SelectDown, SelectLargerSyntaxNode, SelectLeft, @@ -214,6 +209,5 @@ gpui::actions!( Undo, UndoSelection, UnfoldLines, - DisplayCursorNames ] ); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9dd41fa25d5cbf650b4fd36a6462f3978bfdffbe..d15c2591b3e258a0fb6aad8b58c53726e14f7427 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6113,6 +6113,7 @@ impl Editor { || (!movement::is_inside_word(&display_map, display_range.start) && !movement::is_inside_word(&display_map, display_range.end)) { + // TODO: This is n^2, because we might check all the selections if selections .iter() .find(|selection| selection.range().overlaps(&offset_range)) @@ -6222,25 +6223,76 @@ impl Editor { pub fn select_all_matches( &mut self, - action: &SelectAllMatches, + _action: &SelectAllMatches, cx: &mut ViewContext, ) -> Result<()> { self.push_to_selection_history(); let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); - loop { - self.select_next_match_internal(&display_map, action.replace_newest, None, cx)?; + self.select_next_match_internal(&display_map, false, None, cx)?; + let Some(select_next_state) = self.select_next_state.as_mut() else { + return Ok(()); + }; + if select_next_state.done { + return Ok(()); + } + + let mut new_selections = self.selections.all::(cx); - if self - .select_next_state - .as_ref() - .map(|selection_state| selection_state.done) - .unwrap_or(true) + let buffer = &display_map.buffer_snapshot; + let query_matches = select_next_state + .query + .stream_find_iter(buffer.bytes_in_range(0..buffer.len())); + + for query_match in query_matches { + let query_match = query_match.unwrap(); // can only fail due to I/O + let offset_range = query_match.start()..query_match.end(); + let display_range = offset_range.start.to_display_point(&display_map) + ..offset_range.end.to_display_point(&display_map); + + if !select_next_state.wordwise + || (!movement::is_inside_word(&display_map, display_range.start) + && !movement::is_inside_word(&display_map, display_range.end)) { - break; + self.selections.change_with(cx, |selections| { + new_selections.push(Selection { + id: selections.new_selection_id(), + start: offset_range.start, + end: offset_range.end, + reversed: false, + goal: SelectionGoal::None, + }); + }); } } + new_selections.sort_by_key(|selection| selection.start); + let mut ix = 0; + while ix + 1 < new_selections.len() { + let current_selection = &new_selections[ix]; + let next_selection = &new_selections[ix + 1]; + if current_selection.range().overlaps(&next_selection.range()) { + if current_selection.id < next_selection.id { + new_selections.remove(ix + 1); + } else { + new_selections.remove(ix); + } + } else { + ix += 1; + } + } + + select_next_state.done = true; + self.unfold_ranges( + new_selections.iter().map(|selection| selection.range()), + false, + false, + cx, + ); + self.change_selections(Some(Autoscroll::fit()), cx, |selections| { + selections.select(new_selections) + }); + Ok(()) } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index a6e3d19995c2126e57b698178e8d0cac1bc264c5..03a601db997aaafabcc07a1b48bb66eb57fa0241 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -3820,6 +3820,18 @@ async fn test_select_next(cx: &mut gpui::TestAppContext) { cx.assert_editor_state("«abcˇ»\n«abcˇ» «abcˇ»\ndefabc\n«abcˇ»"); } +#[gpui::test] +async fn test_select_all_matches(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorTestContext::new(cx).await; + cx.set_state("abc\nˇabc abc\ndefabc\nabc"); + + cx.update_editor(|e, cx| e.select_all_matches(&SelectAllMatches::default(), cx)) + .unwrap(); + cx.assert_editor_state("«abcˇ»\n«abcˇ» «abcˇ»\ndefabc\n«abcˇ»"); +} + #[gpui::test] async fn test_select_next_with_multiple_carets(cx: &mut gpui::TestAppContext) { init_test(cx, |_| {});