Patchwork D11487: dirstate: Skip no-op conversion in Rust DirstateMap::set_v1

login
register
mail settings
Submitter phabricator
Date Sept. 22, 2021, 8:56 p.m.
Message ID <differential-rev-PHID-DREV-6qbrp2vp6v7qxql7blb6-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49805/
State Superseded
Headers show

Comments

phabricator - Sept. 22, 2021, 8:56 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Now that the `DirstateItem` python class is implemented in Rust containing
  a `DirstateEntry` value, use that value directly instead of reconstructing
  it from v1 data.
  
  Also rename from `set_v1` since dirstate-v1 data is not used anymore.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstatemap.py
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-core/src/dirstate_tree/owning_dispatch.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/item.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/item.rs b/rust/hg-cpython/src/dirstate/item.rs
--- a/rust/hg-cpython/src/dirstate/item.rs
+++ b/rust/hg-cpython/src/dirstate/item.rs
@@ -180,6 +180,10 @@ 
         Ok(DirstateItem::create_instance(py, Cell::new(entry))?.into_object())
     }
 
+    pub fn get_entry(&self, py: Python<'_>) -> DirstateEntry {
+        self.entry(py).get()
+    }
+
     // TODO: Use https://doc.rust-lang.org/std/cell/struct.Cell.html#method.update instead when it’s stable
     pub fn update(&self, py: Python<'_>, f: impl FnOnce(&mut DirstateEntry)) {
         let mut entry = self.entry(py).get();
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
@@ -129,18 +129,14 @@ 
         }
     }
 
-    def set_v1(&self, path: PyObject, item: PyObject) -> PyResult<PyObject> {
+    def set_dirstate_item(
+        &self,
+        path: PyObject,
+        item: DirstateItem
+    ) -> PyResult<PyObject> {
         let f = path.extract::<PyBytes>(py)?;
         let filename = HgPath::new(f.data(py));
-        let state = item.getattr(py, "state")?.extract::<PyBytes>(py)?;
-        let state = state.data(py)[0];
-        let entry = DirstateEntry::from_v1_data(
-            state.try_into().expect("state is always valid"),
-            item.getattr(py, "mode")?.extract(py)?,
-            item.getattr(py, "size")?.extract(py)?,
-            item.getattr(py, "mtime")?.extract(py)?,
-        );
-        self.inner(py).borrow_mut().set_v1(filename, entry);
+        self.inner(py).borrow_mut().set_entry(filename, item.get_entry(py));
         Ok(py.None())
     }
 
diff --git a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/owning_dispatch.rs
@@ -20,8 +20,8 @@ 
         self.get_mut().clear()
     }
 
-    fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry) {
-        self.get_mut().set_v1(filename, entry)
+    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
+        self.get_mut().set_entry(filename, entry)
     }
 
     fn add_file(
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -37,7 +37,9 @@ 
     /// Remove information about all files in this map
     fn clear(&mut self);
 
-    fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry);
+    /// Add the given filename to the map if it is not already there, and
+    /// associate the given entry with it.
+    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry);
 
     /// Add or change the information associated to a given file.
     ///
@@ -319,8 +321,8 @@ 
     ///
     /// XXX Is temporary during a refactor of V1 dirstate and will disappear
     /// shortly.
-    fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry) {
-        self.set_v1_inner(&filename, entry)
+    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
+        self.set_entry(&filename, entry)
     }
 
     fn add_file(
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
@@ -758,7 +758,7 @@ 
         self.nodes_with_copy_source_count = 0;
     }
 
-    fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry) {
+    fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
         let node =
             self.get_or_insert(&filename).expect("no parse error in v1");
         node.data = NodeData::Entry(entry);
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -64,7 +64,7 @@ 
         self.other_parent_set = None;
     }
 
-    pub fn set_v1_inner(&mut self, filename: &HgPath, entry: DirstateEntry) {
+    pub fn set_entry(&mut self, filename: &HgPath, entry: DirstateEntry) {
         self.state_map.insert(filename.to_owned(), entry);
     }
 
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -664,7 +664,7 @@ 
                 new = True
             elif not entry.tracked:
                 entry.set_tracked()
-                self._rustmap.set_v1(filename, entry)
+                self._rustmap.set_dirstate_item(filename, entry)
                 new = True
             else:
                 # XXX This is probably overkill for more case, but we need this to
@@ -949,7 +949,7 @@ 
             """record that the current state of the file on disk is unknown"""
             entry = self[filename]
             entry.set_possibly_dirty()
-            self._rustmap.set_v1(filename, entry)
+            self._rustmap.set_dirstate_item(filename, entry)
 
         def set_clean(self, filename, mode, size, mtime):
             """mark a file as back to a clean state"""
@@ -957,9 +957,9 @@ 
             mtime = mtime & rangemask
             size = size & rangemask
             entry.set_clean(mode, size, mtime)
-            self._rustmap.set_v1(filename, entry)
+            self._rustmap.set_dirstate_item(filename, entry)
             self._rustmap.copymap().pop(filename, None)
 
         def __setitem__(self, key, value):
             assert isinstance(value, DirstateItem)
-            self._rustmap.set_v1(key, value)
+            self._rustmap.set_dirstate_item(key, value)