From 3c8f22456b79d100d0755c8fd0e93a0ccf742ae3 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 11 Mar 2024 20:17:47 +0900 Subject: [PATCH] revset_graph: remove lifetimed IndexEntry<'_> from look_ahead buffer Prepares for removing &CompositeIndex from the RevsetGraphIterator struct. The input iterator will also be changed to position-based. I've turned self.look_ahead.get().unwrap() into assertion, but it's not super important here. It's just for sanity that we've mapped missing edges properly. FWIW, we could say RevsetGraphIterator is an example of iterating *and* testing membership of the input revset (though the yielded entries are discarded.) --- .../default_index/revset_graph_iterator.rs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/src/default_index/revset_graph_iterator.rs b/lib/src/default_index/revset_graph_iterator.rs index 7613f9967..2fc6734f5 100644 --- a/lib/src/default_index/revset_graph_iterator.rs +++ b/lib/src/default_index/revset_graph_iterator.rs @@ -15,7 +15,7 @@ #![allow(missing_docs)] use std::cmp::{min, Ordering}; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use super::composite::CompositeIndex; use super::entry::{IndexEntry, IndexPosition}; @@ -127,9 +127,9 @@ pub(super) struct RevsetGraphIterator<'revset, 'index> { /// Commits in the input set we had to take out of the iterator while /// walking external edges. Does not necessarily include the commit /// we're currently about to emit. - look_ahead: BTreeMap>, + look_ahead: BTreeSet, /// The last consumed position. This is always the smallest key in the - /// look_ahead map, but it's faster to keep a separate field for it. + /// look_ahead set, but it's faster to keep a separate field for it. min_position: IndexPosition, /// Edges for commits not in the input set. edges: BTreeMap>, @@ -152,11 +152,10 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { } } - fn next_index_entry(&mut self) -> Option> { - if let Some(index_entry) = self.look_ahead.last_entry().map(|x| x.remove()) { - return Some(index_entry); - } - self.input_set_iter.next() + fn next_index_position(&mut self) -> Option { + self.look_ahead + .pop_last() + .or_else(|| self.input_set_iter.next().map(|entry| entry.position())) } fn edges_from_internal_commit( @@ -196,7 +195,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { for parent in index_entry.parents() { let parent_position = parent.position(); self.consume_to(parent_position); - if self.look_ahead.contains_key(&parent_position) { + if self.look_ahead.contains(&parent_position) { edges.push(IndexGraphEdge::direct(parent_position)); } else { let parent_edges = self.edges_from_external_commit(parent); @@ -232,7 +231,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { for parent in entry.parents() { let parent_position = parent.position(); self.consume_to(parent_position); - if self.look_ahead.contains_key(&parent_position) { + if self.look_ahead.contains(&parent_position) { // We have found a path back into the input set edges.push(IndexGraphEdge::indirect(parent_position)); } else if let Some(parent_edges) = self.edges.get(&parent_position) { @@ -280,7 +279,8 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { for edge in &edges { initial_targets.insert(edge.target); if edge.edge_type != RevsetGraphEdgeType::Missing { - let entry = self.look_ahead.get(&edge.target).unwrap().clone(); + assert!(self.look_ahead.contains(&edge.target)); + let entry = self.index.entry_by_pos(edge.target); min_generation = min(min_generation, entry.generation_number()); work.extend_from_slice(self.edges_from_internal_commit(&entry)); } @@ -299,7 +299,8 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { // Already visited continue; } - let entry = self.look_ahead.get(&edge.target).unwrap().clone(); + assert!(self.look_ahead.contains(&edge.target)); + let entry = self.index.entry_by_pos(edge.target); if entry.generation_number() < min_generation { continue; } @@ -316,7 +317,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { while pos < self.min_position { if let Some(next_entry) = self.input_set_iter.next() { let next_position = next_entry.position(); - self.look_ahead.insert(next_position, next_entry); + self.look_ahead.insert(next_position); self.min_position = next_position; } else { break; @@ -329,8 +330,9 @@ impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> { type Item = (CommitId, Vec); fn next(&mut self) -> Option { - let index_entry = self.next_index_entry()?; - let mut edges = self.pop_edges_from_internal_commit(&index_entry); + let position = self.next_index_position()?; + let entry = self.index.entry_by_pos(position); + let mut edges = self.pop_edges_from_internal_commit(&entry); if self.skip_transitive_edges { edges = self.remove_transitive_edges(edges); } @@ -338,6 +340,6 @@ impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> { .iter() .map(|edge| edge.to_revset_edge(self.index)) .collect(); - Some((index_entry.commit_id(), edges)) + Some((entry.commit_id(), edges)) } }