Skip to content

Commit 33aa940

Browse files
committed
index: in heads_pos(), compare generation when queuing parents
Since heads_pos() visits all ancestors of candidates down to min generation, traversal order doesn't really matter. We use BinaryHeap just because it's cheaper than DFS/BFS which would have to maintain a visited set. ``` revsets/heads(tags()) --------------------- 1 1.00 4.5±0.08ms 0 1.28 5.8±0.08ms ``` FWIW, is_ancestor_pos() uses DFS, which is fast in happy path, but slow if no match found.
1 parent 66254d5 commit 33aa940

File tree

2 files changed

+16
-48
lines changed

2 files changed

+16
-48
lines changed

lib/src/default_index/composite.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ use ref_cast::RefCastCustom;
3232

3333
use super::entry::IndexEntry;
3434
use super::entry::IndexPosition;
35-
use super::entry::IndexPositionByGeneration;
3635
use super::entry::LocalPosition;
3736
use super::entry::SmallIndexPositionsVec;
3837
use super::entry::SmallLocalPositionsVec;
@@ -386,36 +385,36 @@ impl CompositeIndex {
386385
// Add all parents of the candidates to the work queue. The parents and their
387386
// ancestors are not heads.
388387
// Also find the smallest generation number among the candidates.
389-
let mut work = BinaryHeap::new();
388+
let mut work = Vec::new();
390389
let mut min_generation = u32::MAX;
391390
for pos in &candidate_positions {
392391
let entry = self.entry_by_pos(*pos);
393392
min_generation = min(min_generation, entry.generation_number());
394-
for parent_entry in entry.parents() {
395-
work.push(IndexPositionByGeneration::from(&parent_entry));
396-
}
393+
work.extend(entry.parent_positions());
397394
}
395+
let mut work = BinaryHeap::from(work);
398396

399397
// Walk ancestors of the parents of the candidates. Remove visited commits from
400398
// set of candidates. Stop walking when we have gone past the minimum
401399
// candidate generation.
402-
while let Some(&item) = work.peek() {
403-
if item.generation_number() < min_generation {
404-
break;
405-
}
406-
let cur_pos = item.position();
400+
while let Some(&cur_pos) = work.peek() {
407401
candidate_positions.remove(&cur_pos);
408-
let mut parent_entries = self.entry_by_pos(cur_pos).parents();
409-
if let Some(parent_entry) = parent_entries.next() {
410-
assert!(parent_entry.position() < cur_pos);
411-
dedup_replace(&mut work, IndexPositionByGeneration::from(&parent_entry)).unwrap();
402+
let entry = self.entry_by_pos(cur_pos);
403+
if entry.generation_number() <= min_generation {
404+
dedup_pop(&mut work).unwrap();
405+
continue;
406+
}
407+
let mut parent_positions = entry.parent_positions().into_iter();
408+
if let Some(parent_pos) = parent_positions.next() {
409+
assert!(parent_pos < cur_pos);
410+
dedup_replace(&mut work, parent_pos).unwrap();
412411
} else {
413412
dedup_pop(&mut work).unwrap();
414413
continue;
415414
}
416-
for parent_entry in parent_entries {
417-
assert!(parent_entry.position() < cur_pos);
418-
work.push(IndexPositionByGeneration::from(&parent_entry));
415+
for parent_pos in parent_positions {
416+
assert!(parent_pos < cur_pos);
417+
work.push(parent_pos);
419418
}
420419
}
421420
candidate_positions

lib/src/default_index/entry.rs

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -121,34 +121,3 @@ impl<'a> IndexEntry<'a> {
121121
.map(move |pos| composite.entry_by_pos(pos))
122122
}
123123
}
124-
125-
/// Wrapper to sort `IndexPosition` by its generation number.
126-
///
127-
/// This is similar to `IndexEntry` newtypes, but optimized for size and cache
128-
/// locality. The original `IndexEntry` will have to be looked up when needed.
129-
#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
130-
pub(super) struct IndexPositionByGeneration {
131-
// Packed to single u64 value for faster comparison
132-
value: u64,
133-
}
134-
135-
impl IndexPositionByGeneration {
136-
fn new(IndexPosition(pos): IndexPosition, generation: u32) -> Self {
137-
let value = u64::from(generation) << 32 | u64::from(pos);
138-
IndexPositionByGeneration { value }
139-
}
140-
141-
pub fn position(&self) -> IndexPosition {
142-
IndexPosition(self.value as u32)
143-
}
144-
145-
pub fn generation_number(&self) -> u32 {
146-
(self.value >> 32) as u32
147-
}
148-
}
149-
150-
impl From<&IndexEntry<'_>> for IndexPositionByGeneration {
151-
fn from(entry: &IndexEntry<'_>) -> Self {
152-
Self::new(entry.position(), entry.generation_number())
153-
}
154-
}

0 commit comments

Comments
 (0)