diff --git a/lib/src/default_index/composite.rs b/lib/src/default_index/composite.rs index c4e7c7f74..39fe1565a 100644 --- a/lib/src/default_index/composite.rs +++ b/lib/src/default_index/composite.rs @@ -26,7 +26,7 @@ use super::entry::{ SmallLocalPositionsVec, }; use super::readonly::ReadonlyIndexSegment; -use super::rev_walk::{AncestorsBitSet, RevWalkAncestors}; +use super::rev_walk::AncestorsBitSet; use super::revset_engine; use crate::backend::{ChangeId, CommitId}; use crate::hex_util; @@ -330,17 +330,6 @@ impl<'a> CompositeIndex<'a> { self.heads_pos(result) } - pub(super) fn walk_revs( - &self, - wanted: &[IndexPosition], - unwanted: &[IndexPosition], - ) -> RevWalkAncestors<'a> { - let mut rev_walk = RevWalkAncestors::new(*self); - rev_walk.extend_wanted(wanted.iter().copied()); - rev_walk.extend_unwanted(unwanted.iter().copied()); - rev_walk - } - pub(super) fn all_heads(self) -> impl Iterator + 'a { self.all_heads_pos() .map(move |pos| self.entry_by_pos(pos).commit_id()) diff --git a/lib/src/default_index/rev_walk.rs b/lib/src/default_index/rev_walk.rs index d9bb51cac..1333b86d9 100644 --- a/lib/src/default_index/rev_walk.rs +++ b/lib/src/default_index/rev_walk.rs @@ -199,32 +199,44 @@ impl RevWalkQueue { } } -pub(super) type RevWalkAncestors<'a> = RevWalkImpl<'a, CompositeIndex<'a>>; - #[derive(Clone)] -pub(super) struct RevWalkImpl<'a, I: RevWalkIndex<'a>> { - index: I, - queue: RevWalkQueue, +#[must_use] +pub(super) struct RevWalkBuilder<'a> { + index: CompositeIndex<'a>, + // TODO: Use Vec<_> until queued item type is settled. + queue: RevWalkQueue, } -impl<'a> RevWalkAncestors<'a> { - pub(super) fn new(index: CompositeIndex<'a>) -> Self { +impl<'a> RevWalkBuilder<'a> { + pub fn new(index: CompositeIndex<'a>) -> Self { let queue = RevWalkQueue::new(); - RevWalkImpl { index, queue } + RevWalkBuilder { index, queue } } - pub(super) fn extend_wanted(&mut self, positions: impl IntoIterator) { + /// Adds head positions to be included. + pub fn wanted_heads(mut self, positions: impl IntoIterator) -> Self { self.queue.extend_wanted(positions, ()); + self } - pub(super) fn extend_unwanted(&mut self, positions: impl IntoIterator) { + /// Adds root positions to be excluded. The roots precede the heads. + pub fn unwanted_roots(mut self, positions: impl IntoIterator) -> Self { self.queue.extend_unwanted(positions); + self } - /// Filters entries by generation (or depth from the current wanted set.) + /// Walks ancestors. + pub fn ancestors(self) -> RevWalkAncestors<'a> { + RevWalkImpl { + index: self.index, + queue: self.queue, + } + } + + /// Walks ancestors within the `generation_range`. /// - /// The generation of the current wanted entries starts from 0. - pub fn filter_by_generation( + /// A generation number counts from the heads. + pub fn ancestors_filtered_by_generation( self, generation_range: Range, ) -> RevWalkAncestorsGenerationRange<'a> { @@ -236,7 +248,7 @@ impl<'a> RevWalkAncestors<'a> { /// /// Use this if you are only interested in descendants of the given roots. /// The caller still needs to filter out unwanted entries. - pub fn take_until_roots( + pub fn ancestors_until_roots( self, root_positions: &[IndexPosition], ) -> impl Iterator> + Clone + 'a { @@ -244,36 +256,45 @@ impl<'a> RevWalkAncestors<'a> { // it will perform better for unbalanced branchy history. // https://github.com/martinvonz/jj/pull/1492#discussion_r1160678325 let bottom_position = *root_positions.iter().min().unwrap_or(&IndexPosition::MAX); - self.take_while(move |entry| entry.position() >= bottom_position) + self.ancestors() + .take_while(move |entry| entry.position() >= bottom_position) } - /// Fully consumes the ancestors and walks back from `root_positions`. + /// Fully consumes ancestors and walks back from the `root_positions`. /// /// The returned iterator yields entries in order of ascending index /// position. - pub fn descendants(self, root_positions: &[IndexPosition]) -> RevWalkDescendants<'a> { + pub fn descendants( + self, + root_positions: impl IntoIterator, + ) -> RevWalkDescendants<'a> { + // TODO: collect HashSet<_> directly + let root_positions = Vec::from_iter(root_positions); RevWalkDescendants { - candidate_entries: self.take_until_roots(root_positions).collect(), - root_positions: root_positions.iter().copied().collect(), + candidate_entries: self.ancestors_until_roots(&root_positions).collect(), + root_positions: root_positions.into_iter().collect(), reachable_positions: HashSet::new(), } } - /// Fully consumes the ancestors and walks back from `root_positions` within - /// `generation_range`. + /// Fully consumes ancestors and walks back from the `root_positions` within + /// the `generation_range`. + /// + /// A generation number counts from the roots. /// /// The returned iterator yields entries in order of ascending index /// position. pub fn descendants_filtered_by_generation( self, - root_positions: &[IndexPosition], + root_positions: impl IntoIterator, generation_range: Range, ) -> RevWalkDescendantsGenerationRange<'a> { let index = self.index; - let entries = self.take_until_roots(root_positions); + let root_positions = Vec::from_iter(root_positions); + let entries = self.ancestors_until_roots(&root_positions); let descendants_index = RevWalkDescendantsIndex::build(index, entries); let mut queue = RevWalkQueue::new(); - for &pos in root_positions { + for pos in root_positions { // Do not add unreachable roots which shouldn't be visited if descendants_index.contains_pos(pos) { queue.push_wanted(Reverse(pos), ()); @@ -283,6 +304,15 @@ impl<'a> RevWalkAncestors<'a> { } } +pub(super) type RevWalkAncestors<'a> = RevWalkImpl<'a, CompositeIndex<'a>>; + +#[derive(Clone)] +#[must_use] +pub(super) struct RevWalkImpl<'a, I: RevWalkIndex<'a>> { + index: I, + queue: RevWalkQueue, +} + impl<'a, I: RevWalkIndex<'a>> Iterator for RevWalkImpl<'a, I> { type Item = IndexEntry<'a>; @@ -319,6 +349,7 @@ pub(super) type RevWalkDescendantsGenerationRange<'a> = RevWalkGenerationRangeImpl<'a, RevWalkDescendantsIndex<'a>>; #[derive(Clone)] +#[must_use] pub(super) struct RevWalkGenerationRangeImpl<'a, I: RevWalkIndex<'a>> { index: I, // Sort item generations in ascending order @@ -438,6 +469,7 @@ impl RevWalkItemGenerationRange { /// Walks descendants from the roots, in order of ascending index position. #[derive(Clone)] +#[must_use] pub(super) struct RevWalkDescendants<'a> { candidate_entries: Vec>, root_positions: HashSet, @@ -602,10 +634,10 @@ mod tests { let walk_commit_ids = |wanted: &[CommitId], unwanted: &[CommitId]| { let index = index.as_composite(); - let wanted_positions = to_positions_vec(index, wanted); - let unwanted_positions = to_positions_vec(index, unwanted); - index - .walk_revs(&wanted_positions, &unwanted_positions) + RevWalkBuilder::new(index) + .wanted_heads(to_positions_vec(index, wanted)) + .unwanted_roots(to_positions_vec(index, unwanted)) + .ancestors() .map(|entry| entry.commit_id()) .collect_vec() }; @@ -694,11 +726,10 @@ mod tests { let walk_commit_ids = |wanted: &[CommitId], unwanted: &[CommitId], range: Range| { let index = index.as_composite(); - let wanted_positions = to_positions_vec(index, wanted); - let unwanted_positions = to_positions_vec(index, unwanted); - index - .walk_revs(&wanted_positions, &unwanted_positions) - .filter_by_generation(range) + RevWalkBuilder::new(index) + .wanted_heads(to_positions_vec(index, wanted)) + .unwanted_roots(to_positions_vec(index, unwanted)) + .ancestors_filtered_by_generation(range) .map(|entry| entry.commit_id()) .collect_vec() }; @@ -773,10 +804,9 @@ mod tests { let walk_commit_ids = |wanted: &[CommitId], range: Range| { let index = index.as_composite(); - let wanted_positions = to_positions_vec(index, wanted); - index - .walk_revs(&wanted_positions, &[]) - .filter_by_generation(range) + RevWalkBuilder::new(index) + .wanted_heads(to_positions_vec(index, wanted)) + .ancestors_filtered_by_generation(range) .map(|entry| entry.commit_id()) .collect_vec() }; @@ -844,11 +874,9 @@ mod tests { let visible_heads = [&id_6, &id_8].map(Clone::clone); let walk_commit_ids = |roots: &[CommitId], heads: &[CommitId], range: Range| { let index = index.as_composite(); - let root_positions = to_positions_vec(index, roots); - let head_positions = to_positions_vec(index, heads); - index - .walk_revs(&head_positions, &[]) - .descendants_filtered_by_generation(&root_positions, range) + RevWalkBuilder::new(index) + .wanted_heads(to_positions_vec(index, heads)) + .descendants_filtered_by_generation(to_positions_vec(index, roots), range) .map(|entry| entry.commit_id()) .collect_vec() }; diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index d277516c5..8ed5bad8d 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -23,6 +23,7 @@ use std::sync::Arc; use itertools::Itertools; +use super::rev_walk::RevWalkBuilder; use super::revset_graph_iterator::RevsetGraphIterator; use crate::backend::{ChangeId, CommitId, MillisSinceEpoch}; use crate::default_index::{AsCompositeIndex, CompositeIndex, IndexEntry, IndexPosition}; @@ -744,15 +745,19 @@ impl<'index> EvaluationContext<'index> { let head_positions = head_set.positions(index).collect_vec(); if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset::new(move |index| { - Box::new(index.walk_revs(&head_positions, &[])) + Box::new( + RevWalkBuilder::new(index) + .wanted_heads(head_positions.iter().copied()) + .ancestors(), + ) }))) } else { 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()), + RevWalkBuilder::new(index) + .wanted_heads(head_positions.iter().copied()) + .ancestors_filtered_by_generation(generation.clone()), ) }))) } @@ -776,15 +781,21 @@ impl<'index> EvaluationContext<'index> { .collect_vec(); if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset::new(move |index| { - Box::new(index.walk_revs(&head_positions, &root_positions)) + Box::new( + RevWalkBuilder::new(index) + .wanted_heads(head_positions.iter().copied()) + .unwanted_roots(root_positions.iter().copied()) + .ancestors(), + ) }))) } else { 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()), + RevWalkBuilder::new(index) + .wanted_heads(head_positions.iter().copied()) + .unwanted_roots(root_positions.iter().copied()) + .ancestors_filtered_by_generation(generation.clone()), ) }))) } @@ -802,9 +813,9 @@ impl<'index> EvaluationContext<'index> { let root_positions_set: HashSet<_> = root_positions.iter().copied().collect(); let candidates = RevWalkRevset::new(move |index| { Box::new( - index - .walk_revs(&head_positions, &[]) - .take_until_roots(&root_positions), + RevWalkBuilder::new(index) + .wanted_heads(head_positions.iter().copied()) + .ancestors_until_roots(&root_positions), ) }); let predicate = as_pure_predicate_fn(move |_index, entry| { @@ -820,9 +831,9 @@ impl<'index> EvaluationContext<'index> { predicate, })) } else if generation_from_roots == &GENERATION_RANGE_FULL { - let mut positions = index - .walk_revs(&head_positions, &[]) - .descendants(&root_positions) + let mut positions = RevWalkBuilder::new(index) + .wanted_heads(head_positions) + .descendants(root_positions) .map(|entry| entry.position()) .collect_vec(); positions.reverse(); @@ -831,10 +842,10 @@ impl<'index> EvaluationContext<'index> { // 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 positions = index - .walk_revs(&head_positions, &[]) + let mut positions = RevWalkBuilder::new(index) + .wanted_heads(head_positions) .descendants_filtered_by_generation( - &root_positions, + root_positions, to_u32_generation_range(generation_from_roots)?, ) .map(|entry| entry.position()) @@ -856,9 +867,9 @@ impl<'index> EvaluationContext<'index> { .iter() .map(|entry| entry.position()) .collect_vec(); - let filled = index - .walk_revs(&candidate_positions, &[]) - .descendants(&candidate_positions) + let filled = RevWalkBuilder::new(index) + .wanted_heads(candidate_positions.iter().copied()) + .descendants(candidate_positions) .collect_positions_set(); let mut positions = vec![]; for candidate in candidate_entries {