Store VFS path file_name and parent indices instead of separate strings (#655)

Author: jplatte

Cargo.lock

@@ -6,6 +6,7 @@ rust-version.workspace = true
 
 [dependencies]
 diesel.workspace = true
+stable_deref_trait = "1.2.1"
 triomphe.workspace = true
 
 [lints]

crates/astr/Cargo.toml

@@ -0,0 +1,19 @@
+use std::ops::Deref;
+
+use super::AStr;
+
+pub enum CowAStr<'a> {
+    Borrowed(&'a AStr),
+    Owned(AStr),
+}
+
+impl Deref for CowAStr<'_> {
+    type Target = AStr;
+
+    fn deref(&self) -> &Self::Target {
+        match self {
+            CowAStr::Borrowed(astr) => astr,
+            CowAStr::Owned(astr) => astr,
+        }
+    }
+}

crates/astr/src/cow.rs

@@ -6,10 +6,14 @@ use std::{
     path::Path,
 };
 
+use stable_deref_trait::StableDeref;
 use triomphe::{Arc, HeaderWithLength};
 
+mod cow;
 mod diesel;
 
+pub use self::cow::CowAStr;
+
 /// String 'atom'.
 ///
 /// Cloning doesn't allocate. As of the time of writing, uses reference
@@ -41,6 +45,8 @@ impl Deref for AStr {
     }
 }
 
+unsafe impl StableDeref for AStr {}
+
 impl Borrow<str> for AStr {
     #[inline]
     fn borrow(&self) -> &str {

crates/astr/src/lib.rs

@@ -7,6 +7,7 @@ edition.workspace = true
 
 [dependencies]
 astr.workspace = true
+derive_more.workspace = true
 indextree.workspace = true
 snafu.workspace = true
 

crates/vfs/Cargo.toml

@@ -1,4 +1,7 @@
+use std::ops::Deref;
+
 use astr::AStr;
+use derive_more::Debug;
 
 pub fn join(a: &str, b: impl AsRef<str> + Into<AStr>) -> AStr {
     let b_ = b.as_ref();
@@ -11,15 +14,53 @@ pub fn join(a: &str, b: impl AsRef<str> + Into<AStr>) -> AStr {
     }
 }
 
-pub fn file_name(path: &str) -> Option<&str> {
-    path.trim_end_matches('/').rsplit('/').next()
+#[derive(Clone, Debug)]
+#[debug("{path:?}")]
+pub struct VfsPath {
+    path: AStr,
+    file_name_start_idx: u32,
+    parent_end_idx: u32,
 }
 
-pub fn parent(path: &str) -> Option<&str> {
-    path.trim_end_matches('/').rsplit_once('/').map(|(parent, _)| {
-        // We had to have split on a direct descendent of `/`
-        if parent.is_empty() { "/" } else { parent }
-    })
+impl VfsPath {
+    pub fn new(path: AStr) -> Self {
+        assert!(path.starts_with('/'));
+        if path.len() > 1 {
+            assert!(!path.ends_with('/'));
+        }
+
+        let file_name_start_idx = (path.rfind('/').unwrap() + 1).try_into().unwrap();
+        let parent_end_idx = if file_name_start_idx == 1 {
+            1
+        } else {
+            file_name_start_idx - 1
+        };
+        Self {
+            path,
+            file_name_start_idx,
+            parent_end_idx,
+        }
+    }
+
+    pub fn astr(&self) -> AStr {
+        self.path.clone()
+    }
+
+    pub fn file_name(&self) -> &str {
+        &self.path[self.file_name_start_idx as usize..]
+    }
+
+    pub fn parent(&self) -> Option<&str> {
+        (self.path.len() > 1).then(|| &self.path[..self.parent_end_idx as usize])
+    }
+}
+
+impl Deref for VfsPath {
+    type Target = str;
+
+    fn deref(&self) -> &Self::Target {
+        &self.path
+    }
 }
 
 pub fn components(path: &str) -> impl Iterator<Item = &str> {
@@ -29,3 +70,44 @@ pub fn components(path: &str) -> impl Iterator<Item = &str> {
         .chain(path.split('/'))
         .filter(|s| !s.is_empty())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::VfsPath;
+
+    #[test]
+    fn vfs_path_root_behavior() {
+        let p = VfsPath::new("/".into());
+        assert_eq!(p.parent(), None);
+        assert_eq!(p.file_name(), "");
+    }
+
+    #[test]
+    fn vfs_path_non_root_behavior() {
+        let p = VfsPath::new("/a".into());
+        assert_eq!(p.parent(), Some("/"));
+        assert_eq!(p.file_name(), "a");
+
+        let p = VfsPath::new("/abc/def/xyz".into());
+        assert_eq!(p.parent(), Some("/abc/def"));
+        assert_eq!(p.file_name(), "xyz");
+    }
+
+    #[test]
+    #[should_panic]
+    fn vfs_path_trailing_slash_invalid() {
+        VfsPath::new("/xyz/".into());
+    }
+
+    #[test]
+    #[should_panic]
+    fn vfs_path_no_leading_slash_invalid() {
+        VfsPath::new("xyz/abc".into());
+    }
+
+    #[test]
+    #[should_panic]
+    fn vfs_path_no_leading_slash_invalid_2() {
+        VfsPath::new("abcxyz".into());
+    }
+}

crates/vfs/src/path.rs

@@ -5,7 +5,7 @@
 //! Build a vfs tree incrementally
 use std::collections::BTreeMap;
 
-use astr::AStr;
+use astr::{AStr, CowAStr};
 
 use crate::path;
 use crate::tree::{Kind, Tree};
@@ -51,7 +51,7 @@ impl<T: BlitFile> TreeBuilder<T> {
         let file = File::new(item);
 
         // Find all parent paths
-        if let Some(parent) = &file.parent {
+        if let Some(parent) = file.parent() {
             let mut leading_path: Option<AStr> = None;
             // Build a set of parent paths skipping `/`, yielding `usr`, `usr/bin`, etc.
             for component in path::components(parent) {
@@ -64,6 +64,7 @@ impl<T: BlitFile> TreeBuilder<T> {
                     .insert(full_path.clone(), File::new(full_path.into()));
             }
         }
+
         self.explicit.push(file);
     }
 
@@ -73,7 +74,7 @@ impl<T: BlitFile> TreeBuilder<T> {
 
         // Walk again to remove accidental dupes
         for i in self.explicit.iter() {
-            self.implicit_dirs.remove(&i.path);
+            self.implicit_dirs.remove(&*i.path);
         }
     }
 
@@ -85,7 +86,7 @@ impl<T: BlitFile> TreeBuilder<T> {
             .iter()
             .filter(|f| f.kind.is_directory())
             .chain(self.implicit_dirs.values())
-            .map(|d| (&d.path, d))
+            .map(|d| (&*d.path, d))
             .collect::<BTreeMap<_, _>>();
 
         // build a set of redirects
@@ -96,14 +97,14 @@ impl<T: BlitFile> TreeBuilder<T> {
             if let Kind::Symlink(target) = &link.kind {
                 // Resolve the link.
                 let target = if target.starts_with('/') {
-                    target.clone()
-                } else if let Some(parent) = &link.parent {
-                    path::join(parent, target)
+                    CowAStr::Borrowed(target)
+                } else if let Some(parent) = link.parent() {
+                    CowAStr::Owned(path::join(parent, target))
                 } else {
-                    target.clone()
+                    CowAStr::Borrowed(target)
                 };
-                if all_dirs.contains_key(&target) {
-                    redirects.insert(&link.path, target);
+                if all_dirs.contains_key(&**target) {
+                    redirects.insert(&*link.path, target);
                 }
             }
         }
@@ -123,7 +124,7 @@ impl<T: BlitFile> TreeBuilder<T> {
             // New node for this guy
             let node = tree.new_node(entry.clone());
 
-            if let Some(parent) = &entry.parent {
+            if let Some(parent) = entry.parent() {
                 tree.add_child_to_node(node, parent)?;
             }
         }

crates/vfs/src/tree/builder.rs

@@ -12,7 +12,7 @@ use astr::AStr;
 use indextree::{Arena, Descendants, NodeId};
 use snafu::Snafu;
 
-use crate::path;
+use crate::path::{self, VfsPath};
 
 pub mod builder;
 
@@ -51,28 +51,32 @@ pub trait BlitFile: Clone + Sized + Debug + From<AStr> {
 #[derive(Debug, Clone)]
 struct File<T> {
     id: AStr,
-    path: AStr,
-    file_name: Option<AStr>,
-    parent: Option<AStr>,
+    path: VfsPath,
     kind: Kind,
     inner: T,
 }
 
 impl<T: BlitFile> File<T> {
     pub fn new(inner: T) -> Self {
-        let path = inner.path();
-        let file_name = path::file_name(&path).map(AStr::from);
-        let parent = path::parent(&path).map(AStr::from);
+        let path = VfsPath::new(inner.path());
 
         Self {
             id: inner.id(),
             path,
-            file_name,
-            parent,
             kind: inner.kind(),
             inner,
         }
     }
+
+    #[inline]
+    fn file_name(&self) -> &str {
+        self.path.file_name()
+    }
+
+    #[inline]
+    fn parent(&self) -> Option<&str> {
+        self.path.parent()
+    }
 }
 
 /// Actual tree implementation, encapsulating indextree
@@ -105,7 +109,7 @@ impl<T: BlitFile> Tree<T> {
 
     /// Generate a new node, store the path mapping for it
     fn new_node(&mut self, data: File<T>) -> NodeId {
-        let path = data.path.clone();
+        let path = data.path.astr();
         let node = self.arena.new_node(data);
         self.map.insert(path, node);
         self.length += 1;
@@ -130,7 +134,7 @@ impl<T: BlitFile> Tree<T> {
             .children(&self.arena)
             .filter_map(|n| {
                 let n = self.arena.get(n)?.get();
-                if n.file_name == node.get().file_name {
+                if n.file_name() == node.get().file_name() {
                     Some(n)
                 } else {
                     None
@@ -139,7 +143,7 @@ impl<T: BlitFile> Tree<T> {
             .collect::<Vec<_>>();
         if !others.is_empty() {
             let e = Error::Duplicate {
-                node_path: node.get().path.clone(),
+                node_path: node.get().path.astr(),
                 node_id: node.get().id.clone(),
                 other_id: others.first().unwrap().id.clone(),
             };
@@ -193,7 +197,7 @@ impl<T: BlitFile> Tree<T> {
                 Some(n) => *n,
                 None => self.new_node(orphan.clone()),
             };
-            if let Some(parent) = &orphan.parent {
+            if let Some(parent) = orphan.parent() {
                 self.add_child_to_node(node, parent)?;
             }
         }
@@ -218,7 +222,7 @@ impl<T: BlitFile> Tree<T> {
     fn structured_children(&self, start: &NodeId) -> Element<'_, T> {
         let node = &self.arena[*start];
         let item = node.get();
-        let partial = item.file_name.as_deref().unwrap_or_default();
+        let partial = item.file_name();
 
         if item.kind.is_directory() {
             let children = start