Avoid storing redundant copies of file paths

Max Brunsfeld and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

zed/src/file_finder.rs    |  10 +-
zed/src/worktree.rs       | 126 ++++++++++++++++------------------------
zed/src/worktree/fuzzy.rs |  86 +++++++++++----------------
3 files changed, 91 insertions(+), 131 deletions(-)

Detailed changes

zed/src/file_finder.rs 🔗

@@ -146,12 +146,12 @@ impl FileFinder {
                 &[]
             };
             let path = path_match.path.clone();
-            let path_string = &path_match.path_string;
-            let file_name = Path::new(&path_string)
+            let path_string = path_match.path.to_string_lossy();
+            let file_name = path_match
+                .path
                 .file_name()
                 .unwrap_or_default()
-                .to_string_lossy()
-                .to_string();
+                .to_string_lossy();
 
             let path_positions = path_match.positions.clone();
             let file_name_start =
@@ -192,7 +192,7 @@ impl FileFinder {
                             Flex::column()
                                 .with_child(
                                     Label::new(
-                                        file_name,
+                                        file_name.to_string(),
                                         settings.ui_font_family,
                                         settings.ui_font_size,
                                     )

zed/src/worktree.rs 🔗

@@ -6,7 +6,6 @@ use crate::{
     sum_tree::{self, Cursor, Edit, SeekBias, SumTree},
 };
 use anyhow::{anyhow, Context, Result};
-use fuzzy::PathEntry;
 pub use fuzzy::{match_paths, PathMatch};
 use gpui::{scoped_pool, AppContext, Entity, ModelContext, ModelHandle, Task};
 use ignore::gitignore::Gitignore;
@@ -307,8 +306,8 @@ impl Snapshot {
         let mut edits = Vec::new();
 
         let mut parent_entry = self.entries.get(&PathKey(parent_path)).unwrap().clone();
-        if let Entry::Dir { pending, .. } = &mut parent_entry {
-            *pending = false;
+        if matches!(parent_entry.kind, EntryKind::PendingDir) {
+            parent_entry.kind = EntryKind::Dir;
         } else {
             unreachable!();
         }
@@ -384,54 +383,40 @@ impl FileHandle {
 }
 
 #[derive(Clone, Debug)]
-pub enum Entry {
-    Dir {
-        path: Arc<Path>,
-        inode: u64,
-        is_symlink: bool,
-        pending: bool,
-        is_ignored: Option<bool>,
-    },
-    File {
-        path: Arc<Path>,
-        inode: u64,
-        is_symlink: bool,
-        path_entry: PathEntry,
-        is_ignored: Option<bool>,
-    },
+pub struct Entry {
+    kind: EntryKind,
+    path: Arc<Path>,
+    inode: u64,
+    is_symlink: bool,
+    is_ignored: Option<bool>,
+}
+
+#[derive(Clone, Debug)]
+pub enum EntryKind {
+    PendingDir,
+    Dir,
+    File(CharBag),
 }
 
 impl Entry {
     pub fn path(&self) -> &Arc<Path> {
-        match self {
-            Entry::Dir { path, .. } => path,
-            Entry::File { path, .. } => path,
-        }
+        &self.path
     }
 
     pub fn inode(&self) -> u64 {
-        match self {
-            Entry::Dir { inode, .. } => *inode,
-            Entry::File { inode, .. } => *inode,
-        }
+        self.inode
     }
 
     fn is_ignored(&self) -> Option<bool> {
-        match self {
-            Entry::Dir { is_ignored, .. } => *is_ignored,
-            Entry::File { is_ignored, .. } => *is_ignored,
-        }
+        self.is_ignored
     }
 
     fn set_ignored(&mut self, ignored: bool) {
-        match self {
-            Entry::Dir { is_ignored, .. } => *is_ignored = Some(ignored),
-            Entry::File { is_ignored, .. } => *is_ignored = Some(ignored),
-        }
+        self.is_ignored = Some(ignored);
     }
 
     fn is_dir(&self) -> bool {
-        matches!(self, Entry::Dir { .. })
+        matches!(self.kind, EntryKind::Dir | EntryKind::PendingDir)
     }
 }
 
@@ -441,9 +426,9 @@ impl sum_tree::Item for Entry {
     fn summary(&self) -> Self::Summary {
         let file_count;
         let visible_file_count;
-        if let Entry::File { is_ignored, .. } = self {
+        if matches!(self.kind, EntryKind::File(_)) {
             file_count = 1;
-            if is_ignored.unwrap_or(false) {
+            if self.is_ignored.unwrap_or(false) {
                 visible_file_count = 0;
             } else {
                 visible_file_count = 1;
@@ -653,11 +638,11 @@ impl BackgroundScanner {
         let is_symlink = fs::symlink_metadata(&abs_path)?.file_type().is_symlink();
 
         if metadata.file_type().is_dir() {
-            let dir_entry = Entry::Dir {
+            let dir_entry = Entry {
+                kind: EntryKind::PendingDir,
                 path: path.clone(),
                 inode,
                 is_symlink,
-                pending: true,
                 is_ignored: None,
             };
             self.snapshot.lock().insert_entry(dir_entry);
@@ -684,8 +669,8 @@ impl BackgroundScanner {
                 }
             });
         } else {
-            self.snapshot.lock().insert_entry(Entry::File {
-                path_entry: PathEntry::new(inode, self.root_char_bag, path.clone()),
+            self.snapshot.lock().insert_entry(Entry {
+                kind: EntryKind::File(self.char_bag(&path)),
                 path,
                 inode,
                 is_symlink,
@@ -717,11 +702,11 @@ impl BackgroundScanner {
             }
 
             if child_metadata.is_dir() {
-                new_entries.push(Entry::Dir {
+                new_entries.push(Entry {
+                    kind: EntryKind::PendingDir,
                     path: child_path.clone(),
                     inode: child_inode,
                     is_symlink: child_is_symlink,
-                    pending: true,
                     is_ignored: None,
                 });
                 new_jobs.push(ScanJob {
@@ -730,8 +715,8 @@ impl BackgroundScanner {
                     scan_queue: job.scan_queue.clone(),
                 });
             } else {
-                new_entries.push(Entry::File {
-                    path_entry: PathEntry::new(child_inode, self.root_char_bag, child_path.clone()),
+                new_entries.push(Entry {
+                    kind: EntryKind::File(self.char_bag(&child_path)),
                     path: child_path,
                     inode: child_inode,
                     is_symlink: child_is_symlink,
@@ -960,26 +945,26 @@ impl BackgroundScanner {
             .file_type()
             .is_symlink();
 
-        let entry = if metadata.file_type().is_dir() {
-            Entry::Dir {
-                path,
-                inode,
-                is_symlink,
-                pending: true,
-                is_ignored: None,
-            }
-        } else {
-            Entry::File {
-                path_entry: PathEntry::new(inode, self.root_char_bag, path.clone()),
-                path,
-                inode,
-                is_symlink,
-                is_ignored: None,
-            }
+        let entry = Entry {
+            kind: if metadata.file_type().is_dir() {
+                EntryKind::PendingDir
+            } else {
+                EntryKind::File(self.char_bag(&path))
+            },
+            path,
+            inode,
+            is_symlink,
+            is_ignored: None,
         };
 
         Ok(Some(entry))
     }
+
+    fn char_bag(&self, path: &Path) -> CharBag {
+        let mut result = self.root_char_bag;
+        result.extend(path.to_string_lossy().chars());
+        result
+    }
 }
 
 struct ScanJob {
@@ -1496,13 +1481,10 @@ mod tests {
             let mut files = self.files(0);
             let mut visible_files = self.visible_files(0);
             for entry in self.entries.cursor::<(), ()>() {
-                if let Entry::File {
-                    inode, is_ignored, ..
-                } = entry
-                {
-                    assert_eq!(files.next().unwrap().inode(), *inode);
-                    if !is_ignored.unwrap() {
-                        assert_eq!(visible_files.next().unwrap().inode(), *inode);
+                if matches!(entry.kind, EntryKind::File(_)) {
+                    assert_eq!(files.next().unwrap().inode(), entry.inode);
+                    if !entry.is_ignored.unwrap() {
+                        assert_eq!(visible_files.next().unwrap().inode(), entry.inode);
                     }
                 }
             }
@@ -1518,8 +1500,6 @@ mod tests {
         }
 
         fn to_vec(&self) -> Vec<(&Path, u64, bool)> {
-            use std::iter::FromIterator;
-
             let mut paths = Vec::new();
             for entry in self.entries.cursor::<(), ()>() {
                 paths.push((
@@ -1527,12 +1507,6 @@ mod tests {
                     entry.inode(),
                     entry.is_ignored().unwrap(),
                 ));
-                if let Entry::File { path_entry, .. } = entry {
-                    assert_eq!(
-                        String::from_iter(path_entry.path_chars.iter()),
-                        entry.path().to_str().unwrap()
-                    );
-                }
             }
             paths.sort_by(|a, b| a.0.cmp(&b.0));
             paths

zed/src/worktree/fuzzy.rs 🔗

@@ -1,4 +1,4 @@
-use super::{char_bag::CharBag, Entry, Snapshot};
+use super::{char_bag::CharBag, EntryKind, Snapshot};
 use gpui::scoped_pool;
 use std::{
     cmp::{max, min, Ordering, Reverse},
@@ -12,37 +12,15 @@ const ADDITIONAL_DISTANCE_PENALTY: f64 = 0.05;
 const MIN_DISTANCE_PENALTY: f64 = 0.2;
 
 #[derive(Clone, Debug)]
-pub struct PathEntry {
-    pub ino: u64,
+pub struct MatchCandidate<'a> {
+    pub path: &'a Arc<Path>,
     pub char_bag: CharBag,
-    pub path_chars: Arc<[char]>,
-    pub path: Arc<Path>,
-    pub lowercase_path: Arc<[char]>,
-}
-
-impl PathEntry {
-    pub fn new(ino: u64, root_char_bag: CharBag, path: Arc<Path>) -> Self {
-        let path_str = path.to_string_lossy();
-        let lowercase_path = path_str.to_lowercase().chars().collect::<Vec<_>>().into();
-        let path_chars: Arc<[char]> = path_str.chars().collect::<Vec<_>>().into();
-        let mut char_bag = root_char_bag;
-        char_bag.extend(path_chars.iter().copied());
-
-        Self {
-            ino,
-            char_bag,
-            path_chars,
-            path,
-            lowercase_path,
-        }
-    }
 }
 
 #[derive(Clone, Debug)]
 pub struct PathMatch {
     pub score: f64,
     pub positions: Vec<usize>,
-    pub path_string: String,
     pub tree_id: usize,
     pub path: Arc<Path>,
 }
@@ -77,7 +55,7 @@ pub fn match_paths<'a, T>(
     pool: scoped_pool::Pool,
 ) -> Vec<PathMatch>
 where
-    T: Clone + Send + Iterator<Item = &'a Snapshot>,
+    T: Clone + Send + Iterator<Item = &'a Snapshot> + 'a,
 {
     let lowercase_query = query.to_lowercase().chars().collect::<Vec<_>>();
     let query = query.chars().collect::<Vec<_>>();
@@ -126,12 +104,12 @@ where
                         } else {
                             snapshot.visible_files(start).take(end - start)
                         };
-                        let path_entries = entries.map(|entry| {
-                            if let Entry::File {
-                                path_entry: path, ..
-                            } = entry
-                            {
-                                path
+                        let paths = entries.map(|entry| {
+                            if let EntryKind::File(char_bag) = entry.kind {
+                                MatchCandidate {
+                                    path: &entry.path,
+                                    char_bag,
+                                }
                             } else {
                                 unreachable!()
                             }
@@ -140,7 +118,7 @@ where
                         match_single_tree_paths(
                             snapshot,
                             include_root_name,
-                            path_entries,
+                            paths,
                             query,
                             lowercase_query,
                             query_chars,
@@ -176,7 +154,7 @@ where
 fn match_single_tree_paths<'a>(
     snapshot: &Snapshot,
     include_root_name: bool,
-    path_entries: impl Iterator<Item = &'a PathEntry>,
+    path_entries: impl Iterator<Item = MatchCandidate<'a>>,
     query: &[char],
     lowercase_query: &[char],
     query_chars: CharBag,
@@ -189,6 +167,9 @@ fn match_single_tree_paths<'a>(
     score_matrix: &mut Vec<Option<f64>>,
     best_position_matrix: &mut Vec<usize>,
 ) {
+    let mut path_chars = Vec::new();
+    let mut lowercase_path_chars = Vec::new();
+
     let prefix = if include_root_name {
         snapshot.root_name_chars.as_slice()
     } else {
@@ -200,16 +181,23 @@ fn match_single_tree_paths<'a>(
             continue;
         }
 
+        path_chars.clear();
+        lowercase_path_chars.clear();
+        for c in path_entry.path.to_string_lossy().chars() {
+            path_chars.push(c);
+            lowercase_path_chars.push(c.to_ascii_lowercase());
+        }
+
         if !find_last_positions(
             last_positions,
             prefix,
-            &path_entry.lowercase_path,
+            &lowercase_path_chars,
             &lowercase_query[..],
         ) {
             continue;
         }
 
-        let matrix_len = query.len() * (path_entry.path_chars.len() + prefix.len());
+        let matrix_len = query.len() * (path_chars.len() + prefix.len());
         score_matrix.clear();
         score_matrix.resize(matrix_len, None);
         best_position_matrix.clear();
@@ -218,9 +206,9 @@ fn match_single_tree_paths<'a>(
         let score = score_match(
             &query[..],
             &lowercase_query[..],
-            &path_entry.path_chars,
-            &path_entry.lowercase_path,
-            prefix,
+            &path_chars,
+            &lowercase_path_chars,
+            &prefix,
             smart_case,
             &last_positions,
             score_matrix,
@@ -232,7 +220,6 @@ fn match_single_tree_paths<'a>(
         if score > 0.0 {
             results.push(Reverse(PathMatch {
                 tree_id: snapshot.id,
-                path_string: path_entry.path_chars.iter().collect(),
                 path: path_entry.path.clone(),
                 score,
                 positions: match_positions.clone(),
@@ -533,18 +520,17 @@ mod tests {
         let query = query.chars().collect::<Vec<_>>();
         let query_chars = CharBag::from(&lowercase_query[..]);
 
+        let path_arcs = paths
+            .iter()
+            .map(|path| Arc::from(PathBuf::from(path)))
+            .collect::<Vec<_>>();
         let mut path_entries = Vec::new();
         for (i, path) in paths.iter().enumerate() {
-            let lowercase_path: Arc<[char]> =
-                path.to_lowercase().chars().collect::<Vec<_>>().into();
-            let char_bag = CharBag::from(lowercase_path.as_ref());
-            let path_chars = path.chars().collect();
-            path_entries.push(PathEntry {
-                ino: i as u64,
+            let lowercase_path = path.to_lowercase().chars().collect::<Vec<_>>();
+            let char_bag = CharBag::from(lowercase_path.as_slice());
+            path_entries.push(MatchCandidate {
                 char_bag,
-                path_chars,
-                path: Arc::from(PathBuf::from(path)),
-                lowercase_path,
+                path: path_arcs.get(i).unwrap(),
             });
         }
 
@@ -564,7 +550,7 @@ mod tests {
                 root_name_chars: Vec::new(),
             },
             false,
-            path_entries.iter(),
+            path_entries.into_iter(),
             &query[..],
             &lowercase_query[..],
             query_chars,