Patchwork D10556: rust: Read dirstate from disk in DirstateMap constructor

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

Comments

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

REVISION SUMMARY
  Before this changeset, Python code first creates an empty `DirstateMap` Rust
  object, then immediately calls its `read` method with a byte string of the
  contents of the `.hg/dirstate` file.
  
  This makes that byte string available to the constructor of `DirstateMap`
  in the hg-cpython crate. This is a first step towards enabling parts of
  `DirstateMap` in the hg-core crate to borrow from this buffer without copying.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstate.py
  rust/hg-cpython/src/dirstate/dirstate_map.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -32,8 +32,9 @@ 
     revlog::Node,
     utils::files::normalize_case,
     utils::hg_path::{HgPath, HgPathBuf},
-    DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
-    DirstateMapError, DirstateParents, EntryState, StateMapIter,
+    DirsMultiset, DirstateEntry, DirstateError,
+    DirstateMap as RustDirstateMap, DirstateMapError, DirstateParents,
+    EntryState, StateMapIter,
 };
 
 // TODO
@@ -51,13 +52,25 @@ 
 py_class!(pub class DirstateMap |py| {
     @shared data inner: Box<dyn DirstateMapMethods + Send>;
 
-    def __new__(_cls, use_dirstate_tree: bool) -> PyResult<Self> {
-        let inner = if use_dirstate_tree {
-            Box::new(hg::dirstate_tree::dirstate_map::DirstateMap::new()) as _
+    /// Returns a `(dirstate_map, parents)` tuple
+    @staticmethod
+    def new(use_dirstate_tree: bool, on_disk: PyBytes) -> PyResult<PyObject> {
+        let dirstate_error = |_: DirstateError| {
+            PyErr::new::<exc::OSError, _>(py, "Dirstate error".to_string())
+        };
+        let bytes = on_disk.data(py);
+        let (inner, parents) = if use_dirstate_tree {
+            let mut map = hg::dirstate_tree::dirstate_map::DirstateMap::new();
+            let parents = map.read(bytes).map_err(dirstate_error)?;
+            (Box::new(map) as _, parents)
         } else {
-            Box::new(RustDirstateMap::default()) as _
+            let mut map = RustDirstateMap::default();
+            let parents = map.read(bytes).map_err(dirstate_error)?;
+            (Box::new(map) as _, parents)
         };
-        Self::create_instance(py, inner)
+        let map = Self::create_instance(py, inner)?;
+        let parents = parents.map(|p| dirstate_parents_to_pytuple(py, &p));
+        Ok((map, parents).to_py_object(py).into_object())
     }
 
     def clear(&self) -> PyResult<PyObject> {
@@ -271,21 +284,6 @@ 
             .to_py_object(py))
     }
 
-    def read(&self, st: PyObject) -> PyResult<Option<PyObject>> {
-        match self.inner(py).borrow_mut()
-            .read(st.extract::<PyBytes>(py)?.data(py))
-        {
-            Ok(Some(parents)) => Ok(Some(
-                dirstate_parents_to_pytuple(py, parents)
-                    .into_object()
-            )),
-            Ok(None) => Ok(Some(py.None())),
-            Err(_) => Err(PyErr::new::<exc::OSError, _>(
-                py,
-                "Dirstate error".to_string(),
-            )),
-        }
-    }
     def write(
         &self,
         p1: PyObject,
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -1775,20 +1775,6 @@ 
         def get(self, *args, **kwargs):
             return self._rustmap.get(*args, **kwargs)
 
-        @propertycache
-        def _rustmap(self):
-            """
-            Fills the Dirstatemap when called.
-            """
-            use_dirstate_tree = self._ui.configbool(
-                b"experimental",
-                b"dirstate-tree.in-memory",
-                False,
-            )
-            self._rustmap = rustmod.DirstateMap(use_dirstate_tree)
-            self.read()
-            return self._rustmap
-
         @property
         def copymap(self):
             return self._rustmap.copymap()
@@ -1872,7 +1858,11 @@ 
 
             return self._parents
 
-        def read(self):
+        @propertycache
+        def _rustmap(self):
+            """
+            Fills the Dirstatemap when called.
+            """
             # ignore HG_PENDING because identity is used only for writing
             self.identity = util.filestat.frompath(
                 self._opener.join(self._filename)
@@ -1887,18 +1877,22 @@ 
             except IOError as err:
                 if err.errno != errno.ENOENT:
                     raise
-                return
-            if not st:
-                return
+                st = b''
 
-            parse_dirstate = util.nogc(self._rustmap.read)
-            parents = parse_dirstate(st)
+            use_dirstate_tree = self._ui.configbool(
+                b"experimental",
+                b"dirstate-tree.in-memory",
+                False,
+            )
+            self._rustmap, parents = rustmod.DirstateMap.new(use_dirstate_tree, st)
+
             if parents and not self._dirtyparents:
                 self.setparents(*parents)
 
             self.__contains__ = self._rustmap.__contains__
             self.__getitem__ = self._rustmap.__getitem__
             self.get = self._rustmap.get
+            return self._rustmap
 
         def write(self, st, now):
             parents = self.parents()