From 6628c4df28b61a60502a5e9e80655ad3d6ee3771 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 23 May 2023 14:46:14 -0700 Subject: [PATCH] Store worktree changes in a sorted arc slice, not a HashMap We don't need to look-up change types by an arbitrary key, but we do need to iterate it. It would also be useful to be able to cheaply clone the changes, to use them in a background task. --- crates/project/src/project.rs | 10 +++---- crates/project/src/worktree.rs | 51 ++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 56d5a3317c..16650b4eac 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4847,7 +4847,7 @@ impl Project { if worktree.read(cx).is_local() { cx.subscribe(worktree, |this, worktree, event, cx| match event { worktree::Event::UpdatedEntries(changes) => { - this.update_local_worktree_buffers(&worktree, &changes, cx); + this.update_local_worktree_buffers(&worktree, changes, cx); this.update_local_worktree_language_servers(&worktree, changes, cx); } worktree::Event::UpdatedGitRepositories(updated_repos) => { @@ -4881,13 +4881,13 @@ impl Project { fn update_local_worktree_buffers( &mut self, worktree_handle: &ModelHandle, - changes: &HashMap<(Arc, ProjectEntryId), PathChange>, + changes: &[(Arc, ProjectEntryId, PathChange)], cx: &mut ModelContext, ) { let snapshot = worktree_handle.read(cx).snapshot(); let mut renamed_buffers = Vec::new(); - for (path, entry_id) in changes.keys() { + for (path, entry_id, _) in changes { let worktree_id = worktree_handle.read(cx).id(); let project_path = ProjectPath { worktree_id, @@ -4993,7 +4993,7 @@ impl Project { fn update_local_worktree_language_servers( &mut self, worktree_handle: &ModelHandle, - changes: &HashMap<(Arc, ProjectEntryId), PathChange>, + changes: &[(Arc, ProjectEntryId, PathChange)], cx: &mut ModelContext, ) { if changes.is_empty() { @@ -5024,7 +5024,7 @@ impl Project { let params = lsp::DidChangeWatchedFilesParams { changes: changes .iter() - .filter_map(|((path, _), change)| { + .filter_map(|(path, _, change)| { if !watched_paths.is_match(&path) { return None; } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 336a02f1ef..1f02910404 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -363,7 +363,7 @@ enum ScanState { Started, Updated { snapshot: LocalSnapshot, - changes: HashMap<(Arc, ProjectEntryId), PathChange>, + changes: Arc<[(Arc, ProjectEntryId, PathChange)]>, barrier: Option, scanning: bool, }, @@ -377,7 +377,7 @@ struct ShareState { } pub enum Event { - UpdatedEntries(HashMap<(Arc, ProjectEntryId), PathChange>), + UpdatedEntries(Arc<[(Arc, ProjectEntryId, PathChange)]>), UpdatedGitRepositories(HashMap, LocalRepositoryEntry>), } @@ -548,7 +548,7 @@ impl Worktree { this.update(&mut cx, |this, cx| { let this = this.as_remote_mut().unwrap(); this.snapshot = this.background_snapshot.lock().clone(); - cx.emit(Event::UpdatedEntries(Default::default())); + cx.emit(Event::UpdatedEntries(Arc::from([]))); cx.notify(); while let Some((scan_id, _)) = this.snapshot_subscriptions.front() { if this.observed_snapshot(*scan_id) { @@ -3396,18 +3396,24 @@ impl BackgroundScanner { old_snapshot: &Snapshot, new_snapshot: &Snapshot, event_paths: &[Arc], - ) -> HashMap<(Arc, ProjectEntryId), PathChange> { + ) -> Arc<[(Arc, ProjectEntryId, PathChange)]> { use BackgroundScannerPhase::*; use PathChange::{Added, AddedOrUpdated, Loaded, Removed, Updated}; - let mut changes = HashMap::default(); let mut old_paths = old_snapshot.entries_by_path.cursor::(); let mut new_paths = new_snapshot.entries_by_path.cursor::(); + old_paths.next(&()); + new_paths.next(&()); + let mut changes = Vec::new(); for path in event_paths { let path = PathKey(path.clone()); - old_paths.seek(&path, Bias::Left, &()); - new_paths.seek(&path, Bias::Left, &()); + if old_paths.item().map_or(false, |e| e.path < path.0) { + old_paths.seek_forward(&path, Bias::Left, &()); + } + if new_paths.item().map_or(false, |e| e.path < path.0) { + new_paths.seek_forward(&path, Bias::Left, &()); + } loop { match (old_paths.item(), new_paths.item()) { @@ -3422,7 +3428,7 @@ impl BackgroundScanner { match Ord::cmp(&old_entry.path, &new_entry.path) { Ordering::Less => { - changes.insert((old_entry.path.clone(), old_entry.id), Removed); + changes.push((old_entry.path.clone(), old_entry.id, Removed)); old_paths.next(&()); } Ordering::Equal => { @@ -3430,42 +3436,45 @@ impl BackgroundScanner { // If the worktree was not fully initialized when this event was generated, // we can't know whether this entry was added during the scan or whether // it was merely updated. - changes.insert( - (new_entry.path.clone(), new_entry.id), + changes.push(( + new_entry.path.clone(), + new_entry.id, AddedOrUpdated, - ); + )); } else if old_entry.mtime != new_entry.mtime { - changes.insert((new_entry.path.clone(), new_entry.id), Updated); + changes.push((new_entry.path.clone(), new_entry.id, Updated)); } old_paths.next(&()); new_paths.next(&()); } Ordering::Greater => { - changes.insert( - (new_entry.path.clone(), new_entry.id), + changes.push(( + new_entry.path.clone(), + new_entry.id, if self.phase == InitialScan { Loaded } else { Added }, - ); + )); new_paths.next(&()); } } } (Some(old_entry), None) => { - changes.insert((old_entry.path.clone(), old_entry.id), Removed); + changes.push((old_entry.path.clone(), old_entry.id, Removed)); old_paths.next(&()); } (None, Some(new_entry)) => { - changes.insert( - (new_entry.path.clone(), new_entry.id), + changes.push(( + new_entry.path.clone(), + new_entry.id, if self.phase == InitialScan { Loaded } else { Added }, - ); + )); new_paths.next(&()); } (None, None) => break, @@ -3473,7 +3482,7 @@ impl BackgroundScanner { } } - changes + changes.into() } async fn progress_timer(&self, running: bool) { @@ -4337,7 +4346,7 @@ mod tests { cx.subscribe(&worktree, move |tree, _, event, _| { if let Event::UpdatedEntries(changes) = event { - for ((path, _), change_type) in changes.iter() { + for (path, _, change_type) in changes.iter() { let mtime = tree.entry_for_path(&path).map(|e| e.mtime); let path = path.clone(); let ix = match paths.binary_search_by_key(&&path, |e| &e.0) {