Skip to content

Commit

Permalink
index: implement generation filter on RevWalkGenerationRange
Browse files Browse the repository at this point in the history 
This will be a building block of 'parents(base)' revset. 'base---' will
be .filter_by_generation(3..4) for example. I think 'ancestors(base)' can
also have an optional generation parameter, but I haven't considered any
particular syntax yet.
  • Loading branch information
@yuja
yuja committed Dec 11, 2022
1 parent 832cf8d commit 4a889b9
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 5 deletions.
132 changes: 127 additions & 5 deletions lib/src/index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::fs::File;
use std::hash::{Hash, Hasher};
use std::io;
use std::io::{Cursor, Read, Write};
use std::ops::Bound;
use std::ops::{Bound, Range};
use std::path::PathBuf;
use std::sync::Arc;

Expand Down Expand Up @@ -980,10 +980,20 @@ enum RevWalkWorkItemState<T> {
Unwanted,
}

impl<T> RevWalkWorkItem<'_, T> {
impl<'a, T> RevWalkWorkItem<'a, T> {
fn is_wanted(&self) -> bool {
matches!(self.state, RevWalkWorkItemState::Wanted(_))
}

fn map_wanted<U>(self, f: impl FnOnce(T) -> U) -> RevWalkWorkItem<'a, U> {
RevWalkWorkItem {
entry: self.entry,
state: match self.state {
RevWalkWorkItemState::Wanted(t) => RevWalkWorkItemState::Wanted(f(t)),
RevWalkWorkItemState::Unwanted => RevWalkWorkItemState::Unwanted,
},
}
}
}

#[derive(Clone)]
Expand All @@ -1002,6 +1012,18 @@ impl<'a, T: Ord> RevWalkQueue<'a, T> {
}
}

fn map_wanted<U: Ord>(self, mut f: impl FnMut(T) -> U) -> RevWalkQueue<'a, U> {
RevWalkQueue {
index: self.index,
items: self
.items
.into_iter()
.map(|x| x.map_wanted(&mut f))
.collect(),
unwanted_count: self.unwanted_count,
}
}

fn push_wanted(&mut self, pos: IndexPosition, t: T) {
self.items.push(RevWalkWorkItem {
entry: IndexEntryByPosition(self.index.entry_by_pos(pos)),
Expand Down Expand Up @@ -1074,6 +1096,16 @@ impl<'a> RevWalk<'a> {
fn add_unwanted(&mut self, pos: IndexPosition) {
self.queue.push_unwanted(pos);
}

/// Filters entries by generation (or depth from the current wanted set.)
///
/// The generation of the current wanted entries starts from 0.
pub fn filter_by_generation(self, generation_range: Range<u32>) -> RevWalkGenerationRange<'a> {
RevWalkGenerationRange {
queue: self.queue.map_wanted(|()| 0),
generation_range,
}
}
}

impl<'a> Iterator for RevWalk<'a> {
Expand Down Expand Up @@ -1105,6 +1137,7 @@ impl<'a> Iterator for RevWalk<'a> {
#[derive(Clone)]
pub struct RevWalkGenerationRange<'a> {
queue: RevWalkQueue<'a, u32>,
generation_range: Range<u32>,
}

impl<'a> Iterator for RevWalkGenerationRange<'a> {
Expand All @@ -1113,7 +1146,10 @@ impl<'a> Iterator for RevWalkGenerationRange<'a> {
fn next(&mut self) -> Option<Self::Item> {
while let Some(item) = self.queue.pop() {
if let RevWalkWorkItemState::Wanted(mut known_gen) = item.state {
self.queue.push_wanted_parents(&item.entry.0, known_gen + 1);
let mut some_in_range = self.generation_range.contains(&known_gen);
if known_gen + 1 < self.generation_range.end {
self.queue.push_wanted_parents(&item.entry.0, known_gen + 1);
}
while let Some(x) = self.queue.pop_eq(&item.entry.0) {
// For wanted item, simply track all generation chains. This can
// be optimized if the wanted range is just upper/lower bounded.
Expand All @@ -1122,14 +1158,19 @@ impl<'a> Iterator for RevWalkGenerationRange<'a> {
// merge overlapping generation ranges.
match x.state {
RevWalkWorkItemState::Wanted(gen) if known_gen != gen => {
self.queue.push_wanted_parents(&item.entry.0, gen + 1);
some_in_range |= self.generation_range.contains(&gen);
if gen + 1 < self.generation_range.end {
self.queue.push_wanted_parents(&item.entry.0, gen + 1);
}
known_gen = gen;
}
RevWalkWorkItemState::Wanted(_) => {}
RevWalkWorkItemState::Unwanted => unreachable!(),
}
}
return Some(item.entry.0);
if some_in_range {
return Some(item.entry.0);
}
} else if self.queue.items.len() == self.queue.unwanted_count {
// No more wanted entries to walk
debug_assert!(!self.queue.items.iter().any(|x| x.is_wanted()));
Expand Down Expand Up @@ -2130,6 +2171,87 @@ mod tests {
);
}

#[test]
fn test_walk_revs_filter_by_generation() {
let mut index = MutableIndex::full(3);
// 8 6
// | |
// 7 5
// |/|
// 4 |
// | 3
// 2 |
// |/
// 1
// |
// 0
let id_0 = CommitId::from_hex("000000");
let id_1 = CommitId::from_hex("111111");
let id_2 = CommitId::from_hex("222222");
let id_3 = CommitId::from_hex("333333");
let id_4 = CommitId::from_hex("444444");
let id_5 = CommitId::from_hex("555555");
let id_6 = CommitId::from_hex("666666");
let id_7 = CommitId::from_hex("777777");
let id_8 = CommitId::from_hex("888888");
index.add_commit_data(id_0.clone(), new_change_id(), &[]);
index.add_commit_data(id_1.clone(), new_change_id(), &[id_0.clone()]);
index.add_commit_data(id_2.clone(), new_change_id(), &[id_1.clone()]);
index.add_commit_data(id_3.clone(), new_change_id(), &[id_1.clone()]);
index.add_commit_data(id_4.clone(), new_change_id(), &[id_2.clone()]);
index.add_commit_data(id_5.clone(), new_change_id(), &[id_4.clone(), id_3.clone()]);
index.add_commit_data(id_6.clone(), new_change_id(), &[id_5.clone()]);
index.add_commit_data(id_7.clone(), new_change_id(), &[id_4.clone()]);
index.add_commit_data(id_8.clone(), new_change_id(), &[id_7.clone()]);

let walk_commit_ids = |wanted: &[CommitId], unwanted: &[CommitId], range: Range<u32>| {
index
.walk_revs(wanted, unwanted)
.filter_by_generation(range)
.map(|entry| entry.commit_id())
.collect_vec()
};

// Simple generation bounds
assert_eq!(walk_commit_ids(&[&id_8].map(Clone::clone), &[], 0..0), []);
assert_eq!(
walk_commit_ids(&[&id_2].map(Clone::clone), &[], 0..3),
[&id_2, &id_1, &id_0].map(Clone::clone)
);

// Ancestors may be walked with different generations
assert_eq!(
walk_commit_ids(&[&id_6].map(Clone::clone), &[], 2..4),
[&id_4, &id_3, &id_2, &id_1].map(Clone::clone)
);
assert_eq!(
walk_commit_ids(&[&id_5].map(Clone::clone), &[], 2..3),
[&id_2, &id_1].map(Clone::clone)
);
assert_eq!(
walk_commit_ids(&[&id_5, &id_7].map(Clone::clone), &[], 2..3),
[&id_2, &id_1].map(Clone::clone)
);
assert_eq!(
walk_commit_ids(&[&id_7, &id_8].map(Clone::clone), &[], 0..2),
[&id_8, &id_7, &id_4].map(Clone::clone)
);
assert_eq!(
walk_commit_ids(&[&id_6, &id_7].map(Clone::clone), &[], 0..3),
[&id_7, &id_6, &id_5, &id_4, &id_3, &id_2].map(Clone::clone)
);
assert_eq!(
walk_commit_ids(&[&id_6, &id_7].map(Clone::clone), &[], 2..3),
[&id_4, &id_3, &id_2].map(Clone::clone)
);

// Ancestors of both wanted and unwanted commits are not walked
assert_eq!(
walk_commit_ids(&[&id_5].map(Clone::clone), &[&id_2].map(Clone::clone), 1..5),
[&id_4, &id_3].map(Clone::clone)
);
}

#[test]
fn test_heads() {
let mut index = MutableIndex::full(3);
Expand Down
38 changes: 38 additions & 0 deletions lib/tests/test_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ fn test_index_commits_criss_cross(use_git: bool) {
}
}

// RevWalk deduplicates chains by entry.
assert_eq!(
index
.walk_revs(&[left_commits[num_generations - 1].id().clone()], &[])
Expand Down Expand Up @@ -219,6 +220,43 @@ fn test_index_commits_criss_cross(use_git: bool) {
.count(),
2
);

// RevWalkGenerationRange deduplicates chains by (entry, generation), which may
// be more expensive than RevWalk, but should still finish in reasonable time.
assert_eq!(
index
.walk_revs(&[left_commits[num_generations - 1].id().clone()], &[])
.filter_by_generation(0..(num_generations + 1) as u32)
.count(),
2 * num_generations
);
assert_eq!(
index
.walk_revs(&[right_commits[num_generations - 1].id().clone()], &[])
.filter_by_generation(0..(num_generations + 1) as u32)
.count(),
2 * num_generations
);
assert_eq!(
index
.walk_revs(
&[left_commits[num_generations - 1].id().clone()],
&[left_commits[num_generations - 2].id().clone()]
)
.filter_by_generation(0..(num_generations + 1) as u32)
.count(),
2
);
assert_eq!(
index
.walk_revs(
&[right_commits[num_generations - 1].id().clone()],
&[right_commits[num_generations - 2].id().clone()]
)
.filter_by_generation(0..(num_generations + 1) as u32)
.count(),
2
);
}

#[test_case(false ; "local backend")]
Expand Down

0 comments on commit 4a889b9

Please sign in to comment.