Mercurial > hg
changeset 46569:34827c95092c
copies-rust: remove the ancestor Oracle logic
We are not doing any `is_ancestor` call anymore. So we can drop that logic and
associated arguments.
Differential Revision: https://phab.mercurial-scm.org/D9645
| author | Pierre-Yves David <pierre-yves.david@octobus.net> |
|---|---|
| date | Tue, 15 Dec 2020 18:22:57 +0100 |
| parents | 0d840b9d200d |
| children | 389b0328b789 |
| files | mercurial/copies.py rust/hg-core/src/copy_tracing.rs rust/hg-cpython/src/copy_tracing.rs |
| diffstat | 3 files changed, 20 insertions(+), 104 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/copies.py Tue Dec 15 18:04:23 2020 +0100 +++ b/mercurial/copies.py Tue Dec 15 18:22:57 2020 +0100 @@ -363,7 +363,7 @@ if rustmod is not None: final_copies = rustmod.combine_changeset_copies( - list(revs), children_count, targetrev, revinfo, isancestor + list(revs), children_count, targetrev, revinfo ) else: isancestor = cached_is_ancestor(isancestor)
--- a/rust/hg-core/src/copy_tracing.rs Tue Dec 15 18:04:23 2020 +0100 +++ b/rust/hg-core/src/copy_tracing.rs Tue Dec 15 18:22:57 2020 +0100 @@ -281,46 +281,6 @@ } } -/// A struct responsible for answering "is X ancestors of Y" quickly -/// -/// The structure will delegate ancestors call to a callback, and cache the -/// result. -#[derive(Debug)] -struct AncestorOracle<'a, A: Fn(Revision, Revision) -> bool> { - inner: &'a A, - pairs: HashMap<(Revision, Revision), bool>, -} - -impl<'a, A: Fn(Revision, Revision) -> bool> AncestorOracle<'a, A> { - fn new(func: &'a A) -> Self { - Self { - inner: func, - pairs: HashMap::default(), - } - } - - fn record_overwrite(&mut self, anc: Revision, desc: Revision) { - self.pairs.insert((anc, desc), true); - } - - /// returns `true` if `anc` is an ancestors of `desc`, `false` otherwise - fn is_overwrite(&mut self, anc: Revision, desc: Revision) -> bool { - if anc > desc { - false - } else if anc == desc { - true - } else { - if let Some(b) = self.pairs.get(&(anc, desc)) { - *b - } else { - let b = (self.inner)(anc, desc); - self.pairs.insert((anc, desc), b); - b - } - } - } -} - struct ActionsIterator<'a> { changes: &'a ChangedFiles<'a>, parent: Parent, @@ -419,15 +379,13 @@ /// * ChangedFiles /// isancestors(low_rev, high_rev): callback to check if a revision is an /// ancestor of another -pub fn combine_changeset_copies<A: Fn(Revision, Revision) -> bool, D>( +pub fn combine_changeset_copies<D>( revs: Vec<Revision>, mut children_count: HashMap<Revision, usize>, target_rev: Revision, rev_info: RevInfoMaker<D>, - is_ancestor: &A, ) -> PathCopies { let mut all_copies = HashMap::new(); - let mut oracle = AncestorOracle::new(is_ancestor); let mut path_map = TwoWayPathMap::default(); @@ -450,7 +408,6 @@ // combine it with data for that revision let vertex_copies = add_from_changes( &mut path_map, - &mut oracle, &parent_copies, &changes, Parent::FirstParent, @@ -471,7 +428,6 @@ // combine it with data for that revision let vertex_copies = add_from_changes( &mut path_map, - &mut oracle, &parent_copies, &changes, Parent::SecondParent, @@ -491,7 +447,6 @@ vertex_copies, copies, &changes, - &mut oracle, )), }; } @@ -548,9 +503,8 @@ /// Combine ChangedFiles with some existing PathCopies information and return /// the result -fn add_from_changes<A: Fn(Revision, Revision) -> bool>( +fn add_from_changes( path_map: &mut TwoWayPathMap, - oracle: &mut AncestorOracle<A>, base_copies: &InternalPathCopies, changes: &ChangedFiles, parent: Parent, @@ -582,7 +536,6 @@ } Entry::Occupied(mut slot) => { let ttpc = slot.get_mut(); - oracle.record_overwrite(ttpc.rev, current_rev); ttpc.overwrite(current_rev, entry); } } @@ -595,7 +548,6 @@ // InternalPathCopies object. let deleted = path_map.tokenize(deleted_path); copies.entry(deleted).and_modify(|old| { - oracle.record_overwrite(old.rev, current_rev); old.mark_delete(current_rev); }); } @@ -608,31 +560,27 @@ /// /// In case of conflict, value from "major" will be picked, unless in some /// cases. See inline documentation for details. -fn merge_copies_dict<A: Fn(Revision, Revision) -> bool>( +fn merge_copies_dict( path_map: &TwoWayPathMap, current_merge: Revision, mut minor: InternalPathCopies, mut major: InternalPathCopies, changes: &ChangedFiles, - oracle: &mut AncestorOracle<A>, ) -> InternalPathCopies { // This closure exist as temporary help while multiple developper are // actively working on this code. Feel free to re-inline it once this // code is more settled. - let cmp_value = |oracle: &mut AncestorOracle<A>, - dest: &PathToken, - src_minor: &CopySource, - src_major: &CopySource| { - compare_value( - path_map, - current_merge, - changes, - oracle, - dest, - src_minor, - src_major, - ) - }; + let cmp_value = + |dest: &PathToken, src_minor: &CopySource, src_major: &CopySource| { + compare_value( + path_map, + current_merge, + changes, + dest, + src_minor, + src_major, + ) + }; if minor.is_empty() { major } else if major.is_empty() { @@ -661,10 +609,8 @@ } Some(src_major) => { let (pick, overwrite) = - cmp_value(oracle, &dest, &src_minor, src_major); + cmp_value(&dest, &src_minor, src_major); if overwrite { - oracle.record_overwrite(src_minor.rev, current_merge); - oracle.record_overwrite(src_major.rev, current_merge); let src = match pick { MergePick::Major => CopySource::new_from_merge( current_merge, @@ -704,10 +650,8 @@ } Some(src_minor) => { let (pick, overwrite) = - cmp_value(oracle, &dest, src_minor, &src_major); + cmp_value(&dest, src_minor, &src_major); if overwrite { - oracle.record_overwrite(src_minor.rev, current_merge); - oracle.record_overwrite(src_major.rev, current_merge); let src = match pick { MergePick::Major => CopySource::new_from_merge( current_merge, @@ -769,10 +713,8 @@ let (dest, src_major) = new; let (_, src_minor) = old; let (pick, overwrite) = - cmp_value(oracle, dest, src_minor, src_major); + cmp_value(dest, src_minor, src_major); if overwrite { - oracle.record_overwrite(src_minor.rev, current_merge); - oracle.record_overwrite(src_major.rev, current_merge); let src = match pick { MergePick::Major => CopySource::new_from_merge( current_merge, @@ -840,11 +782,10 @@ /// decide which side prevails in case of conflicting values #[allow(clippy::if_same_then_else)] -fn compare_value<A: Fn(Revision, Revision) -> bool>( +fn compare_value( path_map: &TwoWayPathMap, current_merge: Revision, changes: &ChangedFiles, - oracle: &mut AncestorOracle<A>, dest: &PathToken, src_minor: &CopySource, src_major: &CopySource,
--- a/rust/hg-cpython/src/copy_tracing.rs Tue Dec 15 18:04:23 2020 +0100 +++ b/rust/hg-cpython/src/copy_tracing.rs Tue Dec 15 18:22:57 2020 +0100 @@ -1,5 +1,4 @@ use cpython::ObjectProtocol; -use cpython::PyBool; use cpython::PyBytes; use cpython::PyDict; use cpython::PyList; @@ -26,32 +25,10 @@ children_count: PyDict, target_rev: Revision, rev_info: PyObject, - is_ancestor: PyObject, ) -> PyResult<PyDict> { let revs: PyResult<_> = revs.iter(py).map(|r| Ok(r.extract(py)?)).collect(); - // Wrap the `is_ancestor` python callback as a Rust closure - // - // No errors are expected from the Python side, and they will should only - // happens in case of programing error or severe data corruption. Such - // errors will raise panic and the rust-cpython harness will turn them into - // Python exception. - let is_ancestor_wrap = |anc: Revision, desc: Revision| -> bool { - is_ancestor - .call(py, (anc, desc), None) - .expect( - "rust-copy-tracing: python call to `is_ancestor` \ - failed", - ) - .cast_into::<PyBool>(py) - .expect( - "rust-copy-tracing: python call to `is_ancestor` \ - returned unexpected non-Bool value", - ) - .is_true() - }; - // Wrap the `rev_info_maker` python callback as a Rust closure // // No errors are expected from the Python side, and they will should only @@ -104,7 +81,6 @@ children_count?, target_rev, rev_info_maker, - &is_ancestor_wrap, ); let out = PyDict::new(py); for (dest, source) in res.into_iter() { @@ -134,8 +110,7 @@ revs: PyList, children: PyDict, target_rev: Revision, - rev_info: PyObject, - is_ancestor: PyObject + rev_info: PyObject ) ), )?;