index: make common_ancestors_pos() and heads_pos() return sorted Vec

Thanks to 56b3939, we no longer do random deletion, and most callers keep
Vec<IndexPosition> in descending order.

Author: yuja

lib/src/default_index/composite.rs

@@ -16,8 +16,8 @@
 
 use std::cmp::max;
 use std::cmp::Ordering;
+use std::cmp::Reverse;
 use std::collections::binary_heap;
-use std::collections::BTreeSet;
 use std::collections::BinaryHeap;
 use std::collections::HashSet;
 use std::iter;
@@ -312,20 +312,23 @@ impl CompositeIndex {
         false
     }
 
+    /// Computes the greatest common ancestors.
+    ///
+    /// The returned index positions are sorted in descending order.
     pub(super) fn common_ancestors_pos(
         &self,
         set1: &[IndexPosition],
         set2: &[IndexPosition],
-    ) -> BTreeSet<IndexPosition> {
+    ) -> Vec<IndexPosition> {
         let mut items1: BinaryHeap<_> = set1.iter().copied().collect();
         let mut items2: BinaryHeap<_> = set2.iter().copied().collect();
-        let mut result = BTreeSet::new();
+        let mut result = Vec::new();
         while let (Some(&pos1), Some(&pos2)) = (items1.peek(), items2.peek()) {
             match pos1.cmp(&pos2) {
                 Ordering::Greater => shift_to_parents(&mut items1, &self.entry_by_pos(pos1)),
                 Ordering::Less => shift_to_parents(&mut items2, &self.entry_by_pos(pos2)),
                 Ordering::Equal => {
-                    result.insert(pos1);
+                    result.push(pos1);
                     dedup_pop(&mut items1).unwrap();
                     dedup_pop(&mut items2).unwrap();
                 }
@@ -358,24 +361,29 @@ impl CompositeIndex {
 
     /// Returns the subset of positions in `candidate_positions` which refer to
     /// entries that are heads in the repository.
-    pub fn heads_pos(
-        &self,
-        candidate_positions: BTreeSet<IndexPosition>,
-    ) -> BTreeSet<IndexPosition> {
+    ///
+    /// The `candidate_positions` must be sorted in descending order, and have
+    /// no duplicates. The returned head positions are also sorted in descending
+    /// order.
+    pub fn heads_pos(&self, candidate_positions: Vec<IndexPosition>) -> Vec<IndexPosition> {
+        debug_assert!(candidate_positions
+            .iter()
+            .tuple_windows()
+            .all(|(a, b)| a > b));
         let Some(min_generation) = candidate_positions
             .iter()
             .map(|&pos| self.entry_by_pos(pos).generation_number())
             .min()
         else {
-            return BTreeSet::new();
+            return candidate_positions;
         };
 
         // Iterate though the candidates by reverse index position, keeping track of the
         // ancestors of already-found heads. If a candidate is an ancestor of an
         // already-found head, then it can be removed.
         let mut parents = BinaryHeap::new();
-        let mut heads = BTreeSet::new();
-        'outer: for candidate in candidate_positions.into_iter().rev() {
+        let mut heads = Vec::new();
+        'outer: for candidate in candidate_positions {
             while let Some(&parent) = parents.peek().filter(|&&parent| parent >= candidate) {
                 let entry = self.entry_by_pos(parent);
                 if entry.generation_number() <= min_generation {
@@ -391,7 +399,7 @@ impl CompositeIndex {
             // No parents matched, so this commit is a head.
             let entry = self.entry_by_pos(candidate);
             parents.extend(entry.parent_positions());
-            heads.insert(candidate);
+            heads.push(candidate);
         }
         heads
     }
@@ -475,9 +483,11 @@ impl Index for &CompositeIndex {
         &self,
         candidate_ids: &mut dyn Iterator<Item = &CommitId>,
     ) -> Result<Vec<CommitId>, IndexError> {
-        let candidate_positions: BTreeSet<_> = candidate_ids
+        let mut candidate_positions = candidate_ids
             .map(|id| self.commit_id_to_pos(id).unwrap())
-            .collect();
+            .collect_vec();
+        candidate_positions.sort_unstable_by_key(|&pos| Reverse(pos));
+        candidate_positions.dedup();
 
         Ok(self
             .heads_pos(candidate_positions)

lib/src/default_index/mod.rs

@@ -1097,7 +1097,7 @@ mod tests {
         // to (1,2) and matches the (1,2) of the first input set.
         assert_eq!(
             index.common_ancestors(&[id_1.clone(), id_2.clone()], &[id_5]),
-            vec![id_1.clone(), id_2.clone()]
+            vec![id_2.clone(), id_1.clone()]
         );
         assert_eq!(index.common_ancestors(&[id_1, id_2], &[id_3]), vec![id_0]);
     }
@@ -1198,9 +1198,16 @@ mod tests {
         // Multiple heads
         assert_eq!(
             index
-                .heads(&mut [id_4.clone(), id_3.clone()].iter())
+                .heads(&mut [id_3.clone(), id_4.clone()].iter())
                 .unwrap(),
-            vec![id_3.clone(), id_4]
+            vec![id_4.clone(), id_3.clone()]
+        );
+        // Duplicated inputs
+        assert_eq!(
+            index
+                .heads(&mut [id_4.clone(), id_3.clone(), id_4.clone()].iter())
+                .unwrap(),
+            vec![id_4.clone(), id_3.clone()]
         );
         // Merge commit and ancestors
         assert_eq!(
@@ -1212,7 +1219,7 @@ mod tests {
             index
                 .heads(&mut [id_5.clone(), id_3.clone()].iter())
                 .unwrap(),
-            vec![id_3.clone(), id_5.clone()]
+            vec![id_5.clone(), id_3.clone()]
         );
 
         assert_eq!(

lib/src/default_index/revset_engine.rs

@@ -17,7 +17,6 @@
 use std::cell::RefCell;
 use std::cmp::Ordering;
 use std::cmp::Reverse;
-use std::collections::BTreeSet;
 use std::collections::BinaryHeap;
 use std::collections::HashSet;
 use std::fmt;
@@ -934,9 +933,8 @@ impl EvaluationContext<'_> {
             }
             ResolvedExpression::Heads(candidates) => {
                 let candidate_set = self.evaluate(candidates)?;
-                let head_positions: BTreeSet<_> =
+                let positions =
                     index.heads_pos(candidate_set.positions().attach(index).try_collect()?);
-                let positions = head_positions.into_iter().rev().collect();
                 Ok(Box::new(EagerRevset { positions }))
             }
             ResolvedExpression::Roots(candidates) => {
@@ -966,12 +964,8 @@ impl EvaluationContext<'_> {
                 };
                 let mut positions = vec![position?];
                 for position in expression_positions_iter {
-                    positions = index
-                        .common_ancestors_pos(&positions, [position?].as_slice())
-                        .into_iter()
-                        .collect_vec();
+                    positions = index.common_ancestors_pos(&positions, &[position?]);
                 }
-                positions.reverse();
                 Ok(Box::new(EagerRevset { positions }))
             }
             ResolvedExpression::Latest { candidates, count } => {