changeset 47135:b6339a993b91

rust: Remove handling of `parents` in `DirstateMap` The Python wrapper class `dirstatemap` can take care of it. This removes the need to have both `_rustmap` and `_inner_rustmap`. Differential Revision: https://phab.mercurial-scm.org/D10555
author Simon Sapin <simon.sapin@octobus.net>
date Fri, 30 Apr 2021 15:40:11 +0200
parents 7109a38830c9
children 9aba0cde0ed9
files mercurial/dirstate.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-cpython/src/dirstate/dirstate_map.rs
diffstat 5 files changed, 19 insertions(+), 149 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/dirstate.py	Fri Apr 30 14:22:14 2021 +0200
+++ b/mercurial/dirstate.py	Fri Apr 30 15:40:11 2021 +0200
@@ -1750,6 +1750,7 @@
             self._opener = opener
             self._root = root
             self._filename = b'dirstate'
+            self._nodelen = 20
             self._parents = None
             self._dirtyparents = False
 
@@ -1778,25 +1779,15 @@
         def _rustmap(self):
             """
             Fills the Dirstatemap when called.
-            Use `self._inner_rustmap` if reading the dirstate is not necessary.
-            """
-            self._rustmap = self._inner_rustmap
-            self.read()
-            return self._rustmap
-
-        @propertycache
-        def _inner_rustmap(self):
-            """
-            Does not fill the Dirstatemap when called. This allows for
-            optimizations where only setting/getting the parents is needed.
             """
             use_dirstate_tree = self._ui.configbool(
                 b"experimental",
                 b"dirstate-tree.in-memory",
                 False,
             )
-            self._inner_rustmap = rustmod.DirstateMap(use_dirstate_tree)
-            return self._inner_rustmap
+            self._rustmap = rustmod.DirstateMap(use_dirstate_tree)
+            self.read()
+            return self._rustmap
 
         @property
         def copymap(self):
@@ -1807,7 +1798,6 @@
 
         def clear(self):
             self._rustmap.clear()
-            self._inner_rustmap.clear()
             self.setparents(
                 self._nodeconstants.nullid, self._nodeconstants.nullid
             )
@@ -1849,7 +1839,6 @@
             return fp
 
         def setparents(self, p1, p2):
-            self._rustmap.setparents(p1, p2)
             self._parents = (p1, p2)
             self._dirtyparents = True
 
@@ -1865,9 +1854,18 @@
                     # File doesn't exist, so the current state is empty
                     st = b''
 
-                try:
-                    self._parents = self._inner_rustmap.parents(st)
-                except ValueError:
+                l = len(st)
+                if l == self._nodelen * 2:
+                    self._parents = (
+                        st[: self._nodelen],
+                        st[self._nodelen : 2 * self._nodelen],
+                    )
+                elif l == 0:
+                    self._parents = (
+                        self._nodeconstants.nullid,
+                        self._nodeconstants.nullid,
+                    )
+                else:
                     raise error.Abort(
                         _(b'working directory state appears damaged!')
                     )
--- a/rust/hg-core/src/dirstate/dirstate_map.rs	Fri Apr 30 14:22:14 2021 +0200
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs	Fri Apr 30 15:40:11 2021 +0200
@@ -7,10 +7,8 @@
 
 use crate::dirstate::parsers::clear_ambiguous_mtime;
 use crate::dirstate::parsers::Timestamp;
-use crate::errors::HgError;
-use crate::revlog::node::NULL_NODE;
 use crate::{
-    dirstate::{parsers::PARENT_SIZE, EntryState},
+    dirstate::EntryState,
     pack_dirstate, parse_dirstate,
     utils::hg_path::{HgPath, HgPathBuf},
     CopyMap, DirsMultiset, DirstateEntry, DirstateError, DirstateMapError,
@@ -18,7 +16,6 @@
 };
 use micro_timer::timed;
 use std::collections::HashSet;
-use std::convert::TryInto;
 use std::iter::FromIterator;
 use std::ops::Deref;
 
@@ -30,8 +27,6 @@
     pub all_dirs: Option<DirsMultiset>,
     non_normal_set: Option<HashSet<HgPathBuf>>,
     other_parent_set: Option<HashSet<HgPathBuf>>,
-    parents: Option<DirstateParents>,
-    dirty_parents: bool,
 }
 
 /// Should only really be used in python interface code, for clarity
@@ -64,10 +59,6 @@
         self.copy_map.clear();
         self.non_normal_set = None;
         self.other_parent_set = None;
-        self.set_parents(&DirstateParents {
-            p1: NULL_NODE,
-            p2: NULL_NODE,
-        })
     }
 
     /// Add a tracked file to the dirstate
@@ -292,41 +283,6 @@
         Ok(self.all_dirs.as_ref().unwrap().contains(directory))
     }
 
-    pub fn parents(
-        &mut self,
-        file_contents: &[u8],
-    ) -> Result<&DirstateParents, DirstateError> {
-        if let Some(ref parents) = self.parents {
-            return Ok(parents);
-        }
-        let parents;
-        if file_contents.len() == PARENT_SIZE * 2 {
-            parents = DirstateParents {
-                p1: file_contents[..PARENT_SIZE].try_into().unwrap(),
-                p2: file_contents[PARENT_SIZE..PARENT_SIZE * 2]
-                    .try_into()
-                    .unwrap(),
-            };
-        } else if file_contents.is_empty() {
-            parents = DirstateParents {
-                p1: NULL_NODE,
-                p2: NULL_NODE,
-            };
-        } else {
-            return Err(
-                HgError::corrupted("Dirstate appears to be damaged").into()
-            );
-        }
-
-        self.parents = Some(parents);
-        Ok(self.parents.as_ref().unwrap())
-    }
-
-    pub fn set_parents(&mut self, parents: &DirstateParents) {
-        self.parents = Some(parents.clone());
-        self.dirty_parents = true;
-    }
-
     #[timed]
     pub fn read<'a>(
         &mut self,
@@ -347,11 +303,6 @@
                 .into_iter()
                 .map(|(path, copy)| (path.to_owned(), copy.to_owned())),
         );
-
-        if !self.dirty_parents {
-            self.set_parents(&parents);
-        }
-
         Ok(Some(parents))
     }
 
@@ -363,8 +314,6 @@
         let packed =
             pack_dirstate(&mut self.state_map, &self.copy_map, parents, now)?;
 
-        self.dirty_parents = false;
-
         self.set_non_normal_other_parent_entries(true);
         Ok(packed)
     }
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs	Fri Apr 30 14:22:14 2021 +0200
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs	Fri Apr 30 15:40:11 2021 +0200
@@ -8,10 +8,8 @@
 use crate::dirstate::parsers::pack_entry;
 use crate::dirstate::parsers::packed_entry_size;
 use crate::dirstate::parsers::parse_dirstate_entries;
-use crate::dirstate::parsers::parse_dirstate_parents;
 use crate::dirstate::parsers::Timestamp;
 use crate::matchers::Matcher;
-use crate::revlog::node::NULL_NODE;
 use crate::utils::hg_path::{HgPath, HgPathBuf};
 use crate::CopyMapIter;
 use crate::DirstateEntry;
@@ -27,8 +25,6 @@
 use crate::StatusOptions;
 
 pub struct DirstateMap {
-    parents: Option<DirstateParents>,
-    dirty_parents: bool,
     pub(super) root: ChildNodes,
 
     /// Number of nodes anywhere in the tree that have `.entry.is_some()`.
@@ -76,8 +72,6 @@
 impl DirstateMap {
     pub fn new() -> Self {
         Self {
-            parents: None,
-            dirty_parents: false,
             root: ChildNodes::default(),
             nodes_with_entry_count: 0,
             nodes_with_copy_source_count: 0,
@@ -288,10 +282,6 @@
 
 impl super::dispatch::DirstateMapMethods for DirstateMap {
     fn clear(&mut self) {
-        self.set_parents(&DirstateParents {
-            p1: NULL_NODE,
-            p2: NULL_NODE,
-        });
         self.root.clear();
         self.nodes_with_entry_count = 0;
         self.nodes_with_copy_source_count = 0;
@@ -453,29 +443,6 @@
         }
     }
 
-    fn parents(
-        &mut self,
-        file_contents: &[u8],
-    ) -> Result<&DirstateParents, DirstateError> {
-        if self.parents.is_none() {
-            let parents = if !file_contents.is_empty() {
-                parse_dirstate_parents(file_contents)?.clone()
-            } else {
-                DirstateParents {
-                    p1: NULL_NODE,
-                    p2: NULL_NODE,
-                }
-            };
-            self.parents = Some(parents);
-        }
-        Ok(self.parents.as_ref().unwrap())
-    }
-
-    fn set_parents(&mut self, parents: &DirstateParents) {
-        self.parents = Some(parents.clone());
-        self.dirty_parents = true;
-    }
-
     #[timed]
     fn read<'a>(
         &mut self,
@@ -515,10 +482,6 @@
             },
         )?;
 
-        if !self.dirty_parents {
-            self.set_parents(parents);
-        }
-
         Ok(Some(parents))
     }
 
@@ -554,7 +517,6 @@
                 );
             }
         }
-        self.dirty_parents = false;
         Ok(packed)
     }
 
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs	Fri Apr 30 14:22:14 2021 +0200
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs	Fri Apr 30 15:40:11 2021 +0200
@@ -73,13 +73,6 @@
         directory: &HgPath,
     ) -> Result<bool, DirstateMapError>;
 
-    fn parents(
-        &mut self,
-        file_contents: &[u8],
-    ) -> Result<&DirstateParents, DirstateError>;
-
-    fn set_parents(&mut self, parents: &DirstateParents);
-
     fn read<'a>(
         &mut self,
         file_contents: &'a [u8],
@@ -223,17 +216,6 @@
         self.has_dir(directory)
     }
 
-    fn parents(
-        &mut self,
-        file_contents: &[u8],
-    ) -> Result<&DirstateParents, DirstateError> {
-        self.parents(file_contents)
-    }
-
-    fn set_parents(&mut self, parents: &DirstateParents) {
-        self.set_parents(parents)
-    }
-
     fn read<'a>(
         &mut self,
         file_contents: &'a [u8],
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs	Fri Apr 30 14:22:14 2021 +0200
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs	Fri Apr 30 15:40:11 2021 +0200
@@ -13,8 +13,8 @@
 
 use cpython::{
     exc, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, PyList,
-    PyObject, PyResult, PySet, PyString, PyTuple, Python, PythonObject,
-    ToPyObject, UnsafePyLeaked,
+    PyObject, PyResult, PySet, PyString, Python, PythonObject, ToPyObject,
+    UnsafePyLeaked,
 };
 
 use crate::{
@@ -271,27 +271,6 @@
             .to_py_object(py))
     }
 
-    def parents(&self, st: PyObject) -> PyResult<PyTuple> {
-        self.inner(py).borrow_mut()
-            .parents(st.extract::<PyBytes>(py)?.data(py))
-            .map(|parents| dirstate_parents_to_pytuple(py, parents))
-            .or_else(|_| {
-                Err(PyErr::new::<exc::OSError, _>(
-                    py,
-                    "Dirstate error".to_string(),
-                ))
-            })
-    }
-
-    def setparents(&self, p1: PyObject, p2: PyObject) -> PyResult<PyObject> {
-        let p1 = extract_node_id(py, &p1)?;
-        let p2 = extract_node_id(py, &p2)?;
-
-        self.inner(py).borrow_mut()
-            .set_parents(&DirstateParents { p1, p2 });
-        Ok(py.None())
-    }
-
     def read(&self, st: PyObject) -> PyResult<Option<PyObject>> {
         match self.inner(py).borrow_mut()
             .read(st.extract::<PyBytes>(py)?.data(py))