Patchwork D10750: dirstate-v2: Make the dirstate bytes buffer available in more places

login
register
mail settings
Submitter phabricator
Date May 19, 2021, 4:38 p.m.
Message ID <differential-rev-PHID-DREV-iekz2w3nchpqj33abf5m-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49069/
State Superseded
Headers show

Comments

phabricator - May 19, 2021, 4:38 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10750

AFFECTED FILES
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/on_disk.rs
  rust/hg-core/src/dirstate_tree/status.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate_tree/status.rs b/rust/hg-core/src/dirstate_tree/status.rs
--- a/rust/hg-core/src/dirstate_tree/status.rs
+++ b/rust/hg-core/src/dirstate_tree/status.rs
@@ -32,8 +32,8 @@ 
 /// exists in one of the two trees, depending on information requested by
 /// `options` we may need to traverse the remaining subtree.
 #[timed]
-pub fn status<'tree>(
-    dmap: &'tree mut DirstateMap,
+pub fn status<'tree, 'on_disk: 'tree>(
+    dmap: &'tree mut DirstateMap<'on_disk>,
     matcher: &(dyn Matcher + Sync),
     root_dir: PathBuf,
     ignore_files: Vec<PathBuf>,
@@ -47,6 +47,7 @@ 
         };
 
     let common = StatusCommon {
+        dmap,
         options,
         matcher,
         ignore_fn,
@@ -67,14 +68,15 @@ 
 
 /// Bag of random things needed by various parts of the algorithm. Reduces the
 /// number of parameters passed to functions.
-struct StatusCommon<'tree, 'a> {
+struct StatusCommon<'tree, 'a, 'on_disk: 'tree> {
+    dmap: &'tree DirstateMap<'on_disk>,
     options: StatusOptions,
     matcher: &'a (dyn Matcher + Sync),
     ignore_fn: IgnoreFnType<'a>,
     outcome: Mutex<DirstateStatus<'tree>>,
 }
 
-impl<'tree, 'a> StatusCommon<'tree, 'a> {
+impl<'tree, 'a> StatusCommon<'tree, 'a, '_> {
     fn read_dir(
         &self,
         hg_path: &HgPath,
@@ -119,7 +121,7 @@ 
         // Propagate here any error that would happen inside the comparison
         // callback below
         for dirstate_node in &dirstate_nodes {
-            dirstate_node.base_name()?;
+            dirstate_node.base_name(self.dmap.on_disk)?;
         }
         itertools::merge_join_by(
             dirstate_nodes,
@@ -127,7 +129,10 @@ 
             |dirstate_node, fs_entry| {
                 // This `unwrap` never panics because we already propagated
                 // those errors above
-                dirstate_node.base_name().unwrap().cmp(&fs_entry.base_name)
+                dirstate_node
+                    .base_name(self.dmap.on_disk)
+                    .unwrap()
+                    .cmp(&fs_entry.base_name)
             },
         )
         .par_bridge()
@@ -159,7 +164,7 @@ 
         dirstate_node: NodeRef<'tree, '_>,
         has_ignored_ancestor: bool,
     ) -> Result<(), DirstateV2ParseError> {
-        let hg_path = dirstate_node.full_path()?;
+        let hg_path = dirstate_node.full_path(self.dmap.on_disk)?;
         let file_type = fs_entry.metadata.file_type();
         let file_or_symlink = file_type.is_file() || file_type.is_symlink();
         if !file_or_symlink {
@@ -179,7 +184,7 @@ 
             let is_at_repo_root = false;
             self.traverse_fs_directory_and_dirstate(
                 is_ignored,
-                dirstate_node.children()?,
+                dirstate_node.children(self.dmap.on_disk)?,
                 hg_path,
                 &fs_entry.full_path,
                 is_at_repo_root,
@@ -221,7 +226,8 @@ 
                 }
             }
 
-            for child_node in dirstate_node.children()?.iter() {
+            for child_node in dirstate_node.children(self.dmap.on_disk)?.iter()
+            {
                 self.traverse_dirstate_only(child_node)?
             }
         }
@@ -246,7 +252,7 @@ 
         let entry = dirstate_node
             .entry()?
             .expect("handle_normal_file called with entry-less node");
-        let full_path = Cow::from(dirstate_node.full_path()?);
+        let full_path = Cow::from(dirstate_node.full_path(self.dmap.on_disk)?);
         let mode_changed = || {
             self.options.check_exec && entry.mode_changed(&fs_entry.metadata)
         };
@@ -282,11 +288,11 @@ 
         dirstate_node: NodeRef<'tree, '_>,
     ) -> Result<(), DirstateV2ParseError> {
         self.mark_removed_or_deleted_if_file(
-            dirstate_node.full_path()?,
+            dirstate_node.full_path(self.dmap.on_disk)?,
             dirstate_node.state()?,
         );
         dirstate_node
-            .children()?
+            .children(self.dmap.on_disk)?
             .iter()
             .map(|child_node| self.traverse_dirstate_only(child_node))
             .collect()
diff --git a/rust/hg-core/src/dirstate_tree/on_disk.rs b/rust/hg-core/src/dirstate_tree/on_disk.rs
--- a/rust/hg-core/src/dirstate_tree/on_disk.rs
+++ b/rust/hg-core/src/dirstate_tree/on_disk.rs
@@ -272,7 +272,8 @@ 
     // actual offset for the root nodes.
     out.resize(header_len, 0_u8);
 
-    let root = write_nodes(dirstate_map.root.as_ref(), &mut out)?;
+    let root =
+        write_nodes(dirstate_map, dirstate_map.root.as_ref(), &mut out)?;
 
     let header = Header {
         marker: *V2_FORMAT_MARKER,
@@ -289,6 +290,7 @@ 
 
 /// Serialize the dirstate to the `v2` format after clearing ambigous `mtime`s.
 fn write_nodes(
+    dirstate_map: &DirstateMap,
     nodes: dirstate_map::ChildNodesRef,
     out: &mut Vec<u8>,
 ) -> Result<ChildNodes, DirstateError> {
@@ -299,16 +301,19 @@ 
     // First accumulate serialized nodes in a `Vec`
     let mut on_disk_nodes = Vec::with_capacity(nodes.len());
     for node in nodes {
-        let children = write_nodes(node.children()?, out)?;
-        let full_path = write_slice::<u8>(node.full_path()?.as_bytes(), out);
-        let copy_source = if let Some(source) = node.copy_source()? {
-            write_slice::<u8>(source.as_bytes(), out)
-        } else {
-            Slice {
-                start: 0.into(),
-                len: 0.into(),
-            }
-        };
+        let children = node.children(dirstate_map.on_disk)?;
+        let children = write_nodes(dirstate_map, children, out)?;
+        let full_path = node.full_path(dirstate_map.on_disk)?;
+        let full_path = write_slice::<u8>(full_path.as_bytes(), out);
+        let copy_source =
+            if let Some(source) = node.copy_source(dirstate_map.on_disk)? {
+                write_slice::<u8>(source.as_bytes(), out)
+            } else {
+                Slice {
+                    start: 0.into(),
+                    len: 0.into(),
+                }
+            };
         on_disk_nodes.push(match node {
             NodeRef::InMemory(path, node) => Node {
                 children,
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -81,6 +81,7 @@ 
 
     pub(super) fn make_mut(
         &mut self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<
         &mut FastHashMap<NodeKey<'on_disk>, Node<'on_disk>>,
         DirstateV2ParseError,
@@ -95,6 +96,7 @@ 
     pub(super) fn get(
         &self,
         base_name: &HgPath,
+        _on_disk: &'on_disk [u8],
     ) -> Result<Option<NodeRef<'tree, 'on_disk>>, DirstateV2ParseError> {
         match self {
             ChildNodesRef::InMemory(nodes) => Ok(nodes
@@ -137,6 +139,7 @@ 
 impl<'tree, 'on_disk> NodeRef<'tree, 'on_disk> {
     pub(super) fn full_path(
         &self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<&'tree HgPath, DirstateV2ParseError> {
         match self {
             NodeRef::InMemory(path, _node) => Ok(path.full_path()),
@@ -146,6 +149,7 @@ 
     /// Returns a `Cow` that can borrow 'on_disk but is detached from 'tree
     pub(super) fn full_path_cow(
         &self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<Cow<'on_disk, HgPath>, DirstateV2ParseError> {
         match self {
             NodeRef::InMemory(path, _node) => Ok(path.full_path().clone()),
@@ -154,6 +158,7 @@ 
 
     pub(super) fn base_name(
         &self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<&'tree HgPath, DirstateV2ParseError> {
         match self {
             NodeRef::InMemory(path, _node) => Ok(path.base_name()),
@@ -162,6 +167,7 @@ 
 
     pub(super) fn children(
         &self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<ChildNodesRef<'tree, 'on_disk>, DirstateV2ParseError> {
         match self {
             NodeRef::InMemory(_path, node) => Ok(node.children.as_ref()),
@@ -176,6 +182,7 @@ 
 
     pub(super) fn copy_source(
         &self,
+        _on_disk: &'on_disk [u8],
     ) -> Result<Option<&'tree HgPath>, DirstateV2ParseError> {
         match self {
             NodeRef::InMemory(_path, node) => {
@@ -260,6 +267,7 @@ 
             |path, entry, copy_source| {
                 let tracked = entry.state.is_tracked();
                 let node = Self::get_or_insert_node(
+                    map.on_disk,
                     &mut map.root,
                     path,
                     WithBasename::to_cow_borrowed,
@@ -300,10 +308,10 @@ 
         let mut component =
             components.next().expect("expected at least one components");
         loop {
-            if let Some(child) = children.get(component)? {
+            if let Some(child) = children.get(component, self.on_disk)? {
                 if let Some(next_component) = components.next() {
                     component = next_component;
-                    children = child.children()?;
+                    children = child.children(self.on_disk)?;
                 } else {
                     return Ok(Some(child));
                 }
@@ -318,6 +326,7 @@ 
     /// This takes `root` instead of `&mut self` so that callers can mutate
     /// other fields while the returned borrow is still valid
     fn get_node_mut<'tree>(
+        on_disk: &'on_disk [u8],
         root: &'tree mut ChildNodes<'on_disk>,
         path: &HgPath,
     ) -> Result<Option<&'tree mut Node<'on_disk>>, DirstateV2ParseError> {
@@ -326,7 +335,8 @@ 
         let mut component =
             components.next().expect("expected at least one components");
         loop {
-            if let Some(child) = children.make_mut()?.get_mut(component) {
+            if let Some(child) = children.make_mut(on_disk)?.get_mut(component)
+            {
                 if let Some(next_component) = components.next() {
                     component = next_component;
                     children = &mut child.children;
@@ -340,6 +350,7 @@ 
     }
 
     fn get_or_insert_node<'tree, 'path>(
+        on_disk: &'on_disk [u8],
         root: &'tree mut ChildNodes<'on_disk>,
         path: &'path HgPath,
         to_cow: impl Fn(
@@ -358,7 +369,7 @@ 
             // map already contains that key, without introducing double
             // lookup?
             let child_node = child_nodes
-                .make_mut()?
+                .make_mut(on_disk)?
                 .entry(to_cow(ancestor_path))
                 .or_default();
             if let Some(next) = inclusive_ancestor_paths.next() {
@@ -385,6 +396,7 @@ 
             };
 
         let node = Self::get_or_insert_node(
+            self.on_disk,
             &mut self.root,
             path,
             WithBasename::to_cow_owned,
@@ -434,7 +446,7 @@ 
         let mut iter = self.root.as_ref().iter();
         std::iter::from_fn(move || {
             while let Some(child_node) = iter.next() {
-                let children = match child_node.children() {
+                let children = match child_node.children(self.on_disk) {
                     Ok(children) => children,
                     Err(error) => return Some(Err(error)),
                 };
@@ -462,9 +474,11 @@ 
         paths: &[impl AsRef<HgPath>],
     ) -> Result<(), DirstateV2ParseError> {
         for path in paths {
-            if let Some(node) =
-                Self::get_node_mut(&mut self.root, path.as_ref())?
-            {
+            if let Some(node) = Self::get_node_mut(
+                self.on_disk,
+                &mut self.root,
+                path.as_ref(),
+            )? {
                 if let Some(entry) = node.entry.as_mut() {
                     entry.clear_mtime();
                 }
@@ -487,7 +501,7 @@ 
         filter_map_results(self.iter_nodes(), move |node| {
             if let Some(entry) = node.entry()? {
                 if predicate(&entry) {
-                    return Ok(Some(node.full_path()?));
+                    return Ok(Some(node.full_path(self.on_disk)?));
                 }
             }
             Ok(None)
@@ -556,14 +570,15 @@ 
             had_entry: bool,
             had_copy_source: bool,
         }
-        fn recur(
-            nodes: &mut ChildNodes,
+        fn recur<'on_disk>(
+            on_disk: &'on_disk [u8],
+            nodes: &mut ChildNodes<'on_disk>,
             path: &HgPath,
         ) -> Result<Option<Dropped>, DirstateV2ParseError> {
             let (first_path_component, rest_of_path) =
                 path.split_first_component();
             let node = if let Some(node) =
-                nodes.make_mut()?.get_mut(first_path_component)
+                nodes.make_mut(on_disk)?.get_mut(first_path_component)
             {
                 node
             } else {
@@ -571,7 +586,7 @@ 
             };
             let dropped;
             if let Some(rest) = rest_of_path {
-                if let Some(d) = recur(&mut node.children, rest)? {
+                if let Some(d) = recur(on_disk, &mut node.children, rest)? {
                     dropped = d;
                     if dropped.was_tracked {
                         node.tracked_descendants_count -= 1;
@@ -595,12 +610,12 @@ 
                 && node.copy_source.is_none()
                 && node.children.is_empty()
             {
-                nodes.make_mut()?.remove(first_path_component);
+                nodes.make_mut(on_disk)?.remove(first_path_component);
             }
             Ok(Some(dropped))
         }
 
-        if let Some(dropped) = recur(&mut self.root, filename)? {
+        if let Some(dropped) = recur(self.on_disk, &mut self.root, filename)? {
             if dropped.had_entry {
                 self.nodes_with_entry_count -= 1
             }
@@ -620,7 +635,8 @@ 
         now: i32,
     ) -> Result<(), DirstateV2ParseError> {
         for filename in filenames {
-            if let Some(node) = Self::get_node_mut(&mut self.root, &filename)?
+            if let Some(node) =
+                Self::get_node_mut(self.on_disk, &mut self.root, &filename)?
             {
                 if let Some(entry) = node.entry.as_mut() {
                     entry.clear_ambiguous_mtime(now);
@@ -721,10 +737,12 @@ 
         for node in self.iter_nodes() {
             let node = node?;
             if let Some(entry) = node.entry()? {
-                size +=
-                    packed_entry_size(node.full_path()?, node.copy_source()?);
+                size += packed_entry_size(
+                    node.full_path(self.on_disk)?,
+                    node.copy_source(self.on_disk)?,
+                );
                 if entry.mtime_is_ambigous(now) {
-                    ambiguous_mtimes.push(node.full_path_cow()?)
+                    ambiguous_mtimes.push(node.full_path_cow(self.on_disk)?)
                 }
             }
         }
@@ -737,9 +755,9 @@ 
             let node = node?;
             if let Some(entry) = node.entry()? {
                 pack_entry(
-                    node.full_path()?,
+                    node.full_path(self.on_disk)?,
                     &entry,
-                    node.copy_source()?,
+                    node.copy_source(self.on_disk)?,
                     &mut packed,
                 );
             }
@@ -760,7 +778,7 @@ 
             let node = node?;
             if let Some(entry) = node.entry()? {
                 if entry.mtime_is_ambigous(now) {
-                    paths.push(node.full_path_cow()?)
+                    paths.push(node.full_path_cow(self.on_disk)?)
                 }
             }
         }
@@ -799,9 +817,9 @@ 
     }
 
     fn copy_map_iter(&self) -> CopyMapIter<'_> {
-        Box::new(filter_map_results(self.iter_nodes(), |node| {
-            Ok(if let Some(source) = node.copy_source()? {
-                Some((node.full_path()?, source))
+        Box::new(filter_map_results(self.iter_nodes(), move |node| {
+            Ok(if let Some(source) = node.copy_source(self.on_disk)? {
+                Some((node.full_path(self.on_disk)?, source))
             } else {
                 None
             })
@@ -824,7 +842,7 @@ 
         key: &HgPath,
     ) -> Result<Option<&HgPath>, DirstateV2ParseError> {
         if let Some(node) = self.get_node(key)? {
-            if let Some(source) = node.copy_source()? {
+            if let Some(source) = node.copy_source(self.on_disk)? {
                 return Ok(Some(source));
             }
         }
@@ -836,12 +854,16 @@ 
         key: &HgPath,
     ) -> Result<Option<HgPathBuf>, DirstateV2ParseError> {
         let count = &mut self.nodes_with_copy_source_count;
-        Ok(Self::get_node_mut(&mut self.root, key)?.and_then(|node| {
-            if node.copy_source.is_some() {
-                *count -= 1
-            }
-            node.copy_source.take().map(Cow::into_owned)
-        }))
+        Ok(
+            Self::get_node_mut(self.on_disk, &mut self.root, key)?.and_then(
+                |node| {
+                    if node.copy_source.is_some() {
+                        *count -= 1
+                    }
+                    node.copy_source.take().map(Cow::into_owned)
+                },
+            ),
+        )
     }
 
     fn copy_map_insert(
@@ -850,6 +872,7 @@ 
         value: HgPathBuf,
     ) -> Result<Option<HgPathBuf>, DirstateV2ParseError> {
         let node = Self::get_or_insert_node(
+            self.on_disk,
             &mut self.root,
             &key,
             WithBasename::to_cow_owned,
@@ -884,9 +907,9 @@ 
     }
 
     fn iter(&self) -> StateMapIter<'_> {
-        Box::new(filter_map_results(self.iter_nodes(), |node| {
+        Box::new(filter_map_results(self.iter_nodes(), move |node| {
             Ok(if let Some(entry) = node.entry()? {
-                Some((node.full_path()?, entry))
+                Some((node.full_path(self.on_disk)?, entry))
             } else {
                 None
             })