Skip to content
Snippets Groups Projects
Commit c4839847 authored by Pierre-Yves David's avatar Pierre-Yves David :octopus:
Browse files

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
parent f9cd66dc
No related branches found
No related tags found
No related merge requests found
......@@ -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)
......
......@@ -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,8 +379,8 @@
/// * 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>,
......@@ -423,7 +383,6 @@
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();
......@@ -428,6 +387,5 @@
) -> 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,5 +503,5 @@
/// 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,
......@@ -552,5 +507,4 @@
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,9 +560,9 @@
///
/// 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,
......@@ -612,10 +564,9 @@
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.
......@@ -618,21 +569,18 @@
) -> 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,5 +609,5 @@
}
Some(src_major) => {
let (pick, overwrite) =
cmp_value(oracle, &dest, &src_minor, src_major);
cmp_value(&dest, &src_minor, src_major);
if overwrite {
......@@ -665,6 +613,4 @@
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,5 +650,5 @@
}
Some(src_minor) => {
let (pick, overwrite) =
cmp_value(oracle, &dest, src_minor, &src_major);
cmp_value(&dest, src_minor, &src_major);
if overwrite {
......@@ -708,6 +654,4 @@
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,5 +713,5 @@
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 {
......@@ -773,6 +717,4 @@
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,7 +782,7 @@
/// 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,
......@@ -844,7 +786,6 @@
path_map: &TwoWayPathMap,
current_merge: Revision,
changes: &ChangedFiles,
oracle: &mut AncestorOracle<A>,
dest: &PathToken,
src_minor: &CopySource,
src_major: &CopySource,
......
use cpython::ObjectProtocol;
use cpython::PyBool;
use cpython::PyBytes;
use cpython::PyDict;
use cpython::PyList;
......@@ -26,8 +25,7 @@
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();
......@@ -30,28 +28,7 @@
) -> 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
)
),
)?;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment