From c1e08978b39e3ce1bf572c58b24724c89cf16387 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 3 Aug 2026 16:29:16 +1000 Subject: [PATCH 1/2] Optimize `try_evaluate_obligations` This function is very sub-optimal, perf-wise: it takes `self.obligations.pending` (with `mem::take`) and iterates over the elements, checking each one. But most of the time no progress is made and all the obligations get pushed back onto `self.obligations.pending`. This drain + reconstruct approach is very expensive, mostly because the new `pending` vec is built by pushing one element at a time, which requires repeated reallocations. And this vec can have thousands of elements in it, in extreme cases. Also, `obligation` and `stalled_on` get passed by value to `evaluate_root_goal` (`obligation` as `goal`), which then usually passes the values back in the `GoalEvaluation` which is immediately deconstructed. This is a lot of wasted value moves. This commit optimizes things in two ways. - It prioritizes the hot path. This involves checking in advance if there is an inspector (usually not) and adding `goal_remains_stalled` which takes `stalled_on` by reference. This hot path avoids all the value moves and `GoalEvaluation` construction/deconstruction and gets to the very common "nothing needed to be done" outcome as quickly as possible. - It uses `retain_mut` to update `self.obligations.pending`. This requires some adjustments (e.g. handling recursion via the `overflowed` flag with some cleanup code after the `retain_mut` call, and cloning obligations in the error cases). --- .../src/solve/eval_ctxt/mod.rs | 15 +++++ .../src/solve/fulfill.rs | 59 +++++++++++++++---- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs index 004d1df069d6e..c43bd7b2c494e 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs @@ -179,6 +179,11 @@ pub trait SolverDelegateEvalExt: SolverDelegate { stalled_on: Option>, ) -> Result, NoSolution>; + /// Checks whether a stalled goal would remain stalled if re-evaluated, without consuming + /// `stalled_on`. + fn goal_remains_stalled(&self, stalled_on: &GoalStalledOn) + -> Option; + /// Checks whether evaluating `goal` may hold while treating not-yet-defined /// opaque types as being kind of rigid. /// @@ -260,6 +265,16 @@ where } } + fn goal_remains_stalled( + &self, + stalled_on: &GoalStalledOn, + ) -> Option { + match rerunning_stalled_goal_may_make_progress(self, Some(stalled_on)) { + RerunStalled::WontMakeProgress(certainty) => Some(certainty), + RerunStalled::MayMakeProgress => None, + } + } + #[instrument(level = "debug", skip(self), ret)] fn root_goal_may_hold_opaque_types_jank( &self, diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index b221824c14575..375a6054bab8c 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -138,7 +138,6 @@ impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> { } fn inspect_evaluated_obligation( - &self, infcx: &InferCtxt<'tcx>, obligation: &PredicateObligation<'tcx>, result: &Result>, NoSolution>, @@ -196,22 +195,41 @@ where fn try_evaluate_obligations(&mut self, infcx: &InferCtxt<'tcx>) -> TraitErrors { assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); let mut errors = TraitErrors::NoErrors; + let delegate = <&SolverDelegate<'tcx>>::from(infcx); + let has_inspector = infcx.obligation_inspector.get().is_some(); loop { let mut any_changed = false; - for (mut obligation, stalled_on) in mem::take(&mut self.obligations.pending) { - let goal = obligation.as_goal(); - let delegate = <&SolverDelegate<'tcx>>::from(infcx); + let mut overflowed = false; + + self.obligations.pending.retain_mut(|(obligation, opt_stalled_on)| { + if overflowed { + return false; + } - let result = delegate.evaluate_root_goal(goal, obligation.cause.span, stalled_on); - self.inspect_evaluated_obligation(infcx, &obligation, &result); + // Common case: no inspector, still stalled; keep the obligation. This path is + // extremely hot in some cases; there can be thousands of pending obligations. + if !has_inspector + && let Some(stalled_on) = opt_stalled_on + && let Some(certainty) = delegate.goal_remains_stalled(stalled_on) + && matches!(certainty, Certainty::Maybe(_)) + { + return true; + } + + let result = delegate.evaluate_root_goal( + obligation.as_goal(), + obligation.cause.span, + opt_stalled_on.take(), + ); + Self::inspect_evaluated_obligation(infcx, &obligation, &result); let GoalEvaluation { goal, certainty, has_changed, stalled_on } = match result { Ok(result) => result, Err(NoSolution) => { errors.push(E::from_solver_error( infcx, - NextSolverError::TrueError(obligation), + NextSolverError::TrueError(obligation.clone()), )); - continue; + return false; } }; @@ -229,9 +247,11 @@ where obligation.recursion_depth += 1; if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) { - self.obligations.on_fulfillment_overflow(infcx); - // Only return true errors that we have accumulated while processing. - return errors; + // At this point we want to stop evaluating goals. We can't break out of + // `retain_mut`, so instead we set this flag which causes all other + // elements to be skipped. + overflowed = true; + return false; } else { any_changed = true; } @@ -253,11 +273,24 @@ where if infcx.in_hir_typeck && (obligation.has_non_region_infer() || obligation.has_free_regions()) { - infcx.push_hir_typeck_potentially_region_dependent_goal(obligation); + infcx.push_hir_typeck_potentially_region_dependent_goal( + obligation.clone(), + ); } + false + } + Certainty::Maybe(_) => { + // Update `opt_stalled_on` goal, for the next retain_mut, because we are + // running until a fixpoint. + *opt_stalled_on = stalled_on; + true } - Certainty::Maybe(_) => self.obligations.register(obligation, stalled_on), } + }); + if overflowed { + self.obligations.on_fulfillment_overflow(infcx); + // Only return true errors that we have accumulated while processing. + return errors; } if !any_changed { From 7089725ce269380f301eebb84ed3be534b43b0d7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Aug 2026 08:30:02 +1000 Subject: [PATCH 2/2] Inspect new solver obligations less often Don't call the inspector on the hot path when nothing has changed. This is a visible behaviour change, but as lcnr said: "There's no use in reinspecting a stalled goal as it hasn't changed since the last time" and "inspectors only exist for external tools". --- compiler/rustc_trait_selection/src/solve/fulfill.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index 375a6054bab8c..da596fe3b44b0 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -196,7 +196,6 @@ where assert_eq!(self.usable_in_snapshot, infcx.num_open_snapshots()); let mut errors = TraitErrors::NoErrors; let delegate = <&SolverDelegate<'tcx>>::from(infcx); - let has_inspector = infcx.obligation_inspector.get().is_some(); loop { let mut any_changed = false; let mut overflowed = false; @@ -206,10 +205,9 @@ where return false; } - // Common case: no inspector, still stalled; keep the obligation. This path is - // extremely hot in some cases; there can be thousands of pending obligations. - if !has_inspector - && let Some(stalled_on) = opt_stalled_on + // Common case: still stalled; keep the obligation. This path is extremely hot in + // some cases; there can be thousands of pending obligations. + if let Some(stalled_on) = opt_stalled_on && let Some(certainty) = delegate.goal_remains_stalled(stalled_on) && matches!(certainty, Certainty::Maybe(_)) {