[Commented On] D11134: dirstate-map: move most of `dirstate.update_file` logic in the dsmap
baymax (Baymax, Your Personal Patch-care Companion)
phabricator at mercurial-scm.org
Tue Jul 20 07:22:47 UTC 2021
baymax added a comment.
baymax updated this revision to Diff 29553.
✅ refresh by Heptapod after a successful CI run (🐙 💚)
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D11134?vs=29400&id=29553
BRANCH
default
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D11134/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D11134
AFFECTED FILES
mercurial/dirstate.py
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-cpython/src/dirstate/dirstate_map.rs
rust/hg-cpython/src/dirstate/dispatch.rs
rust/hg-cpython/src/dirstate/non_normal_entries.rs
CHANGE DETAILS
diff --git a/rust/hg-cpython/src/dirstate/non_normal_entries.rs b/rust/hg-cpython/src/dirstate/non_normal_entries.rs
--- a/rust/hg-cpython/src/dirstate/non_normal_entries.rs
+++ b/rust/hg-cpython/src/dirstate/non_normal_entries.rs
@@ -26,6 +26,12 @@
def remove(&self, key: PyObject) -> PyResult<PyObject> {
self.dmap(py).non_normal_entries_remove(py, key)
}
+ def add(&self, key: PyObject) -> PyResult<PyObject> {
+ self.dmap(py).non_normal_entries_add(py, key)
+ }
+ def discard(&self, key: PyObject) -> PyResult<PyObject> {
+ self.dmap(py).non_normal_entries_discard(py, key)
+ }
def __richcmp__(&self, other: PyObject, op: CompareOp) -> PyResult<bool> {
match op {
CompareOp::Eq => self.is_equal_to(py, other),
diff --git a/rust/hg-cpython/src/dirstate/dispatch.rs b/rust/hg-cpython/src/dirstate/dispatch.rs
--- a/rust/hg-cpython/src/dirstate/dispatch.rs
+++ b/rust/hg-cpython/src/dirstate/dispatch.rs
@@ -20,6 +20,10 @@
self.get_mut().clear()
}
+ fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry) {
+ self.get_mut().set_v1(filename, entry)
+ }
+
fn add_file(
&mut self,
filename: &HgPath,
@@ -66,10 +70,14 @@
self.get_mut().non_normal_entries_contains(key)
}
- fn non_normal_entries_remove(&mut self, key: &HgPath) {
+ fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
self.get_mut().non_normal_entries_remove(key)
}
+ fn non_normal_entries_add(&mut self, key: &HgPath) {
+ self.get_mut().non_normal_entries_add(key)
+ }
+
fn non_normal_or_other_parent_paths(
&mut self,
) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>
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
@@ -105,6 +105,21 @@
}
}
+ def set_v1(&self, path: PyObject, item: PyObject) -> 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 {
+ state: state.try_into().expect("state is always valid"),
+ mtime: item.getattr(py, "mtime")?.extract(py)?,
+ size: item.getattr(py, "size")?.extract(py)?,
+ mode: item.getattr(py, "mode")?.extract(py)?,
+ };
+ self.inner(py).borrow_mut().set_v1(filename, entry);
+ Ok(py.None())
+ }
+
def addfile(
&self,
f: PyObject,
@@ -249,6 +264,22 @@
def non_normal_entries_remove(&self, key: PyObject) -> PyResult<PyObject> {
let key = key.extract::<PyBytes>(py)?;
+ let key = key.data(py);
+ let was_present = self
+ .inner(py)
+ .borrow_mut()
+ .non_normal_entries_remove(HgPath::new(key));
+ if !was_present {
+ let msg = String::from_utf8_lossy(key);
+ Err(PyErr::new::<exc::KeyError, _>(py, msg))
+ } else {
+ Ok(py.None())
+ }
+ }
+
+ def non_normal_entries_discard(&self, key: PyObject) -> PyResult<PyObject>
+ {
+ let key = key.extract::<PyBytes>(py)?;
self
.inner(py)
.borrow_mut()
@@ -256,6 +287,15 @@
Ok(py.None())
}
+ def non_normal_entries_add(&self, key: PyObject) -> PyResult<PyObject> {
+ let key = key.extract::<PyBytes>(py)?;
+ self
+ .inner(py)
+ .borrow_mut()
+ .non_normal_entries_add(HgPath::new(key.data(py)));
+ Ok(py.None())
+ }
+
def non_normal_or_other_parent_paths(&self) -> PyResult<PyList> {
let mut inner = self.inner(py).borrow_mut();
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,6 +37,8 @@
/// Remove information about all files in this map
fn clear(&mut self);
+ fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry);
+
/// Add or change the information associated to a given file.
///
/// `old_state` is the state in the entry that `get` would have returned
@@ -95,7 +97,13 @@
/// Mark the given path as "normal" file. This is only relevant in the flat
/// dirstate map where there is a separate `HashSet` that needs to be kept
/// up to date.
- fn non_normal_entries_remove(&mut self, key: &HgPath);
+ /// Returns whether the key was present in the set.
+ fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool;
+
+ /// Mark the given path as "non-normal" file.
+ /// This is only relevant in the flat dirstate map where there is a
+ /// separate `HashSet` that needs to be kept up to date.
+ fn non_normal_entries_add(&mut self, key: &HgPath);
/// Return an iterator of paths whose respective entry are either
/// "non-normal" (see `non_normal_entries_contains`) or "from other
@@ -280,6 +288,14 @@
self.clear()
}
+ /// Used to set a value directory.
+ ///
+ /// 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 add_file(
&mut self,
filename: &HgPath,
@@ -321,10 +337,14 @@
Ok(non_normal.contains(key))
}
- fn non_normal_entries_remove(&mut self, key: &HgPath) {
+ fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
self.non_normal_entries_remove(key)
}
+ fn non_normal_entries_add(&mut self, key: &HgPath) {
+ self.non_normal_entries_add(key)
+ }
+
fn non_normal_or_other_parent_paths(
&mut self,
) -> Box<dyn Iterator<Item = Result<&HgPath, DirstateV2ParseError>> + '_>
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
@@ -718,6 +718,16 @@
self.nodes_with_copy_source_count = 0;
}
+ fn set_v1(&mut self, filename: &HgPath, entry: DirstateEntry) {
+ let node =
+ self.get_or_insert(&filename).expect("no parse error in v1");
+ node.data = NodeData::Entry(entry);
+ node.children = ChildNodes::default();
+ node.copy_source = None;
+ node.descendants_with_entry_count = 0;
+ node.tracked_descendants_count = 0;
+ }
+
fn add_file(
&mut self,
filename: &HgPath,
@@ -925,7 +935,16 @@
})
}
- fn non_normal_entries_remove(&mut self, _key: &HgPath) {
+ fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
+ // Do nothing, this `DirstateMap` does not have a separate "non normal
+ // entries" set that need to be kept up to date.
+ if let Ok(Some(v)) = self.get(key) {
+ return v.is_non_normal();
+ }
+ false
+ }
+
+ fn non_normal_entries_add(&mut self, _key: &HgPath) {
// Do nothing, this `DirstateMap` does not have a separate "non normal
// entries" set that need to be kept up to date
}
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,6 +64,10 @@
self.other_parent_set = None;
}
+ pub fn set_v1_inner(&mut self, filename: &HgPath, entry: DirstateEntry) {
+ self.state_map.insert(filename.to_owned(), entry);
+ }
+
/// Add a tracked file to the dirstate
pub fn add_file(
&mut self,
@@ -245,10 +249,19 @@
}
}
- pub fn non_normal_entries_remove(&mut self, key: impl AsRef<HgPath>) {
+ pub fn non_normal_entries_remove(
+ &mut self,
+ key: impl AsRef<HgPath>,
+ ) -> bool {
self.get_non_normal_other_parent_entries()
.0
- .remove(key.as_ref());
+ .remove(key.as_ref())
+ }
+
+ pub fn non_normal_entries_add(&mut self, key: impl AsRef<HgPath>) {
+ self.get_non_normal_other_parent_entries()
+ .0
+ .insert(key.as_ref().into());
}
pub fn non_normal_entries_union(
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -217,6 +217,83 @@
if e.dm_otherparent:
self.otherparentset.add(f)
+ def reset_state(
+ self,
+ filename,
+ wc_tracked,
+ p1_tracked,
+ p2_tracked=False,
+ merged=False,
+ clean_p1=False,
+ clean_p2=False,
+ possibly_dirty=False,
+ parentfiledata=None,
+ ):
+ """Set a entry to a given state, diregarding all previous state
+
+ This is to be used by the part of the dirstate API dedicated to
+ adjusting the dirstate after a update/merge.
+
+ note: calling this might result to no entry existing at all if the
+ dirstate map does not see any point at having one for this file
+ anymore.
+ """
+ if merged and (clean_p1 or clean_p2):
+ msg = b'`merged` argument incompatible with `clean_p1`/`clean_p2`'
+ raise error.ProgrammingError(msg)
+ # copy information are now outdated
+ # (maybe new information should be in directly passed to this function)
+ self.copymap.pop(filename, None)
+
+ if not (p1_tracked or p2_tracked or wc_tracked):
+ self.dropfile(filename)
+ elif merged:
+ # XXX might be merged and removed ?
+ entry = self.get(filename)
+ if entry is not None and entry.tracked:
+ # XXX mostly replicate dirstate.other parent. We should get
+ # the higher layer to pass us more reliable data where `merged`
+ # actually mean merged. Dropping the else clause will show
+ # failure in `test-graft.t`
+ self.addfile(filename, merged=True)
+ else:
+ self.addfile(filename, from_p2=True)
+ elif not (p1_tracked or p2_tracked) and wc_tracked:
+ self.addfile(filename, added=True, possibly_dirty=possibly_dirty)
+ elif (p1_tracked or p2_tracked) and not wc_tracked:
+ # XXX might be merged and removed ?
+ old_entry = self._map.get(filename)
+ self._dirs_decr(filename, old_entry=old_entry, remove_variant=True)
+ self._map[filename] = DirstateItem(b'r', 0, 0, 0)
+ self.nonnormalset.add(filename)
+ elif clean_p2 and wc_tracked:
+ if p1_tracked or self.get(filename) is not None:
+ # XXX the `self.get` call is catching some case in
+ # `test-merge-remove.t` where the file is tracked in p1, the
+ # p1_tracked argument is False.
+ #
+ # In addition, this seems to be a case where the file is marked
+ # as merged without actually being the result of a merge
+ # action. So thing are not ideal here.
+ self.addfile(filename, merged=True)
+ else:
+ self.addfile(filename, from_p2=True)
+ elif not p1_tracked and p2_tracked and wc_tracked:
+ self.addfile(filename, from_p2=True, possibly_dirty=possibly_dirty)
+ elif possibly_dirty:
+ self.addfile(filename, possibly_dirty=possibly_dirty)
+ elif wc_tracked:
+ # this is a "normal" file
+ if parentfiledata is None:
+ msg = b'failed to pass parentfiledata for a normal file: %s'
+ msg %= filename
+ raise error.ProgrammingError(msg)
+ mode, size, mtime = parentfiledata
+ self.addfile(filename, mode=mode, size=size, mtime=mtime)
+ self.nonnormalset.discard(filename)
+ else:
+ assert False, 'unreachable'
+
def removefile(self, f, in_merge=False):
"""
Mark a file as removed in the dirstate.
@@ -496,6 +573,87 @@
possibly_dirty,
)
+ def reset_state(
+ self,
+ filename,
+ wc_tracked,
+ p1_tracked,
+ p2_tracked=False,
+ merged=False,
+ clean_p1=False,
+ clean_p2=False,
+ possibly_dirty=False,
+ parentfiledata=None,
+ ):
+ """Set a entry to a given state, disregarding all previous state
+
+ This is to be used by the part of the dirstate API dedicated to
+ adjusting the dirstate after a update/merge.
+
+ note: calling this might result to no entry existing at all if the
+ dirstate map does not see any point at having one for this file
+ anymore.
+ """
+ if merged and (clean_p1 or clean_p2):
+ msg = (
+ b'`merged` argument incompatible with `clean_p1`/`clean_p2`'
+ )
+ raise error.ProgrammingError(msg)
+ # copy information are now outdated
+ # (maybe new information should be in directly passed to this function)
+ self.copymap.pop(filename, None)
+
+ if not (p1_tracked or p2_tracked or wc_tracked):
+ self.dropfile(filename)
+ elif merged:
+ # XXX might be merged and removed ?
+ entry = self.get(filename)
+ if entry is not None and entry.tracked:
+ # XXX mostly replicate dirstate.other parent. We should get
+ # the higher layer to pass us more reliable data where `merged`
+ # actually mean merged. Dropping the else clause will show
+ # failure in `test-graft.t`
+ self.addfile(filename, merged=True)
+ else:
+ self.addfile(filename, from_p2=True)
+ elif not (p1_tracked or p2_tracked) and wc_tracked:
+ self.addfile(
+ filename, added=True, possibly_dirty=possibly_dirty
+ )
+ elif (p1_tracked or p2_tracked) and not wc_tracked:
+ # XXX might be merged and removed ?
+ self[filename] = DirstateItem(b'r', 0, 0, 0)
+ self.nonnormalset.add(filename)
+ elif clean_p2 and wc_tracked:
+ if p1_tracked or self.get(filename) is not None:
+ # XXX the `self.get` call is catching some case in
+ # `test-merge-remove.t` where the file is tracked in p1, the
+ # p1_tracked argument is False.
+ #
+ # In addition, this seems to be a case where the file is marked
+ # as merged without actually being the result of a merge
+ # action. So thing are not ideal here.
+ self.addfile(filename, merged=True)
+ else:
+ self.addfile(filename, from_p2=True)
+ elif not p1_tracked and p2_tracked and wc_tracked:
+ self.addfile(
+ filename, from_p2=True, possibly_dirty=possibly_dirty
+ )
+ elif possibly_dirty:
+ self.addfile(filename, possibly_dirty=possibly_dirty)
+ elif wc_tracked:
+ # this is a "normal" file
+ if parentfiledata is None:
+ msg = b'failed to pass parentfiledata for a normal file: %s'
+ msg %= filename
+ raise error.ProgrammingError(msg)
+ mode, size, mtime = parentfiledata
+ self.addfile(filename, mode=mode, size=size, mtime=mtime)
+ self.nonnormalset.discard(filename)
+ else:
+ assert False, 'unreachable'
+
def removefile(self, *args, **kwargs):
return self._rustmap.removefile(*args, **kwargs)
@@ -683,3 +841,7 @@
for name, _pseudo_entry in self.directories():
f[normcase(name)] = name
return f
+
+ def __setitem__(self, key, value):
+ assert isinstance(value, DirstateItem)
+ self._rustmap.set_v1(key, value)
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -564,30 +564,46 @@
if merged and (clean_p1 or clean_p2):
msg = b'`merged` argument incompatible with `clean_p1`/`clean_p2`'
raise error.ProgrammingError(msg)
- if not (p1_tracked or p2_tracked or wc_tracked):
- self._drop(filename)
- elif merged:
- assert wc_tracked
- assert self.in_merge # we are never in the "normallookup" case
- self.otherparent(filename)
- elif not (p1_tracked or p2_tracked) and wc_tracked:
- self._addpath(filename, added=True, possibly_dirty=possibly_dirty)
- self._map.copymap.pop(filename, None)
- elif (p1_tracked or p2_tracked) and not wc_tracked:
- self._remove(filename)
- elif clean_p2 and wc_tracked:
- assert p2_tracked
- self.otherparent(filename)
- elif not p1_tracked and p2_tracked and wc_tracked:
- self._addpath(filename, from_p2=True, possibly_dirty=possibly_dirty)
- self._map.copymap.pop(filename, None)
- elif possibly_dirty:
- self._addpath(filename, possibly_dirty=possibly_dirty)
- elif wc_tracked:
- self.normal(filename, parentfiledata=parentfiledata)
- # XXX We need something for file that are dirty after an update
- else:
- assert False, 'unreachable'
+
+ # note: I do not think we need to double check name clash here since we
+ # are in a update/merge case that should already have taken care of
+ # this. The test agrees
+
+ self._dirty = True
+ self._updatedfiles.add(filename)
+
+ need_parent_file_data = (
+ not (possibly_dirty or clean_p2 or merged)
+ and wc_tracked
+ and p1_tracked
+ )
+
+ # this mean we are doing call for file we do not really care about the
+ # data (eg: added or removed), however this should be a minor overhead
+ # compared to the overall update process calling this.
+ if need_parent_file_data:
+ if parentfiledata is None:
+ parentfiledata = self._get_filedata(filename)
+ mtime = parentfiledata[2]
+
+ if mtime > self._lastnormaltime:
+ # Remember the most recent modification timeslot for
+ # status(), to make sure we won't miss future
+ # size-preserving file content modifications that happen
+ # within the same timeslot.
+ self._lastnormaltime = mtime
+
+ self._map.reset_state(
+ filename,
+ wc_tracked,
+ p1_tracked,
+ p2_tracked=p2_tracked,
+ merged=merged,
+ clean_p1=clean_p1,
+ clean_p2=clean_p2,
+ possibly_dirty=possibly_dirty,
+ parentfiledata=parentfiledata,
+ )
def _addpath(
self,
To: marmoute, #hg-reviewers, Alphare
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210720/47843db3/attachment-0002.html>
More information about the Mercurial-patches
mailing list