This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10746

rust: Return owned instead of borrowed DirstateEntry in DirstateMap APIs
ClosedPublic

Authored by SimonSapin on May 19 2021, 12:37 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG4ee9f419c52e: rust: Return owned instead of borrowed DirstateEntry in DirstateMap APIs
Summary

This will enable the tree-based DirstateMap to not always have an actual
DirstateEntry in memory for all nodes, but construct it on demand.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.May 19 2021, 12:37 PM
Alphare accepted this revision.May 21 2021, 3:48 AM
This revision is now accepted and ready to land.May 21 2021, 3:48 AM
baymax updated this revision to Diff 28170.May 21 2021, 4:37 AM

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

baymax updated this revision to Diff 28269.May 30 2021, 3:27 PM

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

Revision Contents
Changeset List

Diff 28281

rust/hg-core/src/dirstate.rs

/// A `DirstateEntry` with a size of `-2` means that it was merged from the /// A `DirstateEntry` with a size of `-2` means that it was merged from the
/// other parent. This allows revert to pick the right status back during a /// other parent. This allows revert to pick the right status back during a
/// merge. /// merge.
pub const SIZE_FROM_OTHER_PARENT: i32 = -2; pub const SIZE_FROM_OTHER_PARENT: i32 = -2;
pub type StateMap = FastHashMap<HgPathBuf, DirstateEntry>; pub type StateMap = FastHashMap<HgPathBuf, DirstateEntry>;
pub type StateMapIter<'a> = pub type StateMapIter<'a> =
Box<dyn Iterator<Item = (&'a HgPath, &'a DirstateEntry)> + Send + 'a>; Box<dyn Iterator<Item = (&'a HgPath, DirstateEntry)> + Send + 'a>;
pub type CopyMap = FastHashMap<HgPathBuf, HgPathBuf>; pub type CopyMap = FastHashMap<HgPathBuf, HgPathBuf>;
pub type CopyMapIter<'a> = pub type CopyMapIter<'a> =
Box<dyn Iterator<Item = (&'a HgPath, &'a HgPath)> + Send + 'a>; Box<dyn Iterator<Item = (&'a HgPath, &'a HgPath)> + Send + 'a>;
#[derive(Copy, Clone, Debug, Eq, PartialEq)] #[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum EntryState { pub enum EntryState {
Normal, Normal,

rust/hg-core/src/dirstate/dirs_multiset.rs

pub struct DirsMultiset { pub struct DirsMultiset {
inner: FastHashMap<HgPathBuf, u32>, inner: FastHashMap<HgPathBuf, u32>,
} }
impl DirsMultiset { impl DirsMultiset {
/// Initializes the multiset from a dirstate. /// Initializes the multiset from a dirstate.
/// ///
/// If `skip_state` is provided, skips dirstate entries with equal state. /// If `skip_state` is provided, skips dirstate entries with equal state.
pub fn from_dirstate<'a, I, P>( pub fn from_dirstate<I, P>(
dirstate: I, dirstate: I,
skip_state: Option<EntryState>, skip_state: Option<EntryState>,
) -> Result<Self, DirstateMapError> ) -> Result<Self, DirstateMapError>
where where
I: IntoIterator<Item = (P, &'a DirstateEntry)>, I: IntoIterator<Item = (P, DirstateEntry)>,
P: AsRef<HgPath>, P: AsRef<HgPath>,
{ {
let mut multiset = DirsMultiset { let mut multiset = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
for (filename, entry) in dirstate { for (filename, entry) in dirstate {
let filename = filename.as_ref(); let filename = filename.as_ref();
// This `if` is optimized out of the loop // This `if` is optimized out of the loop
let manifest: Vec<HgPathBuf> = vec![]; let manifest: Vec<HgPathBuf> = vec![];
let new = DirsMultiset::from_manifest(&manifest).unwrap(); let new = DirsMultiset::from_manifest(&manifest).unwrap();
let expected = DirsMultiset { let expected = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
let new = let new =
DirsMultiset::from_dirstate(&StateMap::default(), None).unwrap(); DirsMultiset::from_dirstate(StateMap::default(), None).unwrap();
let expected = DirsMultiset { let expected = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
} }
#[test] #[test]
fn test_dirsmultiset_new_no_skip() { fn test_dirsmultiset_new_no_skip() {
) )
}) })
.collect(); .collect();
let expected_inner = [("", 2), ("a", 2), ("b", 1), ("a/d", 1)] let expected_inner = [("", 2), ("a", 2), ("b", 1), ("a/d", 1)]
.iter() .iter()
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v)) .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect(); .collect();
let new = DirsMultiset::from_dirstate(&input_map, None).unwrap(); let new = DirsMultiset::from_dirstate(input_map, None).unwrap();
let expected = DirsMultiset { let expected = DirsMultiset {
inner: expected_inner, inner: expected_inner,
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
} }
#[test] #[test]
fn test_dirsmultiset_new_skip() { fn test_dirsmultiset_new_skip() {
// "a" incremented with "a/c" and "a/d/" // "a" incremented with "a/c" and "a/d/"
let expected_inner = [("", 1), ("a", 2)] let expected_inner = [("", 1), ("a", 2)]
.iter() .iter()
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v)) .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect(); .collect();
let new = let new =
DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal)) DirsMultiset::from_dirstate(input_map, Some(EntryState::Normal))
.unwrap(); .unwrap();
let expected = DirsMultiset { let expected = DirsMultiset {
inner: expected_inner, inner: expected_inner,
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
} }
} }

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

/// Both of these setters and their uses appear to be the simplest way to /// Both of these setters and their uses appear to be the simplest way to
/// emulate a Python lazy property, but it is ugly and unidiomatic. /// emulate a Python lazy property, but it is ugly and unidiomatic.
/// TODO One day, rewriting this struct using the typestate might be a /// TODO One day, rewriting this struct using the typestate might be a
/// good idea. /// good idea.
pub fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> { pub fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.all_dirs.is_none() { if self.all_dirs.is_none() {
self.all_dirs = Some(DirsMultiset::from_dirstate( self.all_dirs = Some(DirsMultiset::from_dirstate(
self.state_map.iter(), self.state_map.iter().map(|(k, v)| (k, *v)),
None, None,
)?); )?);
} }
Ok(()) Ok(())
} }
pub fn set_dirs(&mut self) -> Result<(), DirstateMapError> { pub fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.dirs.is_none() { if self.dirs.is_none() {
self.dirs = Some(DirsMultiset::from_dirstate( self.dirs = Some(DirsMultiset::from_dirstate(
self.state_map.iter(), self.state_map.iter().map(|(k, v)| (k, *v)),
Some(EntryState::Removed), Some(EntryState::Removed),
)?); )?);
} }
Ok(()) Ok(())
} }
pub fn has_tracked_dir( pub fn has_tracked_dir(
&mut self, &mut self,

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

fn len(&self) -> usize { fn len(&self) -> usize {
self.nodes_with_entry_count as usize self.nodes_with_entry_count as usize
} }
fn contains_key(&self, key: &HgPath) -> bool { fn contains_key(&self, key: &HgPath) -> bool {
self.get(key).is_some() self.get(key).is_some()
} }
fn get(&self, key: &HgPath) -> Option<&DirstateEntry> { fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
self.get_node(key)?.entry.as_ref() self.get_node(key)?.entry
} }
fn iter(&self) -> StateMapIter<'_> { fn iter(&self) -> StateMapIter<'_> {
Box::new(self.iter_nodes().filter_map(|(path, node)| { Box::new(self.iter_nodes().filter_map(|(path, node)| {
node.entry.as_ref().map(|entry| (&**path, entry)) node.entry.map(|entry| (&**path, entry))
})) }))
} }
} }

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

key: HgPathBuf, key: HgPathBuf,
value: HgPathBuf, value: HgPathBuf,
) -> Option<HgPathBuf>; ) -> Option<HgPathBuf>;
fn len(&self) -> usize; fn len(&self) -> usize;
fn contains_key(&self, key: &HgPath) -> bool; fn contains_key(&self, key: &HgPath) -> bool;
fn get(&self, key: &HgPath) -> Option<&DirstateEntry>; fn get(&self, key: &HgPath) -> Option<DirstateEntry>;
fn iter(&self) -> StateMapIter<'_>; fn iter(&self) -> StateMapIter<'_>;
} }
impl DirstateMapMethods for DirstateMap { impl DirstateMapMethods for DirstateMap {
fn clear(&mut self) { fn clear(&mut self) {
self.clear() self.clear()
} }
fn len(&self) -> usize { fn len(&self) -> usize {
(&**self).len() (&**self).len()
} }
fn contains_key(&self, key: &HgPath) -> bool { fn contains_key(&self, key: &HgPath) -> bool {
(&**self).contains_key(key) (&**self).contains_key(key)
} }
fn get(&self, key: &HgPath) -> Option<&DirstateEntry> { fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
(&**self).get(key) (&**self).get(key).cloned()
} }
fn iter(&self) -> StateMapIter<'_> { fn iter(&self) -> StateMapIter<'_> {
Box::new((&**self).iter().map(|(key, value)| (&**key, value))) Box::new((&**self).iter().map(|(key, value)| (&**key, *value)))
} }
} }

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

.try_into() .try_into()
.map_err(|e: HgError| { .map_err(|e: HgError| {
PyErr::new::<exc::ValueError, _>(py, e.to_string()) PyErr::new::<exc::ValueError, _>(py, e.to_string())
})?, })?,
); );
} }
let inner = if let Ok(map) = map.cast_as::<PyDict>(py) { let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
let dirstate = extract_dirstate(py, &map)?; let dirstate = extract_dirstate(py, &map)?;
DirsMultiset::from_dirstate(&dirstate, skip_state) let dirstate = dirstate.iter().map(|(k, v)| (k, *v));
DirsMultiset::from_dirstate(dirstate, skip_state)
.map_err(|e: DirstateMapError| { .map_err(|e: DirstateMapError| {
PyErr::new::<exc::ValueError, _>(py, e.to_string()) PyErr::new::<exc::ValueError, _>(py, e.to_string())
})? })?
} else { } else {
let map: Result<Vec<HgPathBuf>, PyErr> = map let map: Result<Vec<HgPathBuf>, PyErr> = map
.iter(py)? .iter(py)?
.map(|o| { .map(|o| {
Ok(HgPathBuf::from_bytes( Ok(HgPathBuf::from_bytes(

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

def get( def get(
&self, &self,
key: PyObject, key: PyObject,
default: Option<PyObject> = None default: Option<PyObject> = None
) -> PyResult<Option<PyObject>> { ) -> PyResult<Option<PyObject>> {
let key = key.extract::<PyBytes>(py)?; let key = key.extract::<PyBytes>(py)?;
match self.inner(py).borrow().get(HgPath::new(key.data(py))) { match self.inner(py).borrow().get(HgPath::new(key.data(py))) {
Some(entry) => { Some(entry) => {
Ok(Some(make_dirstate_tuple(py, entry)?)) Ok(Some(make_dirstate_tuple(py, &entry)?))
}, },
None => Ok(default) None => Ok(default)
} }
} }
def addfile( def addfile(
&self, &self,
f: PyObject, f: PyObject,
Ok(self.inner(py).borrow().contains_key(HgPath::new(key.data(py)))) Ok(self.inner(py).borrow().contains_key(HgPath::new(key.data(py))))
} }
def __getitem__(&self, key: PyObject) -> PyResult<PyObject> { def __getitem__(&self, key: PyObject) -> PyResult<PyObject> {
let key = key.extract::<PyBytes>(py)?; let key = key.extract::<PyBytes>(py)?;
let key = HgPath::new(key.data(py)); let key = HgPath::new(key.data(py));
match self.inner(py).borrow().get(key) { match self.inner(py).borrow().get(key) {
Some(entry) => { Some(entry) => {
Ok(make_dirstate_tuple(py, entry)?) Ok(make_dirstate_tuple(py, &entry)?)
}, },
None => Err(PyErr::new::<exc::KeyError, _>( None => Err(PyErr::new::<exc::KeyError, _>(
py, py,
String::from_utf8_lossy(key.as_bytes()), String::from_utf8_lossy(key.as_bytes()),
)), )),
} }
} }
pub fn get_inner_mut<'a>( pub fn get_inner_mut<'a>(
&'a self, &'a self,
py: Python<'a>, py: Python<'a>,
) -> RefMut<'a, Box<dyn DirstateMapMethods + Send>> { ) -> RefMut<'a, Box<dyn DirstateMapMethods + Send>> {
self.inner(py).borrow_mut() self.inner(py).borrow_mut()
} }
fn translate_key( fn translate_key(
py: Python, py: Python,
res: (&HgPath, &DirstateEntry), res: (&HgPath, DirstateEntry),
) -> PyResult<Option<PyBytes>> { ) -> PyResult<Option<PyBytes>> {
Ok(Some(PyBytes::new(py, res.0.as_bytes()))) Ok(Some(PyBytes::new(py, res.0.as_bytes())))
} }
fn translate_key_value( fn translate_key_value(
py: Python, py: Python,
res: (&HgPath, &DirstateEntry), res: (&HgPath, DirstateEntry),
) -> PyResult<Option<(PyBytes, PyObject)>> { ) -> PyResult<Option<(PyBytes, PyObject)>> {
let (f, entry) = res; let (f, entry) = res;
Ok(Some(( Ok(Some((
PyBytes::new(py, f.as_bytes()), PyBytes::new(py, f.as_bytes()),
make_dirstate_tuple(py, &entry)?, make_dirstate_tuple(py, &entry)?,
))) )))
} }
} }

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

fn len(&self) -> usize { fn len(&self) -> usize {
self.get().len() self.get().len()
} }
fn contains_key(&self, key: &HgPath) -> bool { fn contains_key(&self, key: &HgPath) -> bool {
self.get().contains_key(key) self.get().contains_key(key)
} }
fn get(&self, key: &HgPath) -> Option<&DirstateEntry> { fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
self.get().get(key) self.get().get(key)
} }
fn iter(&self) -> StateMapIter<'_> { fn iter(&self) -> StateMapIter<'_> {
self.get().iter() self.get().iter()
} }
} }