Auto merge of #131422 - GnomedDev:smallvec-predicate-obligations, r=compiler-errors

Use `ThinVec` for PredicateObligation storage

~~I noticed while profiling clippy on a project that a large amount of time is being spent allocating `Vec`s for `PredicateObligation`, and the `Vec`s are often quite small. This is an attempt to optimise this by using SmallVec to avoid heap allocations for these common small Vecs.~~

This PR turns all the `Vec<PredicateObligation>` into a single type alias while avoiding referring to `Vec` around it, then swaps the type over to `ThinVec<PredicateObligation>` and fixes the fallout. This also contains an implementation of `ThinVec::extract_if`, copied from `Vec::extract_if` and currently being upstreamed to https://github.com/Gankra/thin-vec/pull/66.

This leads to a small (0.2-0.7%) performance gain in the latest perf run.

4 files changed, 133 insertions, 34 deletions

diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs
index f225684d99f..fba2707922b 100644
--- a/compiler/rustc_data_structures/src/lib.rs
+++ b/compiler/rustc_data_structures/src/lib.rs
@@ -76,6 +76,7 @@ pub mod svh;
 pub mod sync;
 pub mod tagged_ptr;
 pub mod temp_dir;
+pub mod thinvec;
 pub mod transitive_relation;
 pub mod unhash;
 pub mod unord;
diff --git a/compiler/rustc_data_structures/src/obligation_forest/mod.rs b/compiler/rustc_data_structures/src/obligation_forest/mod.rs
index cfe7dd13e80..34a2464972a 100644
--- a/compiler/rustc_data_structures/src/obligation_forest/mod.rs
+++ b/compiler/rustc_data_structures/src/obligation_forest/mod.rs
@@ -75,6 +75,7 @@ use std::fmt::Debug;
 use std::hash;
 use std::marker::PhantomData;
 
+use thin_vec::ThinVec;
 use tracing::debug;
 
 use crate::fx::{FxHashMap, FxHashSet};
@@ -141,7 +142,7 @@ pub trait ObligationProcessor {
 #[derive(Debug)]
 pub enum ProcessResult<O, E> {
     Unchanged,
-    Changed(Vec<O>),
+    Changed(ThinVec<O>),
     Error(E),
 }
 
@@ -402,9 +403,10 @@ impl<O: ForestObligation> ObligationForest<O> {
     }
 
     /// Returns the set of obligations that are in a pending state.
-    pub fn map_pending_obligations<P, F>(&self, f: F) -> Vec<P>
+    pub fn map_pending_obligations<P, F, R>(&self, f: F) -> R
     where
         F: Fn(&O) -> P,
+        R: FromIterator<P>,
     {
         self.nodes
             .iter()
diff --git a/compiler/rustc_data_structures/src/obligation_forest/tests.rs b/compiler/rustc_data_structures/src/obligation_forest/tests.rs
index 8391e98ba8b..739ef74e3f7 100644
--- a/compiler/rustc_data_structures/src/obligation_forest/tests.rs
+++ b/compiler/rustc_data_structures/src/obligation_forest/tests.rs
@@ -1,5 +1,7 @@
 use std::fmt;
 
+use thin_vec::thin_vec;
+
 use super::*;
 
 impl<'a> super::ForestObligation for &'a str {
@@ -101,9 +103,9 @@ fn push_pop() {
     //        |-> A.3
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
+            "A" => ProcessResult::Changed(thin_vec!["A.1", "A.2", "A.3"]),
             "B" => ProcessResult::Error("B is for broken"),
-            "C" => ProcessResult::Changed(vec![]),
+            "C" => ProcessResult::Changed(thin_vec![]),
             "A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -123,8 +125,8 @@ fn push_pop() {
         |obligation| match *obligation {
             "A.1" => ProcessResult::Unchanged,
             "A.2" => ProcessResult::Unchanged,
-            "A.3" => ProcessResult::Changed(vec!["A.3.i"]),
-            "D" => ProcessResult::Changed(vec!["D.1", "D.2"]),
+            "A.3" => ProcessResult::Changed(thin_vec!["A.3.i"]),
+            "D" => ProcessResult::Changed(thin_vec!["D.1", "D.2"]),
             "A.3.i" | "D.1" | "D.2" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -139,11 +141,11 @@ fn push_pop() {
     //        |-> D.2 |-> D.2.i
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A.1" => ProcessResult::Changed(vec![]),
+            "A.1" => ProcessResult::Changed(thin_vec![]),
             "A.2" => ProcessResult::Error("A is for apple"),
-            "A.3.i" => ProcessResult::Changed(vec![]),
-            "D.1" => ProcessResult::Changed(vec!["D.1.i"]),
-            "D.2" => ProcessResult::Changed(vec!["D.2.i"]),
+            "A.3.i" => ProcessResult::Changed(thin_vec![]),
+            "D.1" => ProcessResult::Changed(thin_vec!["D.1.i"]),
+            "D.2" => ProcessResult::Changed(thin_vec!["D.2.i"]),
             "D.1.i" | "D.2.i" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -158,7 +160,7 @@ fn push_pop() {
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
             "D.1.i" => ProcessResult::Error("D is for dumb"),
-            "D.2.i" => ProcessResult::Changed(vec![]),
+            "D.2.i" => ProcessResult::Changed(thin_vec![]),
             _ => panic!("unexpected obligation {:?}", obligation),
         },
         |_| {},
@@ -184,10 +186,10 @@ fn success_in_grandchildren() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
-            "A.1" => ProcessResult::Changed(vec![]),
-            "A.2" => ProcessResult::Changed(vec!["A.2.i", "A.2.ii"]),
-            "A.3" => ProcessResult::Changed(vec![]),
+            "A" => ProcessResult::Changed(thin_vec!["A.1", "A.2", "A.3"]),
+            "A.1" => ProcessResult::Changed(thin_vec![]),
+            "A.2" => ProcessResult::Changed(thin_vec!["A.2.i", "A.2.ii"]),
+            "A.3" => ProcessResult::Changed(thin_vec![]),
             "A.2.i" | "A.2.ii" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -201,7 +203,7 @@ fn success_in_grandchildren() {
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
             "A.2.i" => ProcessResult::Unchanged,
-            "A.2.ii" => ProcessResult::Changed(vec![]),
+            "A.2.ii" => ProcessResult::Changed(thin_vec![]),
             _ => unreachable!(),
         },
         |_| {},
@@ -211,7 +213,7 @@ fn success_in_grandchildren() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A.2.i" => ProcessResult::Changed(vec!["A.2.i.a"]),
+            "A.2.i" => ProcessResult::Changed(thin_vec!["A.2.i.a"]),
             "A.2.i.a" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -222,7 +224,7 @@ fn success_in_grandchildren() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A.2.i.a" => ProcessResult::Changed(vec![]),
+            "A.2.i.a" => ProcessResult::Changed(thin_vec![]),
             _ => unreachable!(),
         },
         |_| {},
@@ -247,7 +249,7 @@ fn to_errors_no_throw() {
     forest.register_obligation("A");
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
+            "A" => ProcessResult::Changed(thin_vec!["A.1", "A.2", "A.3"]),
             "A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -269,7 +271,7 @@ fn diamond() {
     forest.register_obligation("A");
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A" => ProcessResult::Changed(vec!["A.1", "A.2"]),
+            "A" => ProcessResult::Changed(thin_vec!["A.1", "A.2"]),
             "A.1" | "A.2" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -280,8 +282,8 @@ fn diamond() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A.1" => ProcessResult::Changed(vec!["D"]),
-            "A.2" => ProcessResult::Changed(vec!["D"]),
+            "A.1" => ProcessResult::Changed(thin_vec!["D"]),
+            "A.2" => ProcessResult::Changed(thin_vec!["D"]),
             "D" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -295,7 +297,7 @@ fn diamond() {
         |obligation| match *obligation {
             "D" => {
                 d_count += 1;
-                ProcessResult::Changed(vec![])
+                ProcessResult::Changed(thin_vec![])
             }
             _ => unreachable!(),
         },
@@ -313,7 +315,7 @@ fn diamond() {
     forest.register_obligation("A'");
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A'" => ProcessResult::Changed(vec!["A'.1", "A'.2"]),
+            "A'" => ProcessResult::Changed(thin_vec!["A'.1", "A'.2"]),
             "A'.1" | "A'.2" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -324,8 +326,8 @@ fn diamond() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A'.1" => ProcessResult::Changed(vec!["D'", "A'"]),
-            "A'.2" => ProcessResult::Changed(vec!["D'"]),
+            "A'.1" => ProcessResult::Changed(thin_vec!["D'", "A'"]),
+            "A'.2" => ProcessResult::Changed(thin_vec!["D'"]),
             "D'" | "A'" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -366,7 +368,7 @@ fn done_dependency() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(vec![]),
+            "A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(thin_vec![]),
             _ => unreachable!(),
         },
         |_| {},
@@ -379,7 +381,9 @@ fn done_dependency() {
     forest.register_obligation("(A,B,C): Sized");
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "(A,B,C): Sized" => ProcessResult::Changed(vec!["A: Sized", "B: Sized", "C: Sized"]),
+            "(A,B,C): Sized" => {
+                ProcessResult::Changed(thin_vec!["A: Sized", "B: Sized", "C: Sized"])
+            }
             _ => unreachable!(),
         },
         |_| {},
@@ -399,10 +403,10 @@ fn orphan() {
 
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
-            "A" => ProcessResult::Changed(vec!["D", "E"]),
+            "A" => ProcessResult::Changed(thin_vec!["D", "E"]),
             "B" => ProcessResult::Unchanged,
-            "C1" => ProcessResult::Changed(vec![]),
-            "C2" => ProcessResult::Changed(vec![]),
+            "C1" => ProcessResult::Changed(thin_vec![]),
+            "C2" => ProcessResult::Changed(thin_vec![]),
             "D" | "E" => ProcessResult::Unchanged,
             _ => unreachable!(),
         },
@@ -416,7 +420,7 @@ fn orphan() {
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
             "D" | "E" => ProcessResult::Unchanged,
-            "B" => ProcessResult::Changed(vec!["D"]),
+            "B" => ProcessResult::Changed(thin_vec!["D"]),
             _ => unreachable!(),
         },
         |_| {},
@@ -459,7 +463,7 @@ fn simultaneous_register_and_error() {
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
             "A" => ProcessResult::Error("An error"),
-            "B" => ProcessResult::Changed(vec!["A"]),
+            "B" => ProcessResult::Changed(thin_vec!["A"]),
             _ => unreachable!(),
         },
         |_| {},
@@ -474,7 +478,7 @@ fn simultaneous_register_and_error() {
     let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
         |obligation| match *obligation {
             "A" => ProcessResult::Error("An error"),
-            "B" => ProcessResult::Changed(vec!["A"]),
+            "B" => ProcessResult::Changed(thin_vec!["A"]),
             _ => unreachable!(),
         },
         |_| {},
diff --git a/compiler/rustc_data_structures/src/thinvec.rs b/compiler/rustc_data_structures/src/thinvec.rs
new file mode 100644
index 00000000000..e60ac2cbc8b
--- /dev/null
+++ b/compiler/rustc_data_structures/src/thinvec.rs
@@ -0,0 +1,92 @@
+//! This is a copy-paste of `Vec::extract_if` for `ThinVec`.
+//!
+//! FIXME: <https://github.com/Gankra/thin-vec/pull/66> is merged, this can be removed.
+
+use std::{ptr, slice};
+
+use thin_vec::ThinVec;
+
+/// An iterator for [`ThinVec`] which uses a closure to determine if an element should be removed.
+#[must_use = "iterators are lazy and do nothing unless consumed"]
+pub struct ExtractIf<'a, T, F> {
+    vec: &'a mut ThinVec<T>,
+    /// The index of the item that will be inspected by the next call to `next`.
+    idx: usize,
+    /// The number of items that have been drained (removed) thus far.
+    del: usize,
+    /// The original length of `vec` prior to draining.
+    old_len: usize,
+    /// The filter test predicate.
+    pred: F,
+}
+
+impl<'a, T, F> ExtractIf<'a, T, F>
+where
+    F: FnMut(&mut T) -> bool,
+{
+    pub fn new(vec: &'a mut ThinVec<T>, filter: F) -> Self {
+        let old_len = vec.len();
+
+        // Guard against us getting leaked (leak amplification)
+        unsafe {
+            vec.set_len(0);
+        }
+
+        ExtractIf { vec, idx: 0, del: 0, old_len, pred: filter }
+    }
+}
+
+impl<T, F> Iterator for ExtractIf<'_, T, F>
+where
+    F: FnMut(&mut T) -> bool,
+{
+    type Item = T;
+    fn next(&mut self) -> Option<Self::Item> {
+        unsafe {
+            while self.idx < self.old_len {
+                let i = self.idx;
+                let v = slice::from_raw_parts_mut(self.vec.as_mut_ptr(), self.old_len);
+                let drained = (self.pred)(&mut v[i]);
+                // Update the index *after* the predicate is called. If the index
+                // is updated prior and the predicate panics, the element at this
+                // index would be leaked.
+                self.idx += 1;
+                if drained {
+                    self.del += 1;
+                    return Some(ptr::read(&v[i]));
+                } else if self.del > 0 {
+                    let del = self.del;
+                    let src: *const T = &v[i];
+                    let dst: *mut T = &mut v[i - del];
+                    ptr::copy_nonoverlapping(src, dst, 1);
+                }
+            }
+            None
+        }
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        (0, Some(self.old_len - self.idx))
+    }
+}
+
+impl<A, F> Drop for ExtractIf<'_, A, F> {
+    fn drop(&mut self) {
+        unsafe {
+            if self.idx < self.old_len && self.del > 0 {
+                // This is a pretty messed up state, and there isn't really an
+                // obviously right thing to do. We don't want to keep trying
+                // to execute `pred`, so we just backshift all the unprocessed
+                // elements and tell the vec that they still exist. The backshift
+                // is required to prevent a double-drop of the last successfully
+                // drained item prior to a panic in the predicate.
+                let ptr = self.vec.as_mut_ptr();
+                let src = ptr.add(self.idx);
+                let dst = src.sub(self.del);
+                let tail_len = self.old_len - self.idx;
+                src.copy_to(dst, tail_len);
+            }
+            self.vec.set_len(self.old_len - self.del);
+        }
+    }
+}