lib: small memory-optimization for RepoPath

Signed-off-by: Austin Seipp <[email protected]>
Change-Id: I2007f35f645b890ab94467614c6094c0

cli/src/merge_tools/builtin.rs

@@ -23,7 +23,7 @@ pub enum BuiltinToolError {
     ReadFileBackend(BackendError),
     #[error("Failed to read file {path:?} with ID {id:?}: {source}")]
     ReadFileIo {
-        path: RepoPath,
+        path: Box<RepoPath>,
         id: FileId,
         source: std::io::Error,
     },
@@ -119,7 +119,7 @@ fn read_file_contents(
             reader
                 .read_to_end(&mut buf)
                 .map_err(|err| BuiltinToolError::ReadFileIo {
-                    path: path.clone(),
+                    path: Box::new(path.clone()),
                     id: id.clone(),
                     source: err,
                 })?;

cli/src/merge_tools/mod.rs

@@ -66,16 +66,16 @@ pub enum ConflictResolveError {
     #[error(transparent)]
     ExternalTool(#[from] ExternalToolError),
     #[error("Couldn't find the path {0:?} in this revision")]
-    PathNotFound(RepoPath),
+    PathNotFound(Box<RepoPath>),
     #[error("Couldn't find any conflicts at {0:?} in this revision")]
-    NotAConflict(RepoPath),
+    NotAConflict(Box<RepoPath>),
     #[error(
         "Only conflicts that involve normal files (not symlinks, not executable, etc.) are \
          supported. Conflict summary for {0:?}:\n{1}"
     )]
-    NotNormalFiles(RepoPath, String),
+    NotNormalFiles(Box<RepoPath>, String),
     #[error("The conflict at {path:?} has {sides} sides. At most 2 sides are supported.")]
-    ConflictTooComplicated { path: RepoPath, sides: usize },
+    ConflictTooComplicated { path: Box<RepoPath>, sides: usize },
     #[error(
         "The output file is either unchanged or empty after the editor quit (run with --verbose \
          to see the exact invocation)."
@@ -93,23 +93,31 @@ pub fn run_mergetool(
 ) -> Result<MergedTreeId, ConflictResolveError> {
     let conflict = match tree.path_value(repo_path).into_resolved() {
         Err(conflict) => conflict,
-        Ok(Some(_)) => return Err(ConflictResolveError::NotAConflict(repo_path.clone())),
-        Ok(None) => return Err(ConflictResolveError::PathNotFound(repo_path.clone())),
+        Ok(Some(_)) => {
+            return Err(ConflictResolveError::NotAConflict(Box::new(
+                repo_path.clone(),
+            )))
+        }
+        Ok(None) => {
+            return Err(ConflictResolveError::PathNotFound(Box::new(
+                repo_path.clone(),
+            )))
+        }
     };
     let file_merge = conflict.to_file_merge().ok_or_else(|| {
         let mut summary_bytes: Vec<u8> = vec![];
         conflict
             .describe(&mut summary_bytes)
             .expect("Writing to an in-memory buffer should never fail");
         ConflictResolveError::NotNormalFiles(
-            repo_path.clone(),
+            Box::new(repo_path.clone()),
             String::from_utf8_lossy(summary_bytes.as_slice()).to_string(),
         )
     })?;
     // We only support conflicts with 2 sides (3-way conflicts)
     if file_merge.num_sides() > 2 {
         return Err(ConflictResolveError::ConflictTooComplicated {
-            path: repo_path.clone(),
+            path: Box::new(repo_path.clone()),
             sides: file_merge.num_sides(),
         });
     };

lib/src/merged_tree.rs

@@ -828,7 +828,7 @@ enum TreeDiffItem {
     // This is used for making sure that when a directory gets replaced by a file, we
     // yield the value for the addition of the file after we yield the values
     // for removing files in the directory.
-    File(RepoPath, MergedTreeValue, MergedTreeValue),
+    File(Box<RepoPath>, MergedTreeValue, MergedTreeValue),
 }
 
 impl<'matcher> TreeDiffIterator<'matcher> {
@@ -909,7 +909,7 @@ impl Iterator for TreeDiffIterator<'_> {
                 }
                 TreeDiffItem::File(..) => {
                     if let TreeDiffItem::File(name, before, after) = self.stack.pop().unwrap() {
-                        return Some((name, Ok((before, after))));
+                        return Some((*name, Ok((before, after))));
                     } else {
                         unreachable!();
                     }
@@ -954,7 +954,7 @@ impl Iterator for TreeDiffIterator<'_> {
                     if after.is_present() {
                         self.stack.insert(
                             post_subdir,
-                            TreeDiffItem::File(path, Merge::absent(), after),
+                            TreeDiffItem::File(Box::new(path), Merge::absent(), after),
                         );
                     }
                 } else if !tree_before && !tree_after {

lib/src/repo_path.rs

@@ -19,6 +19,7 @@ use std::path::{Component, Path, PathBuf};
 
 use compact_str::CompactString;
 use itertools::Itertools;
+use smallvec::{smallvec, SmallVec};
 use thiserror::Error;
 
 use crate::file_util;
@@ -55,9 +56,11 @@ impl From<String> for RepoPathComponent {
     }
 }
 
+pub type RepoPathComponents = SmallVec<[RepoPathComponent; 4]>;
+
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct RepoPath {
-    components: Vec<RepoPathComponent>,
+    components: RepoPathComponents,
 }
 
 impl Debug for RepoPath {
@@ -68,7 +71,9 @@ impl Debug for RepoPath {
 
 impl RepoPath {
     pub fn root() -> Self {
-        RepoPath { components: vec![] }
+        RepoPath {
+            components: smallvec![],
+        }
     }
 
     pub fn from_internal_string(value: &str) -> Self {
@@ -86,7 +91,7 @@ impl RepoPath {
         }
     }
 
-    pub fn from_components(components: Vec<RepoPathComponent>) -> Self {
+    pub fn from_components(components: RepoPathComponents) -> Self {
         RepoPath { components }
     }
 
@@ -161,7 +166,7 @@ impl RepoPath {
             None
         } else {
             Some(RepoPath {
-                components: self.components[0..self.components.len() - 1].to_vec(),
+                components: SmallVec::from(&self.components[0..self.components.len() - 1]),
             })
         }
     }
@@ -174,7 +179,7 @@ impl RepoPath {
         }
     }
 
-    pub fn components(&self) -> &Vec<RepoPathComponent> {
+    pub fn components(&self) -> &RepoPathComponents {
         &self.components
     }
 }
@@ -284,17 +289,20 @@ mod tests {
 
     #[test]
     fn test_components() {
-        assert_eq!(RepoPath::root().components(), &vec![]);
+        assert_eq!(
+            RepoPath::root().components(),
+            &smallvec![] as &RepoPathComponents
+        );
         assert_eq!(
             RepoPath::from_internal_string("dir").components(),
-            &vec![RepoPathComponent::from("dir")]
+            &smallvec![RepoPathComponent::from("dir")] as &RepoPathComponents
         );
         assert_eq!(
             RepoPath::from_internal_string("dir/subdir").components(),
-            &vec![
+            &smallvec![
                 RepoPathComponent::from("dir"),
                 RepoPathComponent::from("subdir")
-            ]
+            ] as &RepoPathComponents
         );
     }
 

lib/tests/test_merge_trees.rs

@@ -18,6 +18,7 @@ use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathComponent};
 use jj_lib::rewrite::rebase_commit;
 use jj_lib::tree::{merge_trees, Tree};
+use smallvec::smallvec;
 use testutils::{create_single_tree, create_tree, TestRepo};
 
 #[test]
@@ -504,7 +505,7 @@ fn test_simplify_conflict() {
     match further_rebased_tree.value(&component).unwrap() {
         TreeValue::Conflict(id) => {
             let conflict = store
-                .read_conflict(&RepoPath::from_components(vec![component.clone()]), id)
+                .read_conflict(&RepoPath::from_components(smallvec![component.clone()]), id)
                 .unwrap();
             assert_eq!(
                 conflict.removes(),