diff --git a/lib/src/default_index/entry.rs b/lib/src/default_index/entry.rs index cfe4c57ed3..d741da06ec 100644 --- a/lib/src/default_index/entry.rs +++ b/lib/src/default_index/entry.rs @@ -14,7 +14,6 @@ #![allow(missing_docs)] -use std::cmp::Ordering; use std::fmt::{Debug, Formatter}; use std::hash::{Hash, Hasher}; @@ -107,21 +106,6 @@ impl<'a> IndexEntry<'a> { } } -#[derive(Clone, Eq, PartialEq)] -pub struct IndexEntryByPosition<'a>(pub IndexEntry<'a>); - -impl Ord for IndexEntryByPosition<'_> { - fn cmp(&self, other: &Self) -> Ordering { - self.0.pos.cmp(&other.0.pos) - } -} - -impl PartialOrd for IndexEntryByPosition<'_> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - /// Wrapper to sort `IndexPosition` by its generation number. /// /// This is similar to `IndexEntry` newtypes, but optimized for size and cache diff --git a/lib/src/default_index/mod.rs b/lib/src/default_index/mod.rs index ea3a0f41ac..4db14c9277 100644 --- a/lib/src/default_index/mod.rs +++ b/lib/src/default_index/mod.rs @@ -22,7 +22,7 @@ mod rev_walk; mod store; pub use self::composite::{CompositeIndex, IndexLevelStats, IndexStats}; -pub use self::entry::{IndexEntry, IndexEntryByPosition, IndexPosition}; +pub use self::entry::{IndexEntry, IndexPosition}; pub use self::mutable::DefaultMutableIndex; pub use self::readonly::DefaultReadonlyIndex; pub use self::rev_walk::{ diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 49b06b9fb3..3e91ab37ca 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -24,7 +24,7 @@ use std::sync::Arc; use itertools::Itertools; use crate::backend::{ChangeId, CommitId, MillisSinceEpoch}; -use crate::default_index::{CompositeIndex, IndexEntry, IndexEntryByPosition, IndexPosition}; +use crate::default_index::{CompositeIndex, IndexEntry, IndexPosition}; use crate::default_revset_graph_iterator::RevsetGraphIterator; use crate::id_prefix::{IdIndex, IdIndexSource, IdIndexSourceEntry}; use crate::index::{HexPrefix, PrefixResolution}; @@ -42,18 +42,27 @@ trait ToPredicateFn: fmt::Debug { /// Creates function that tests if the given entry is included in the set. /// /// The predicate function is evaluated in order of `RevsetIterator`. - fn to_predicate_fn(&self) -> Box) -> bool + '_>; + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a>; } impl ToPredicateFn for Box { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - ::to_predicate_fn(self) + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + ::to_predicate_fn(self, index) } } -trait InternalRevset<'index>: fmt::Debug + ToPredicateFn { +trait InternalRevset: fmt::Debug + ToPredicateFn { // All revsets currently iterate in order of descending index position - fn iter(&self) -> Box> + '_>; + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a>; fn into_predicate<'a>(self: Box) -> Box where @@ -61,23 +70,24 @@ trait InternalRevset<'index>: fmt::Debug + ToPredicateFn { } pub struct RevsetImpl<'index> { - inner: Box + 'index>, + inner: Box, index: CompositeIndex<'index>, } impl<'index> RevsetImpl<'index> { - fn new( - revset: Box + 'index>, - index: CompositeIndex<'index>, - ) -> Self { + fn new(revset: Box, index: CompositeIndex<'index>) -> Self { Self { inner: revset, index, } } + fn entries(&self) -> Box> + '_> { + self.inner.iter(self.index) + } + pub fn iter_graph_impl(&self) -> RevsetGraphIterator<'_, 'index> { - RevsetGraphIterator::new(self.index, self.inner.iter()) + RevsetGraphIterator::new(self.index, self.entries()) } } @@ -91,13 +101,12 @@ impl fmt::Debug for RevsetImpl<'_> { impl<'index> Revset<'index> for RevsetImpl<'index> { fn iter(&self) -> Box + '_> { - Box::new(self.inner.iter().map(|index_entry| index_entry.commit_id())) + Box::new(self.entries().map(|index_entry| index_entry.commit_id())) } fn commit_change_ids(&self) -> Box + '_> { Box::new( - self.inner - .iter() + self.entries() .map(|index_entry| (index_entry.commit_id(), index_entry.change_id())), ) } @@ -109,7 +118,7 @@ impl<'index> Revset<'index> for RevsetImpl<'index> { fn change_id_index(&self) -> Box { // TODO: Create a persistent lookup from change id to commit ids. let mut pos_by_change = IdIndex::builder(); - for entry in self.inner.iter() { + for entry in self.entries() { pos_by_change.insert(&entry.change_id(), entry.position()); } Box::new(ChangeIdIndexImpl { @@ -119,11 +128,11 @@ impl<'index> Revset<'index> for RevsetImpl<'index> { } fn is_empty(&self) -> bool { - self.iter().next().is_none() + self.entries().next().is_none() } fn count(&self) -> usize { - self.inner.iter().count() + self.entries().count() } } @@ -160,21 +169,28 @@ impl IdIndexSourceEntry for IndexEntry<'_> { } #[derive(Debug)] -struct EagerRevset<'index> { - index_entries: Vec>, +struct EagerRevset { + positions: Vec, } -impl EagerRevset<'static> { +impl EagerRevset { pub const fn empty() -> Self { EagerRevset { - index_entries: Vec::new(), + positions: Vec::new(), } } } -impl<'index> InternalRevset<'index> for EagerRevset<'index> { - fn iter(&self) -> Box> + '_> { - Box::new(self.index_entries.iter().cloned()) +impl InternalRevset for EagerRevset { + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { + let entries = self + .positions + .iter() + .map(move |&pos| index.entry_by_pos(pos)); + Box::new(entries) } fn into_predicate<'a>(self: Box) -> Box @@ -185,28 +201,53 @@ impl<'index> InternalRevset<'index> for EagerRevset<'index> { } } -impl ToPredicateFn for EagerRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - predicate_fn_from_iter(self.iter()) +impl ToPredicateFn for EagerRevset { + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + _index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + predicate_fn_from_positions(self.positions.iter().copied()) } } -struct RevWalkRevset { - walk: T, +struct RevWalkRevset { + walk: F, +} + +impl RevWalkRevset +where + // Returns trait object because we can't express the following constraints + // without using named lifetime and type parameter: + // + // for<'index> + // F: Fn(CompositeIndex<'index>) -> _, + // F::Output: Iterator> + 'index + // + // There's a workaround, but it doesn't help infer closure types. + // https://github.com/rust-lang/rust/issues/47815 + // https://users.rust-lang.org/t/hrtb-on-multiple-generics/34255 + F: Fn(CompositeIndex<'_>) -> Box> + '_>, +{ + fn new(walk: F) -> Self { + RevWalkRevset { walk } + } } -impl fmt::Debug for RevWalkRevset { +impl fmt::Debug for RevWalkRevset { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RevWalkRevset").finish_non_exhaustive() } } -impl<'index, T> InternalRevset<'index> for RevWalkRevset +impl InternalRevset for RevWalkRevset where - T: Iterator> + Clone, + F: Fn(CompositeIndex<'_>) -> Box> + '_>, { - fn iter(&self) -> Box> + '_> { - Box::new(self.walk.clone()) + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { + (self.walk)(index) } fn into_predicate<'a>(self: Box) -> Box @@ -217,37 +258,49 @@ where } } -impl<'index, T> ToPredicateFn for RevWalkRevset +impl ToPredicateFn for RevWalkRevset where - T: Iterator> + Clone, + F: Fn(CompositeIndex<'_>) -> Box> + '_>, { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - predicate_fn_from_iter(self.walk.clone()) + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + predicate_fn_from_entries(self.iter(index)) } } -fn predicate_fn_from_iter<'index, 'iter>( +fn predicate_fn_from_entries<'index, 'iter>( iter: impl Iterator> + 'iter, +) -> Box) -> bool + 'iter> { + predicate_fn_from_positions(iter.map(|entry| entry.position())) +} + +fn predicate_fn_from_positions<'iter>( + iter: impl Iterator + 'iter, ) -> Box) -> bool + 'iter> { let mut iter = iter.fuse().peekable(); Box::new(move |entry| { - while iter.next_if(|e| e.position() > entry.position()).is_some() { + while iter.next_if(|&pos| pos > entry.position()).is_some() { continue; } - iter.next_if(|e| e.position() == entry.position()).is_some() + iter.next_if(|&pos| pos == entry.position()).is_some() }) } #[derive(Debug)] -struct FilterRevset<'index, P> { - candidates: Box + 'index>, +struct FilterRevset

{ + candidates: Box, predicate: P, } -impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P> { - fn iter(&self) -> Box> + '_> { - let p = self.predicate.to_predicate_fn(); - Box::new(self.candidates.iter().filter(p)) +impl InternalRevset for FilterRevset

{ + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { + let p = self.predicate.to_predicate_fn(index); + Box::new(self.candidates.iter(index).filter(p)) } fn into_predicate<'a>(self: Box) -> Box @@ -258,10 +311,13 @@ impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P } } -impl ToPredicateFn for FilterRevset<'_, P> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.candidates.to_predicate_fn(); - let mut p2 = self.predicate.to_predicate_fn(); +impl ToPredicateFn for FilterRevset

{ + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.candidates.to_predicate_fn(index); + let mut p2 = self.predicate.to_predicate_fn(index); Box::new(move |entry| p1(entry) && p2(entry)) } } @@ -270,23 +326,29 @@ impl ToPredicateFn for FilterRevset<'_, P> { struct NotInPredicate(S); impl ToPredicateFn for NotInPredicate { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p = self.0.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p = self.0.to_predicate_fn(index); Box::new(move |entry| !p(entry)) } } #[derive(Debug)] -struct UnionRevset<'index> { - set1: Box + 'index>, - set2: Box + 'index>, +struct UnionRevset { + set1: Box, + set2: Box, } -impl<'index> InternalRevset<'index> for UnionRevset<'index> { - fn iter(&self) -> Box> + '_> { +impl InternalRevset for UnionRevset { + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { Box::new(UnionRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -298,10 +360,13 @@ impl<'index> InternalRevset<'index> for UnionRevset<'index> { } } -impl ToPredicateFn for UnionRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); +impl ToPredicateFn for UnionRevset { + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) || p2(entry)) } } @@ -317,9 +382,12 @@ where S1: ToPredicateFn, S2: ToPredicateFn, { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) || p2(entry)) } } @@ -355,16 +423,19 @@ impl<'index, I1: Iterator>, I2: Iterator { - set1: Box + 'index>, - set2: Box + 'index>, +struct IntersectionRevset { + set1: Box, + set2: Box, } -impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { - fn iter(&self) -> Box> + '_> { +impl InternalRevset for IntersectionRevset { + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { Box::new(IntersectionRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -376,10 +447,13 @@ impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { } } -impl ToPredicateFn for IntersectionRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); +impl ToPredicateFn for IntersectionRevset { + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) && p2(entry)) } } @@ -425,18 +499,21 @@ impl<'index, I1: Iterator>, I2: Iterator { +struct DifferenceRevset { // The minuend (what to subtract from) - set1: Box + 'index>, + set1: Box, // The subtrahend (what to subtract) - set2: Box + 'index>, + set2: Box, } -impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { - fn iter(&self) -> Box> + '_> { +impl InternalRevset for DifferenceRevset { + fn iter<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> { Box::new(DifferenceRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -448,10 +525,13 @@ impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { } } -impl ToPredicateFn for DifferenceRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); +impl ToPredicateFn for DifferenceRevset { + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) && !p2(entry)) } } @@ -529,20 +609,31 @@ impl<'index> EvaluationContext<'index> { fn evaluate( &self, expression: &ResolvedExpression, - ) -> Result + 'index>, RevsetEvaluationError> { + ) -> Result, RevsetEvaluationError> { + let index = self.index; match expression { ResolvedExpression::Commits(commit_ids) => { Ok(Box::new(self.revset_for_commit_ids(commit_ids))) } ResolvedExpression::Ancestors { heads, generation } => { let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); - let walk = self.index.walk_revs(&head_positions, &[]); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); if generation == &GENERATION_RANGE_FULL { - Ok(Box::new(RevWalkRevset { walk })) + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new(index.walk_revs(&head_positions, &[])) + }))) } else { - let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); - Ok(Box::new(RevWalkRevset { walk })) + let generation = to_u32_generation_range(generation)?; + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &[]) + .filter_by_generation(generation.clone()), + ) + }))) } } ResolvedExpression::Range { @@ -551,15 +642,28 @@ impl<'index> EvaluationContext<'index> { generation, } => { let root_set = self.evaluate(roots)?; - let root_positions = root_set.iter().map(|entry| entry.position()).collect_vec(); + let root_positions = root_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); - let walk = self.index.walk_revs(&head_positions, &root_positions); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); if generation == &GENERATION_RANGE_FULL { - Ok(Box::new(RevWalkRevset { walk })) + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new(index.walk_revs(&head_positions, &root_positions)) + }))) } else { - let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); - Ok(Box::new(RevWalkRevset { walk })) + let generation = to_u32_generation_range(generation)?; + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &root_positions) + .filter_by_generation(generation.clone()), + ) + }))) } } ResolvedExpression::DagRange { @@ -567,18 +671,26 @@ impl<'index> EvaluationContext<'index> { heads, generation_from_roots, } => { - let index = self.index; let root_set = self.evaluate(roots)?; - let root_positions = root_set.iter().map(|entry| entry.position()).collect_vec(); + let root_positions = root_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); if generation_from_roots == &(1..2) { - let walk = index - .walk_revs(&head_positions, &[]) - .take_until_roots(&root_positions); - let root_positions_set: HashSet<_> = root_positions.into_iter().collect(); - let candidates = Box::new(RevWalkRevset { walk }); - let predicate = PurePredicateFn(move |entry: &IndexEntry| { + let root_positions_set: HashSet<_> = root_positions.iter().copied().collect(); + let candidates = Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &[]) + .take_until_roots(&root_positions), + ) + })); + let predicate = as_pure_predicate_fn(move |_index, entry| { entry .parent_positions() .iter() @@ -591,61 +703,61 @@ impl<'index> EvaluationContext<'index> { predicate, })) } else if generation_from_roots == &GENERATION_RANGE_FULL { - let mut index_entries = index + let mut positions = index .walk_revs(&head_positions, &[]) .descendants(&root_positions) + .map(|entry| entry.position()) .collect_vec(); - index_entries.reverse(); - Ok(Box::new(EagerRevset { index_entries })) + positions.reverse(); + Ok(Box::new(EagerRevset { positions })) } else { // For small generation range, it might be better to build a reachable map // with generation bit set, which can be calculated incrementally from roots: // reachable[pos] = (reachable[parent_pos] | ...) << 1 - let mut index_entries = index + let mut positions = index .walk_revs(&head_positions, &[]) .descendants_filtered_by_generation( &root_positions, to_u32_generation_range(generation_from_roots)?, ) + .map(|entry| entry.position()) .collect_vec(); - index_entries.reverse(); - Ok(Box::new(EagerRevset { index_entries })) + positions.reverse(); + Ok(Box::new(EagerRevset { positions })) } } ResolvedExpression::Heads(candidates) => { let candidate_set = self.evaluate(candidates)?; - let head_positions: BTreeSet<_> = self - .index - .heads_pos(candidate_set.iter().map(|entry| entry.position()).collect()); - let index_entries = head_positions - .into_iter() - .rev() - .map(|pos| self.index.entry_by_pos(pos)) - .collect(); - Ok(Box::new(EagerRevset { index_entries })) + let head_positions: BTreeSet<_> = index.heads_pos( + candidate_set + .iter(index) + .map(|entry| entry.position()) + .collect(), + ); + let positions = head_positions.into_iter().rev().collect(); + Ok(Box::new(EagerRevset { positions })) } ResolvedExpression::Roots(candidates) => { - let candidate_entries = self.evaluate(candidates)?.iter().collect_vec(); + let candidate_entries = self.evaluate(candidates)?.iter(index).collect_vec(); let candidate_positions = candidate_entries .iter() .map(|entry| entry.position()) .collect_vec(); - let filled = self - .index + let filled = index .walk_revs(&candidate_positions, &[]) .descendants(&candidate_positions) .collect_positions_set(); - let mut index_entries = vec![]; + let mut positions = vec![]; for candidate in candidate_entries { if !candidate .parent_positions() .iter() .any(|parent| filled.contains(parent)) { - index_entries.push(candidate); + positions.push(candidate.position()); } } - Ok(Box::new(EagerRevset { index_entries })) + Ok(Box::new(EagerRevset { positions })) } ResolvedExpression::Latest { candidates, count } => { let candidate_set = self.evaluate(candidates)?; @@ -681,13 +793,11 @@ impl<'index> EvaluationContext<'index> { fn evaluate_predicate( &self, expression: &ResolvedPredicateExpression, - ) -> Result, RevsetEvaluationError> { + ) -> Result, RevsetEvaluationError> { match expression { - ResolvedPredicateExpression::Filter(predicate) => Ok(build_predicate_fn( - self.store.clone(), - self.index, - predicate, - )), + ResolvedPredicateExpression::Filter(predicate) => { + Ok(build_predicate_fn(self.store.clone(), predicate)) + } ResolvedPredicateExpression::Set(expression) => { Ok(self.evaluate(expression)?.into_predicate()) } @@ -703,43 +813,39 @@ impl<'index> EvaluationContext<'index> { } } - fn revset_for_commit_ids(&self, commit_ids: &[CommitId]) -> EagerRevset<'index> { - let mut index_entries = vec![]; - for id in commit_ids { - index_entries.push(self.index.entry_by_id(id).unwrap()); - } - index_entries.sort_unstable_by_key(|b| Reverse(b.position())); - index_entries.dedup(); - EagerRevset { index_entries } + fn revset_for_commit_ids(&self, commit_ids: &[CommitId]) -> EagerRevset { + let mut positions = commit_ids + .iter() + .map(|id| self.index.commit_id_to_pos(id).unwrap()) + .collect_vec(); + positions.sort_unstable_by_key(|&pos| Reverse(pos)); + positions.dedup(); + EagerRevset { positions } } - fn take_latest_revset( - &self, - candidate_set: &dyn InternalRevset<'index>, - count: usize, - ) -> EagerRevset<'index> { + fn take_latest_revset(&self, candidate_set: &dyn InternalRevset, count: usize) -> EagerRevset { if count == 0 { return EagerRevset::empty(); } #[derive(Clone, Eq, Ord, PartialEq, PartialOrd)] - struct Item<'a> { + struct Item { timestamp: MillisSinceEpoch, - entry: IndexEntryByPosition<'a>, // tie-breaker + pos: IndexPosition, // tie-breaker } - let make_rev_item = |entry: IndexEntry<'index>| { + let make_rev_item = |entry: IndexEntry<'_>| { let commit = self.store.get_commit(&entry.commit_id()).unwrap(); Reverse(Item { timestamp: commit.committer().timestamp.timestamp.clone(), - entry: IndexEntryByPosition(entry), + pos: entry.position(), }) }; // Maintain min-heap containing the latest (greatest) count items. For small // count and large candidate set, this is probably cheaper than building vec // and applying selection algorithm. - let mut candidate_iter = candidate_set.iter().map(make_rev_item).fuse(); + let mut candidate_iter = candidate_set.iter(self.index).map(make_rev_item).fuse(); let mut latest_items = BinaryHeap::from_iter(candidate_iter.by_ref().take(count)); for item in candidate_iter { let mut earliest = latest_items.peek_mut().unwrap(); @@ -749,12 +855,12 @@ impl<'index> EvaluationContext<'index> { } assert!(latest_items.len() <= count); - let mut index_entries = latest_items + let mut positions = latest_items .into_iter() - .map(|item| item.0.entry.0) + .map(|item| item.0.pos) .collect_vec(); - index_entries.sort_unstable_by_key(|b| Reverse(b.position())); - EagerRevset { index_entries } + positions.sort_unstable_by_key(|&pos| Reverse(pos)); + EagerRevset { positions } } } @@ -766,31 +872,46 @@ impl fmt::Debug for PurePredicateFn { } } -impl) -> bool> ToPredicateFn for PurePredicateFn { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - Box::new(&self.0) +impl ToPredicateFn for PurePredicateFn +where + F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, +{ + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let f = &self.0; + Box::new(move |entry| f(index, entry)) } } -fn pure_predicate_fn<'index>( - f: impl Fn(&IndexEntry<'_>) -> bool + 'index, -) -> Box { +fn as_pure_predicate_fn(f: F) -> PurePredicateFn +where + F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, +{ + PurePredicateFn(f) +} + +fn box_pure_predicate_fn<'a>( + f: impl Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool + 'a, +) -> Box { Box::new(PurePredicateFn(f)) } -fn build_predicate_fn<'index>( +fn build_predicate_fn( store: Arc, - index: CompositeIndex<'index>, predicate: &RevsetFilterPredicate, -) -> Box { +) -> Box { match predicate { RevsetFilterPredicate::ParentCount(parent_count_range) => { let parent_count_range = parent_count_range.clone(); - pure_predicate_fn(move |entry| parent_count_range.contains(&entry.num_parents())) + box_pure_predicate_fn(move |_index, entry| { + parent_count_range.contains(&entry.num_parents()) + }) } RevsetFilterPredicate::Description(pattern) => { let pattern = pattern.clone(); - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(commit.description()) }) @@ -800,14 +921,14 @@ fn build_predicate_fn<'index>( // TODO: Make these functions that take a needle to search for accept some // syntax for specifying whether it's a regex and whether it's // case-sensitive. - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.author().name) || pattern.matches(&commit.author().email) }) } RevsetFilterPredicate::Committer(pattern) => { let pattern = pattern.clone(); - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.committer().name) || pattern.matches(&commit.committer().email) @@ -820,11 +941,11 @@ fn build_predicate_fn<'index>( } else { Box::new(EverythingMatcher) }; - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |index, entry| { has_diff_from_parent(&store, index, entry, matcher.as_ref()) }) } - RevsetFilterPredicate::HasConflict => pure_predicate_fn(move |entry| { + RevsetFilterPredicate::HasConflict => box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); commit.has_conflict().unwrap() }), @@ -880,32 +1001,36 @@ mod tests { index.add_commit_data(id_3.clone(), new_change_id(), &[id_2.clone()]); index.add_commit_data(id_4.clone(), new_change_id(), &[id_3.clone()]); + let get_pos = |id: &CommitId| index.as_composite().commit_id_to_pos(id).unwrap(); let get_entry = |id: &CommitId| index.as_composite().entry_by_id(id).unwrap(); let make_entries = |ids: &[&CommitId]| ids.iter().map(|id| get_entry(id)).collect_vec(); let make_set = |ids: &[&CommitId]| -> Box { - let index_entries = make_entries(ids); - Box::new(EagerRevset { index_entries }) + let positions = ids.iter().copied().map(get_pos).collect(); + Box::new(EagerRevset { positions }) }; let set = make_set(&[&id_4, &id_3, &id_2, &id_0]); - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); assert!(!p(&get_entry(&id_1))); assert!(p(&get_entry(&id_0))); // Uninteresting entries can be skipped - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_3))); assert!(!p(&get_entry(&id_1))); assert!(p(&get_entry(&id_0))); let set = FilterRevset { candidates: make_set(&[&id_4, &id_2, &id_0]), - predicate: pure_predicate_fn(|entry| entry.commit_id() != id_4), + predicate: as_pure_predicate_fn(|_index, entry| entry.commit_id() != id_4), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2, &id_0])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2, &id_0]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -917,8 +1042,11 @@ mod tests { candidates: make_set(&[&id_4, &id_2, &id_0]), predicate: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -930,10 +1058,10 @@ mod tests { set2: make_set(&[&id_3, &id_2, &id_1]), }; assert_eq!( - set.iter().collect_vec(), + set.iter(index.as_composite()).collect_vec(), make_entries(&[&id_4, &id_3, &id_2, &id_1]) ); - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -944,8 +1072,11 @@ mod tests { set1: make_set(&[&id_4, &id_2, &id_0]), set2: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -956,8 +1087,11 @@ mod tests { set1: make_set(&[&id_4, &id_2, &id_0]), set2: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_4, &id_0])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_4, &id_0]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(!p(&get_entry(&id_2)));