Patchwork D10560: dirstate-tree: Borrow paths from the "on disk" bytes

login
register
mail settings
Submitter phabricator
Date May 3, 2021, 10:29 a.m.
Message ID <differential-rev-PHID-DREV-onbfbj4q2mpo2an3gvl3-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48877/
State Superseded
Headers show

Comments

phabricator - May 3, 2021, 10:29 a.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Use std::borrow::Cow to avoid some memory allocations and copying.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-core/src/dirstate_tree/path_with_basename.rs b/rust/hg-core/src/dirstate_tree/path_with_basename.rs
--- a/rust/hg-core/src/dirstate_tree/path_with_basename.rs
+++ b/rust/hg-core/src/dirstate_tree/path_with_basename.rs
@@ -1,5 +1,5 @@ 
 use crate::utils::hg_path::HgPath;
-use std::borrow::Borrow;
+use std::borrow::{Borrow, Cow};
 
 /// Wraps `HgPath` or `HgPathBuf` to make it behave "as" its last path
 /// component, a.k.a. its base name (as in Python’s `os.path.basename`), but
@@ -81,10 +81,17 @@ 
     }
 }
 
-impl<T: ?Sized + ToOwned> WithBasename<&'_ T> {
-    pub fn to_owned(&self) -> WithBasename<T::Owned> {
+impl<'a> WithBasename<&'a HgPath> {
+    pub fn to_cow_borrowed(self) -> WithBasename<Cow<'a, HgPath>> {
         WithBasename {
-            full_path: self.full_path.to_owned(),
+            full_path: Cow::Borrowed(self.full_path),
+            base_name_start: self.base_name_start,
+        }
+    }
+
+    pub fn to_cow_owned<'b>(self) -> WithBasename<Cow<'b, HgPath>> {
+        WithBasename {
+            full_path: Cow::Owned(self.full_path.to_owned()),
             base_name_start: self.base_name_start,
         }
     }
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
@@ -45,7 +45,7 @@ 
 /// names. However `BTreeMap` would waste time always re-comparing the same
 /// string prefix.
 pub(super) type ChildNodes<'on_disk> =
-    FastHashMap<WithBasename<HgPathBuf>, Node<'on_disk>>;
+    FastHashMap<WithBasename<Cow<'on_disk, HgPath>>, Node<'on_disk>>;
 
 /// Represents a file or a directory
 #[derive(Default)]
@@ -99,9 +99,10 @@ 
             self.on_disk,
             |path, entry, copy_source| {
                 let tracked = entry.state.is_tracked();
-                let node = Self::get_or_insert_node_tracing_ancestors(
+                let node = Self::get_or_insert_node(
                     &mut self.root,
                     path,
+                    WithBasename::to_cow_borrowed,
                     |ancestor| {
                         if tracked {
                             ancestor.tracked_descendants_count += 1
@@ -181,16 +182,12 @@ 
         }
     }
 
-    fn get_or_insert_node<'tree>(
+    fn get_or_insert_node<'tree, 'path>(
         root: &'tree mut ChildNodes<'on_disk>,
-        path: &HgPath,
-    ) -> &'tree mut Node<'on_disk> {
-        Self::get_or_insert_node_tracing_ancestors(root, path, |_| {})
-    }
-
-    fn get_or_insert_node_tracing_ancestors<'tree>(
-        root: &'tree mut ChildNodes<'on_disk>,
-        path: &HgPath,
+        path: &'path HgPath,
+        to_cow: impl Fn(
+            WithBasename<&'path HgPath>,
+        ) -> WithBasename<Cow<'on_disk, HgPath>>,
         mut each_ancestor: impl FnMut(&mut Node),
     ) -> &'tree mut Node<'on_disk> {
         let mut child_nodes = root;
@@ -204,7 +201,7 @@ 
             // map already contains that key, without introducing double
             // lookup?
             let child_node =
-                child_nodes.entry(ancestor_path.to_owned()).or_default();
+                child_nodes.entry(to_cow(ancestor_path)).or_default();
             if let Some(next) = inclusive_ancestor_paths.next() {
                 each_ancestor(child_node);
                 ancestor_path = next;
@@ -228,9 +225,10 @@ 
                 _ => 0,
             };
 
-        let node = Self::get_or_insert_node_tracing_ancestors(
+        let node = Self::get_or_insert_node(
             &mut self.root,
             path,
+            WithBasename::to_cow_owned,
             |ancestor| {
                 // We can’t use `+= increment` because the counter is unsigned,
                 // and we want debug builds to detect accidental underflow
@@ -593,7 +591,12 @@ 
         key: HgPathBuf,
         value: HgPathBuf,
     ) -> Option<HgPathBuf> {
-        let node = Self::get_or_insert_node(&mut self.root, &key);
+        let node = Self::get_or_insert_node(
+            &mut self.root,
+            &key,
+            WithBasename::to_cow_owned,
+            |_ancestor| {},
+        );
         if node.copy_source.is_none() {
             self.nodes_with_copy_source_count += 1
         }