From a59b75c83998dd4a28a7e58744fe349b91724978 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Apr 2021 14:02:55 -0700 Subject: [PATCH 1/3] Keep results stable when using file-finder while scanning files --- zed/src/file_finder.rs | 85 +++++++++++++++++++++++++++++++++++++-- zed/src/util.rs | 37 +++++++++++++++++ zed/src/worktree/fuzzy.rs | 10 +++-- 3 files changed, 125 insertions(+), 7 deletions(-) diff --git a/zed/src/file_finder.rs b/zed/src/file_finder.rs index d6ec299593b4cbce237369304a8a028731042794..6a7c7280c1bd2b94db6b44ef262f4aa041ebd739 100644 --- a/zed/src/file_finder.rs +++ b/zed/src/file_finder.rs @@ -30,6 +30,8 @@ pub struct FileFinder { query_buffer: ViewHandle, search_count: usize, latest_search_id: usize, + latest_search_did_cancel: bool, + latest_search_query: String, matches: Vec, include_root_name: bool, selected: Option>, @@ -293,6 +295,8 @@ impl FileFinder { query_buffer, search_count: 0, latest_search_id: 0, + latest_search_did_cancel: false, + latest_search_query: String::new(), matches: Vec::new(), include_root_name: false, selected: None, @@ -395,10 +399,11 @@ impl FileFinder { false, false, 100, - cancel_flag, + cancel_flag.clone(), pool, ); - (search_id, include_root_name, matches) + let did_cancel = cancel_flag.load(atomic::Ordering::Relaxed); + (search_id, include_root_name, did_cancel, query, matches) }); ctx.spawn(task, Self::update_matches).detach(); @@ -406,12 +411,24 @@ impl FileFinder { fn update_matches( &mut self, - (search_id, include_root_name, matches): (usize, bool, Vec), + (search_id, include_root_name, did_cancel, query, matches): ( + usize, + bool, + bool, + String, + Vec, + ), ctx: &mut ViewContext, ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; - self.matches = matches; + if did_cancel && self.latest_search_did_cancel && query == self.latest_search_query { + util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); + } else { + self.matches = matches; + self.latest_search_did_cancel = did_cancel; + self.latest_search_query = query; + } self.include_root_name = include_root_name; self.list_state.scroll_to(self.selected_index()); ctx.notify(); @@ -432,9 +449,11 @@ mod tests { use super::*; use crate::{ editor, settings, + test::temp_tree, workspace::{Workspace, WorkspaceView}, }; use gpui::App; + use serde_json::json; use smol::fs; use tempdir::TempDir; @@ -508,4 +527,62 @@ mod tests { }); }); } + + #[test] + fn test_matching_cancellation() { + App::test_async((), |mut app| async move { + let tmp_dir = temp_tree(json!({ + "hello": "", + "goodbye": "", + "halogen-light": "", + "happiness": "", + "height": "", + "hi": "", + "hiccup": "", + })); + let settings = settings::channel(&app.font_cache()).unwrap().1; + let workspace = app.add_model(|ctx| Workspace::new(vec![tmp_dir.path().into()], ctx)); + app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx)) + .await; + let (_, finder) = + app.add_window(|ctx| FileFinder::new(settings, workspace.clone(), ctx)); + + let query = "hi".to_string(); + finder.update(&mut app, |f, ctx| f.spawn_search(query.clone(), ctx)); + finder.condition(&app, |f, _| f.matches.len() == 5).await; + + finder.update(&mut app, |finder, ctx| { + let matches = finder.matches.clone(); + + // Simulate a search being cancelled after the time limit, + // returning only a subset of the matches that would have been found. + finder.spawn_search(query.clone(), ctx); + finder.update_matches( + ( + finder.latest_search_id, + true, + true, // did-cancel + query.clone(), + vec![matches[1].clone(), matches[3].clone()], + ), + ctx, + ); + + // Simulate another cancellation. + finder.spawn_search(query.clone(), ctx); + finder.update_matches( + ( + finder.latest_search_id, + true, + true, // did-cancel + query.clone(), + vec![matches[0].clone(), matches[2].clone(), matches[3].clone()], + ), + ctx, + ); + + assert_eq!(finder.matches, matches[0..4]) + }); + }); + } } diff --git a/zed/src/util.rs b/zed/src/util.rs index 70f2b1a6d6485445862042fe749ca6d69714b23d..09ce2f87e155c30c996f75f5b1a8a62127ff9c4f 100644 --- a/zed/src/util.rs +++ b/zed/src/util.rs @@ -7,6 +7,29 @@ pub fn post_inc(value: &mut usize) -> usize { prev } +/// Extend a sorted vector with a sorted sequence of items, maintaining the vector's sort order and +/// enforcing a maximum length. Sort the items according to the given callback. Before calling this, +/// both `vec` and `new_items` should already be sorted according to the `cmp` comparator. +pub fn extend_sorted(vec: &mut Vec, new_items: I, limit: usize, mut cmp: F) +where + I: IntoIterator, + F: FnMut(&T, &T) -> Ordering, +{ + let mut start_index = 0; + for new_item in new_items { + if let Err(i) = vec[start_index..].binary_search_by(|m| cmp(m, &new_item)) { + let index = start_index + i; + if vec.len() < limit { + vec.insert(index, new_item); + } else if index < vec.len() { + vec.pop(); + vec.insert(index, new_item); + } + start_index = index; + } + } +} + pub fn find_insertion_index<'a, F, T, E>(slice: &'a [T], mut f: F) -> Result where F: FnMut(&'a T) -> Result, @@ -69,4 +92,18 @@ mod tests { Ok(1) ); } + + #[test] + fn test_extend_sorted() { + let mut vec = vec![]; + + extend_sorted(&mut vec, vec![21, 17, 13, 8, 1, 0], 5, |a, b| b.cmp(a)); + assert_eq!(vec, &[21, 17, 13, 8, 1]); + + extend_sorted(&mut vec, vec![101, 19, 17, 8, 2], 8, |a, b| b.cmp(a)); + assert_eq!(vec, &[101, 21, 19, 17, 13, 8, 2, 1]); + + extend_sorted(&mut vec, vec![1000, 19, 17, 9, 5], 8, |a, b| b.cmp(a)); + assert_eq!(vec, &[1000, 101, 21, 19, 17, 13, 9, 8]); + } } diff --git a/zed/src/worktree/fuzzy.rs b/zed/src/worktree/fuzzy.rs index 10a1cace3196de0a5f6392b0839efcd86ab5deb5..3831084e0ac863280d755d90cc62de754568fa8f 100644 --- a/zed/src/worktree/fuzzy.rs +++ b/zed/src/worktree/fuzzy.rs @@ -36,13 +36,17 @@ impl Eq for PathMatch {} impl PartialOrd for PathMatch { fn partial_cmp(&self, other: &Self) -> Option { - self.score.partial_cmp(&other.score) + Some(self.cmp(other)) } } impl Ord for PathMatch { fn cmp(&self, other: &Self) -> Ordering { - self.partial_cmp(other).unwrap_or(Ordering::Equal) + self.score + .partial_cmp(&other.score) + .unwrap_or(Ordering::Equal) + .then_with(|| self.tree_id.cmp(&other.tree_id)) + .then_with(|| Arc::as_ptr(&self.path).cmp(&Arc::as_ptr(&other.path))) } } @@ -150,7 +154,7 @@ where .flatten() .map(|r| r.0) .collect::>(); - results.sort_unstable_by(|a, b| b.score.partial_cmp(&a.score).unwrap()); + results.sort_unstable_by(|a, b| b.cmp(&a)); results.truncate(max_results); results } From 75b8f7425d52425d60d979beb04ee275a9540140 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Apr 2021 14:34:29 -0700 Subject: [PATCH 2/3] Avoid redundant `sort_unstable_by` call on merged fuzzy matches --- zed/src/worktree/fuzzy.rs | 48 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/zed/src/worktree/fuzzy.rs b/zed/src/worktree/fuzzy.rs index 3831084e0ac863280d755d90cc62de754568fa8f..813147d929b0f4ebad3f0bc839861148f1bad4b1 100644 --- a/zed/src/worktree/fuzzy.rs +++ b/zed/src/worktree/fuzzy.rs @@ -1,8 +1,8 @@ use super::{char_bag::CharBag, EntryKind, Snapshot}; +use crate::util; use gpui::scoped_pool; use std::{ - cmp::{max, min, Ordering, Reverse}, - collections::BinaryHeap, + cmp::{max, min, Ordering}, path::Path, sync::atomic::{self, AtomicBool}, sync::Arc, @@ -78,7 +78,9 @@ where }; let segment_size = (path_count + cpus - 1) / cpus; - let mut segment_results = (0..cpus).map(|_| BinaryHeap::new()).collect::>(); + let mut segment_results = (0..cpus) + .map(|_| Vec::with_capacity(max_results)) + .collect::>(); pool.scoped(|scope| { for (segment_idx, results) in segment_results.iter_mut().enumerate() { @@ -149,13 +151,14 @@ where } }); - let mut results = segment_results - .into_iter() - .flatten() - .map(|r| r.0) - .collect::>(); - results.sort_unstable_by(|a, b| b.cmp(&a)); - results.truncate(max_results); + let mut results = Vec::new(); + for segment_result in segment_results { + if results.is_empty() { + results = segment_result; + } else { + util::extend_sorted(&mut results, segment_result, max_results, |a, b| b.cmp(&a)); + } + } results } @@ -167,7 +170,7 @@ fn match_single_tree_paths<'a>( lowercase_query: &[char], query_chars: CharBag, smart_case: bool, - results: &mut BinaryHeap>, + results: &mut Vec, max_results: usize, min_score: &mut f64, match_positions: &mut Vec, @@ -238,14 +241,22 @@ fn match_single_tree_paths<'a>( ); if score > 0.0 { - results.push(Reverse(PathMatch { + let mat = PathMatch { tree_id: snapshot.id, path: candidate.path.clone(), score, positions: match_positions.clone(), - })); - if results.len() == max_results { - *min_score = results.peek().unwrap().0.score; + }; + if let Err(i) = results.binary_search_by(|m| mat.cmp(&m)) { + if results.len() < max_results { + results.insert(i, mat); + } else if i < results.len() { + results.pop(); + results.insert(i, mat); + } + if results.len() == max_results { + *min_score = results.last().unwrap().score; + } } } } @@ -564,7 +575,7 @@ mod tests { last_positions.resize(query.len(), 0); let cancel_flag = AtomicBool::new(false); - let mut results = BinaryHeap::new(); + let mut results = Vec::new(); match_single_tree_paths( &Snapshot { id: 0, @@ -592,15 +603,14 @@ mod tests { results .into_iter() - .rev() .map(|result| { ( paths .iter() .copied() - .find(|p| result.0.path.as_ref() == Path::new(p)) + .find(|p| result.path.as_ref() == Path::new(p)) .unwrap(), - result.0.positions, + result.positions, ) }) .collect() From eec8a3b555d4e96365cfc78abe01a5f5cd0b673e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Apr 2021 14:58:54 -0700 Subject: [PATCH 3/3] Simplify file finder update_matches logic --- zed/src/file_finder.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zed/src/file_finder.rs b/zed/src/file_finder.rs index 6a7c7280c1bd2b94db6b44ef262f4aa041ebd739..e9eb580340ab150107f3b35b828eb1e1f16c2e36 100644 --- a/zed/src/file_finder.rs +++ b/zed/src/file_finder.rs @@ -422,13 +422,13 @@ impl FileFinder { ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; - if did_cancel && self.latest_search_did_cancel && query == self.latest_search_query { + if self.latest_search_did_cancel && query == self.latest_search_query { util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); } else { self.matches = matches; - self.latest_search_did_cancel = did_cancel; - self.latest_search_query = query; } + self.latest_search_query = query; + self.latest_search_did_cancel = did_cancel; self.include_root_name = include_root_name; self.list_state.scroll_to(self.selected_index()); ctx.notify();