mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-03 18:24:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

index: migrate RevWalk constructors to builder API

Browse source 
The current RevWalk constructors insert intermediate items to BinaryHeap
and convert them as needed. This is redundant, and I'm going to add another
parameter that should be applied to the queue first. That's why I decided
to factor out a builder type. I considered adding a few set of factory
functions that receive all parameters, but they looked messy because most of
the parameters are of [IndexPosition] type.

This patch also adds must_use to the builder and its return types, which are
all iterator-like.
This commit is contained in:
Yuya Nishihara 2024-03-08 17:54:23 +09:00
parent 008adecf23
commit 8480ee9e05
3 changed files with 102 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
lib/src/default_index/composite.rs
View file

@ -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<Item = CommitId> + 'a {
self.all_heads_pos()
.map(move |pos| self.entry_by_pos(pos).commit_id())

112
lib/src/default_index/rev_walk.rs
View file

@ -199,32 +199,44 @@ impl<P: Ord, T: Ord> RevWalkQueue<P, T> {
}
}
pub(super) type RevWalkAncestors<'a> = RevWalkImpl<'a, CompositeIndex<'a>>;
#[derive(Clone)]
pub(super) struct RevWalkImpl<'a, I: RevWalkIndex<'a>> {
index: I,
queue: RevWalkQueue<I::Position, ()>,
#[must_use]
pub(super) struct RevWalkBuilder<'a> {
index: CompositeIndex<'a>,
// TODO: Use Vec<_> until queued item type is settled.
queue: RevWalkQueue<IndexPosition, ()>,
}
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<Item = IndexPosition>) {
/// Adds head positions to be included.
pub fn wanted_heads(mut self, positions: impl IntoIterator<Item = IndexPosition>) -> Self {
self.queue.extend_wanted(positions, ());
self
}
pub(super) fn extend_unwanted(&mut self, positions: impl IntoIterator<Item = IndexPosition>) {
/// Adds root positions to be excluded. The roots precede the heads.
pub fn unwanted_roots(mut self, positions: impl IntoIterator<Item = IndexPosition>) -> 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<u32>,
) -> 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<Item = IndexEntry<'a>> + 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<Item = IndexPosition>,
) -> 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<Item = IndexPosition>,
generation_range: Range<u32>,
) -> 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<I::Position, ()>,
}
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<IndexEntry<'a>>,
root_positions: HashSet<IndexPosition>,
@ -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<u32>| {
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<u32>| {
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<u32>| {
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()
};

51
lib/src/default_index/revset_engine.rs
View file

@ -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 {