fix: restore the merge_collapse invariant to prevent quadratic merges

src/sort.rs

@@ -94,26 +94,28 @@ impl<'a, T, C: Comparator<T>> SortState<'a, T, C> {
         let runs = &mut self.runs;
         while runs.len() > 1 {
             let l = runs.len();
-            if (l >= 3 && runs[l - 1].len <= runs[l - 2].len + runs[l - 1].len)
+            let (pos1, pos2) = if (l >= 3 && runs[l - 3].len <= runs[l - 2].len + runs[l - 1].len)
                 || (l >= 4 && runs[l - 4].len <= runs[l - 2].len + runs[l - 3].len)
             {
-                let (pos1, pos2) = if runs[l - 3].len < runs[l - 1].len {
+                if runs[l - 3].len < runs[l - 1].len {
                     (l - 3, l - 2)
                 } else {
                     (l - 2, l - 1)
-                };
-                let (run1, run2) = (runs[pos1], runs[pos2]);
-                debug_assert_eq!(run1.pos + run1.len, run2.pos);
-                runs.remove(pos2);
-                runs[pos1] = Run {
-                    pos: run1.pos,
-                    len: run1.len + run2.len,
-                };
-                let l = &mut self.list[run1.pos..][..run1.len + run2.len];
-                merge(l, run1.len, self.cmp)?;
+                }
+            } else if runs[l - 2].len <= runs[l - 1].len {
+                (l - 2, l - 1)
             } else {
                 break; // Invariant established.
-            }
+            };
+            let (run1, run2) = (runs[pos1], runs[pos2]);
+            debug_assert_eq!(run1.pos + run1.len, run2.pos);
+            runs.remove(pos2);
+            runs[pos1] = Run {
+                pos: run1.pos,
+                len: run1.len + run2.len,
+            };
+            let l = &mut self.list[run1.pos..][..run1.len + run2.len];
+            merge(l, run1.len, self.cmp)?;
         }
         Ok(())
     }
@@ -128,7 +130,7 @@ impl<'a, T, C: Comparator<T>> SortState<'a, T, C> {
                 pos2 -= 1;
             }
             let (run1, run2) = (runs[pos1], runs[pos2]);
-            debug_assert_eq!(run1.len, run2.pos);
+            debug_assert_eq!(run1.pos + run1.len, run2.pos);
             runs.remove(pos2);
             runs[pos1] = Run {
                 pos: run1.pos,

src/sort/tests.rs

@@ -107,6 +107,45 @@ fn stable() {
     }
 }
 
+/// Make sure the comparison count stays O(n log n) on random input. This
+/// regresses if `merge_collapse` stops maintaining the run stack invariant,
+/// which makes the merge pattern quadratic.
+#[test]
+fn merge_collapse_comparison_bound() {
+    use core::cell::Cell;
+    let n: usize = 100_000;
+    // xorshift64* so the input is deterministic.
+    let mut state: u64 = 0xC0FFEE ^ (n as u64) | 1;
+    let mut list: Vec<u32> = (0..n)
+        .map(|_| {
+            state ^= state >> 12;
+            state ^= state << 25;
+            state ^= state >> 27;
+            (state.wrapping_mul(0x2545F4914F6CDD1D) >> 33) as u32
+        })
+        .collect();
+    let count = Cell::new(0u64);
+    crate::sort_by(&mut list, |a, b| {
+        count.set(count.get() + 1);
+        a.cmp(b)
+    });
+    for pair in list.windows(2) {
+        assert!(pair[0] <= pair[1]);
+    }
+    // 3 * n * ceil(log2(n)) is generous for a merge sort (measured ~1.7x
+    // n log2 n here), while a quadratic merge pattern exceeds it by more
+    // than an order of magnitude at this size.
+    let log2_n = u64::from(usize::BITS - (n - 1).leading_zeros());
+    let bound = 3 * (n as u64) * log2_n;
+    assert!(
+        count.get() <= bound,
+        "{} comparisons at n={} exceeds bound {}",
+        count.get(),
+        n,
+        bound
+    );
+}
+
 /// Sort implementation convenience used for tests.
 fn sort<T: Ord>(list: &mut [T]) {
     super::SortState::new(list, &ord_t_comparator())