This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12472

rust-dirstatemap: implement part of the `setparents` logic
ClosedPublic

Authored by Alphare on Apr 6 2022, 10:13 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHGfd21a9296af4: rust-dirstatemap: implement part of the `setparents` logic
rHG83b23e477d4f: rust-dirstatemap: implement part of the `setparents` logic
Summary

The Python code does many round-trip calls to the Rust dirstatemap when copy
information needs to be dropped in setparents.

This may result in improved performance on commit, update and other such
commands, but was mostly done to drop the last use of set_dirstate_item.
See inline comments for an asterisk about performance, and see next patch for
why set_dirstate_item has to go.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Apr 6 2022, 10:13 AM
marmoute accepted this revision.Apr 8 2022, 7:33 AM
Alphare added a commit: rHG83b23e477d4f: rust-dirstatemap: implement part of the `setparents` logic.Apr 8 2022, 11:57 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG83b23e477d4f: rust-dirstatemap: implement part of the `setparents` logic (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 32944.Apr 8 2022, 12:29 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 33046.Apr 12 2022, 10:58 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax updated this revision to Diff 33128.Apr 12 2022, 12:06 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Revision Contents
Changeset List

Diff 33128

mercurial/dirstatemap.py

### reading/setting parents ### reading/setting parents
def setparents(self, p1, p2, fold_p2=False): def setparents(self, p1, p2, fold_p2=False):
self._parents = (p1, p2) self._parents = (p1, p2)
self._dirtyparents = True self._dirtyparents = True
copies = {} copies = {}
if fold_p2: if fold_p2:
# Collect into an intermediate list to avoid a `RuntimeError` copies = self._map.setparents_fixup()
# exception due to mutation during iteration.
# TODO: move this the whole loop to Rust where `iter_mut`
# enables in-place mutation of elements of a collection while
# iterating it, without mutating the collection itself.
files_with_p2_info = [
f for f, s in self._map.items() if s.p2_info
]
rust_map = self._map
for f in files_with_p2_info:
e = rust_map.get(f)
source = self.copymap.pop(f, None)
if source:
copies[f] = source
e.drop_merge_data()
rust_map.set_dirstate_item(f, e)
return copies return copies
### disk interaction ### disk interaction
@propertycache @propertycache
def identity(self): def identity(self):
self._map self._map
return self.identity return self.identity

rust/hg-core/src/dirstate_tree/dirstate_map.rs

Some(node.full_path(on_disk)?) Some(node.full_path(on_disk)?)
} else { } else {
None None
}) })
}, },
))) )))
} }
/// Only public because it needs to be exposed to the Python layer.
/// It is not the full `setparents` logic, only the parts that mutate the
/// entries.
pub fn setparents_fixup(
&mut self,
) -> Result<Vec<(HgPathBuf, HgPathBuf)>, DirstateV2ParseError> {
// XXX
// All the copying and re-querying is quite inefficient, but this is
// still a lot better than doing it from Python.
//
// The better solution is to develop a mechanism for `iter_mut`,
// which will be a lot more involved: we're dealing with a lazy,
// append-mostly, tree-like data structure. This will do for now.
let mut copies = vec![];
let mut files_with_p2_info = vec![];
for res in self.iter() {
let (path, entry) = res?;
if entry.p2_info() {
files_with_p2_info.push(path.to_owned())
}
}
self.with_dmap_mut(|map| {
for path in files_with_p2_info.iter() {
let node = map.get_or_insert(path)?;
let entry =
node.data.as_entry_mut().expect("entry should exist");
entry.drop_merge_data();
if let Some(source) = node.copy_source.take().as_deref() {
copies.push((path.to_owned(), source.to_owned()));
}
}
Ok(copies)
})
}
pub fn debug_iter( pub fn debug_iter(
&self, &self,
all: bool, all: bool,
) -> Box< ) -> Box<
dyn Iterator< dyn Iterator<
Item = Result< Item = Result<
(&HgPath, (u8, i32, i32, i32)), (&HgPath, (u8, i32, i32, i32)),
DirstateV2ParseError, DirstateV2ParseError,

rust/hg-cpython/src/dirstate/dirstate_map.rs

{ {
let path = path.map_err(|e| v2_error(py, e))?; let path = path.map_err(|e| v2_error(py, e))?;
let path = PyBytes::new(py, path.as_bytes()); let path = PyBytes::new(py, path.as_bytes());
dirs.append(py, path.into_object()) dirs.append(py, path.into_object())
} }
Ok(dirs) Ok(dirs)
} }
def setparents_fixup(&self) -> PyResult<PyDict> {
let dict = PyDict::new(py);
let copies = self.inner(py).borrow_mut().setparents_fixup();
for (key, value) in copies.map_err(|e| v2_error(py, e))? {
dict.set_item(
py,
PyBytes::new(py, key.as_bytes()),
PyBytes::new(py, value.as_bytes()),
)?;
}
Ok(dict)
}
def debug_iter(&self, all: bool) -> PyResult<PyList> { def debug_iter(&self, all: bool) -> PyResult<PyList> {
let dirs = PyList::new(py, &[]); let dirs = PyList::new(py, &[]);
for item in self.inner(py).borrow().debug_iter(all) { for item in self.inner(py).borrow().debug_iter(all) {
let (path, (state, mode, size, mtime)) = let (path, (state, mode, size, mtime)) =
item.map_err(|e| v2_error(py, e))?; item.map_err(|e| v2_error(py, e))?;
let path = PyBytes::new(py, path.as_bytes()); let path = PyBytes::new(py, path.as_bytes());
let item = (path, state, mode, size, mtime); let item = (path, state, mode, size, mtime);
dirs.append(py, item.to_py_object(py).into_object()) dirs.append(py, item.to_py_object(py).into_object())