From 563f13925fc7fc488788719f2fdf812a2e56a5f3 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 1 May 2023 16:29:14 -0700 Subject: [PATCH] WIP: Convert old git repository vec to new treemap based approach. co-authored-by: Nathan --- crates/project/src/project.rs | 16 ++-- crates/project/src/worktree.rs | 155 ++++++++++++-------------------- crates/sum_tree/src/tree_map.rs | 19 ++++ 3 files changed, 84 insertions(+), 106 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 7eede73c75fd2f6e3905fd58abb9bde373b8541d..3d59aefde30c33c294f846286d3e026be0affcd2 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -64,6 +64,7 @@ use std::{ }, time::{Duration, Instant, SystemTime}, }; +use sum_tree::TreeMap; use terminals::Terminals; use util::{debug_panic, defer, merge_json_value_into, post_inc, ResultExt, TryFutureExt as _}; @@ -4696,7 +4697,7 @@ impl Project { fn update_local_worktree_buffers_git_repos( &mut self, worktree: ModelHandle, - repos: &[LocalGitRepositoryEntry], + repos: &TreeMap, cx: &mut ModelContext, ) { for (_, buffer) in &self.opened_buffers { @@ -4711,14 +4712,17 @@ impl Project { let path = file.path().clone(); - let repo = match repos.iter().find(|repo| repo.manages(&path)) { - Some(repo) => repo.clone(), + let (work_directory, repo) = match repos + .iter() + .find(|(work_directory, _)| work_directory.contains(&path)) + { + Some((work_directory, repo)) => (work_directory, repo.clone()), None => return, }; - let relative_repo = match path.strip_prefix(repo.content_path) { - Ok(relative_repo) => relative_repo.to_owned(), - Err(_) => return, + let relative_repo = match work_directory.relativize(&path) { + Some(relative_repo) => relative_repo.to_owned(), + None => return, }; let remote_id = self.remote_id(); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 2746bc95d225246ee5054f73b18f41fb4b967559..d49b71135b9021453dd3d9c16bda12546a387db8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -102,7 +102,7 @@ pub struct Snapshot { root_char_bag: CharBag, entries_by_path: SumTree, entries_by_id: SumTree, - repository_entries: TreeMap, + repository_entries: TreeMap, /// A number that increases every time the worktree begins scanning /// a set of paths from the filesystem. This scanning could be caused @@ -136,39 +136,28 @@ impl RepositoryEntry { /// This path corresponds to the 'content path' (the folder that contains the .git) #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] -pub struct RepositoryEntryKey(Arc); +pub struct RepositoryWorkDirectory(Arc); -impl Default for RepositoryEntryKey { - fn default() -> Self { - RepositoryEntryKey(Arc::from(Path::new(""))) +impl RepositoryWorkDirectory { + // Note that these paths should be relative to the worktree root. + pub(crate) fn contains(&self, path: &Path) -> bool { + path.starts_with(self.0.as_ref()) } -} -#[derive(Clone)] -pub struct LocalGitRepositoryEntry { - pub(crate) repo: Arc>, - - pub(crate) scan_id: usize, - // Path to folder containing the .git file or directory - pub(crate) content_path: Arc, - // Path to the actual .git folder. - // Note: if .git is a file, this points to the folder indicated by the .git file - pub(crate) git_dir_path: Arc, + pub(crate) fn relativize(&self, path: &Path) -> Option<&Path> { + path.strip_prefix(self.0.as_ref()).ok() + } } -impl std::fmt::Debug for LocalGitRepositoryEntry { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("GitRepositoryEntry") - .field("content_path", &self.content_path) - .field("git_dir_path", &self.git_dir_path) - .finish() +impl Default for RepositoryWorkDirectory { + fn default() -> Self { + RepositoryWorkDirectory(Arc::from(Path::new(""))) } } #[derive(Debug, Clone)] pub struct LocalSnapshot { ignores_by_parent_abs_path: HashMap, (Arc, usize)>, - git_repositories_old: Vec, // The ProjectEntryId corresponds to the entry for the .git dir git_repositories: TreeMap>>, removed_entry_ids: HashMap, @@ -209,7 +198,7 @@ struct ShareState { pub enum Event { UpdatedEntries(HashMap, PathChange>), - UpdatedGitRepositories(Vec), + UpdatedGitRepositories(Vec), } impl Entity for Worktree { @@ -240,7 +229,6 @@ impl Worktree { let mut snapshot = LocalSnapshot { ignores_by_parent_abs_path: Default::default(), - git_repositories_old: Default::default(), removed_entry_ids: Default::default(), git_repositories: Default::default(), next_entry_id, @@ -620,8 +608,8 @@ impl LocalWorktree { fn set_snapshot(&mut self, new_snapshot: LocalSnapshot, cx: &mut ModelContext) { let updated_repos = Self::changed_repos( - &self.snapshot.git_repositories_old, - &new_snapshot.git_repositories_old, + &self.snapshot.repository_entries, + &new_snapshot.repository_entries, ); self.snapshot = new_snapshot; @@ -635,16 +623,16 @@ impl LocalWorktree { } fn changed_repos( - old_repos: &[LocalGitRepositoryEntry], - new_repos: &[LocalGitRepositoryEntry], - ) -> Vec { + old_repos: &TreeMap, + new_repos: &TreeMap, + ) -> Vec { fn diff<'a>( - a: &'a [LocalGitRepositoryEntry], - b: &'a [LocalGitRepositoryEntry], - updated: &mut HashMap<&'a Path, LocalGitRepositoryEntry>, + a: impl Iterator, + b: impl Iterator, + updated: &mut HashMap<&'a Path, RepositoryEntry>, ) { for a_repo in a { - let matched = b.iter().find(|b_repo| { + let matched = b.find(|b_repo| { a_repo.git_dir_path == b_repo.git_dir_path && a_repo.scan_id == b_repo.scan_id }); @@ -654,10 +642,10 @@ impl LocalWorktree { } } - let mut updated = HashMap::<&Path, LocalGitRepositoryEntry>::default(); + let mut updated = HashMap::<&Path, RepositoryEntry>::default(); - diff(old_repos, new_repos, &mut updated); - diff(new_repos, old_repos, &mut updated); + diff(old_repos.values(), new_repos.values(), &mut updated); + diff(new_repos.values(), old_repos.values(), &mut updated); updated.into_values().collect() } @@ -1023,7 +1011,6 @@ impl LocalWorktree { let mut share_tx = Some(share_tx); let mut prev_snapshot = LocalSnapshot { ignores_by_parent_abs_path: Default::default(), - git_repositories_old: Default::default(), removed_entry_ids: Default::default(), next_entry_id: Default::default(), git_repositories: Default::default(), @@ -1432,18 +1419,16 @@ impl Snapshot { impl LocalSnapshot { // Gives the most specific git repository for a given path - pub(crate) fn repo_for(&self, path: &Path) -> Option { - self.git_repositories_old - .iter() - .rev() //git_repository is ordered lexicographically - .find(|repo| repo.manages(path)) - .cloned() + pub(crate) fn repo_for(&self, path: &Path) -> Option { + self.repository_entries + .closest(&RepositoryWorkDirectory(path.into())) + .map(|(_, entry)| entry.to_owned()) } pub(crate) fn repo_with_dot_git_containing( &mut self, path: &Path, - ) -> Option<&mut LocalGitRepositoryEntry> { + ) -> Option<&mut RepositoryEntry> { // Git repositories cannot be nested, so we don't need to reverse the order self.git_repositories_old .iter_mut() @@ -1621,7 +1606,7 @@ impl LocalSnapshot { let abs_path = self.abs_path.join(&parent_path); let content_path: Arc = parent_path.parent().unwrap().into(); - let key = RepositoryEntryKey(content_path.clone()); + let key = RepositoryWorkDirectory(content_path.clone()); if self.repository_entries.get(&key).is_none() { if let Some(repo) = fs.open_repo(abs_path.as_path()) { self.repository_entries.insert( @@ -1636,23 +1621,6 @@ impl LocalSnapshot { self.git_repositories.insert(parent_entry.id, repo) } } - - if let Err(ix) = self - .git_repositories_old - .binary_search_by_key(&&content_path, |repo| &repo.content_path) - { - if let Some(repo) = fs.open_repo(abs_path.as_path()) { - self.git_repositories_old.insert( - ix, - LocalGitRepositoryEntry { - repo, - scan_id: 0, - content_path, - git_dir_path: parent_path, - }, - ); - } - } } let mut entries_by_path_edits = vec![Edit::Insert(parent_entry)]; @@ -1712,18 +1680,10 @@ impl LocalSnapshot { *scan_id = self.snapshot.scan_id; } } else if path.file_name() == Some(&DOT_GIT) { - let repo_entry_key = RepositoryEntryKey(path.parent().unwrap().into()); + let repo_entry_key = RepositoryWorkDirectory(path.parent().unwrap().into()); self.snapshot .repository_entries .update(&repo_entry_key, |repo| repo.scan_id = self.snapshot.scan_id); - - let parent_path = path.parent().unwrap(); - if let Ok(ix) = self - .git_repositories_old - .binary_search_by_key(&parent_path, |repo| repo.content_path.as_ref()) - { - self.git_repositories_old[ix].scan_id = self.snapshot.scan_id; - } } } @@ -1763,22 +1723,6 @@ impl LocalSnapshot { ignore_stack } - - pub fn git_repo_entries(&self) -> &[LocalGitRepositoryEntry] { - &self.git_repositories_old - } -} - -impl LocalGitRepositoryEntry { - // Note that these paths should be relative to the worktree root. - pub(crate) fn manages(&self, path: &Path) -> bool { - path.starts_with(self.content_path.as_ref()) - } - - // Note that this path should be relative to the worktree root. - pub(crate) fn in_dot_git(&self, path: &Path) -> bool { - path.starts_with(self.git_dir_path.as_ref()) - } } async fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { @@ -2364,10 +2308,6 @@ impl BackgroundScanner { let mut snapshot = self.snapshot.lock(); - let mut git_repositories = mem::take(&mut snapshot.git_repositories_old); - git_repositories.retain(|repo| snapshot.entry_for_path(&repo.git_dir_path).is_some()); - snapshot.git_repositories_old = git_repositories; - let mut git_repositories = mem::take(&mut snapshot.git_repositories); git_repositories.retain(|project_entry_id, _| snapshot.contains_entry(*project_entry_id)); snapshot.git_repositories = git_repositories; @@ -3489,26 +3429,41 @@ mod tests { #[test] fn test_changed_repos() { - fn fake_entry(git_dir_path: impl AsRef, scan_id: usize) -> LocalGitRepositoryEntry { - LocalGitRepositoryEntry { - repo: Arc::new(Mutex::new(FakeGitRepository::default())), + fn fake_entry(git_dir_path: impl AsRef, scan_id: usize) -> RepositoryEntry { + RepositoryEntry { scan_id, - content_path: git_dir_path.as_ref().parent().unwrap().into(), git_dir_path: git_dir_path.as_ref().into(), + git_dir_entry_id: ProjectEntryId(0), } } - let prev_repos: Vec = vec![ + let mut prev_repos = TreeMap::::default(); + prev_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-1").into()), fake_entry("/.git", 0), + ); + prev_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-2").into()), fake_entry("/a/.git", 0), + ); + prev_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-3").into()), fake_entry("/a/b/.git", 0), - ]; + ); - let new_repos: Vec = vec![ + let mut new_repos = TreeMap::::default(); + new_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-4").into()), fake_entry("/a/.git", 1), + ); + new_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-5").into()), fake_entry("/a/b/.git", 0), + ); + new_repos.insert( + RepositoryWorkDirectory(Path::new("don't-care-6").into()), fake_entry("/a/c/.git", 0), - ]; + ); let res = LocalWorktree::changed_repos(&prev_repos, &new_repos); diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index ab44aa9f09c21cd730295660ff99bb082817a919..2580b08783b4e69d0228d0f0c2662a4259f347a0 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -73,6 +73,15 @@ impl TreeMap { removed } + /// Returns the key-value pair with the greatest key less than or equal to the given key. + pub fn closest(&self, key: &K) -> Option<(&K, &V)> { + let mut cursor = self.0.cursor::>(); + let key = MapKeyRef(Some(key)); + cursor.seek(&key, Bias::Right, &()); + cursor.prev(&()); + cursor.item().map(|item| (&item.key, &item.value)) + } + pub fn update(&mut self, key: &K, f: F) -> Option where F: FnOnce(&mut V) -> T, @@ -112,6 +121,10 @@ impl TreeMap { pub fn iter(&self) -> impl Iterator + '_ { self.0.iter().map(|entry| (&entry.key, &entry.value)) } + + pub fn values(&self) -> impl Iterator + '_ { + self.0.iter().map(|entry| &entry.value) + } } impl Default for TreeMap @@ -235,10 +248,16 @@ mod tests { vec![(&1, &"a"), (&2, &"b"), (&3, &"c")] ); + assert_eq!(map.closest(&0), None); + assert_eq!(map.closest(&1), Some((&1, &"a"))); + assert_eq!(map.closest(&10), Some((&3, &"c"))); + map.remove(&2); assert_eq!(map.get(&2), None); assert_eq!(map.iter().collect::>(), vec![(&1, &"a"), (&3, &"c")]); + assert_eq!(map.closest(&2), Some((&1, &"a"))); + map.remove(&3); assert_eq!(map.get(&3), None); assert_eq!(map.iter().collect::>(), vec![(&1, &"a")]);