diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index 549dcc24cfa9a..f7eb76bbe72b5 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -21,7 +21,11 @@ use crate::traits::{FulfillmentError, ScrubbedTraitError}; mod derive_errors; -// FIXME: Do we need to use a `ThinVec` here? +// `ThinVec` is important for performance, but not for the usual memory layout reasons. +// `try_evaluate_obligations` is extremely hot and uses `retain_mut`. `ThinVec::retain_mut` is +// simple and sub-optimal in terms of how it moves elements, but it can be inlined. +// `Vec::retain_mut` is more sophisticated and minimizes element moves, but also contains more code +// and doesn't get inlined in `try_evaluate_obligations`, giving worse performance overall. type PendingObligations<'tcx> = ThinVec<(PredicateObligation<'tcx>, Option>>)>; @@ -453,7 +457,9 @@ mod size_asserts { use super::*; // tidy-alphabetical-start // Before #160005 this pair was greater than 128 bytes, which triggered the use of (slow) - // `memcpy` for moving elements of `PendingObligations`. + // `memcpy` for moving elements of `PendingObligations`. Then #160479 greatly reduced the + // number of `memcpy` operations in `try_evaluate_obligations`. So the size of this pair is + // much less important than it was, but still shouldn't be changed without some thought. static_assert_size!((PredicateObligation<'_>, Option>>), 104); // tidy-alphabetical-end }