Optimize `try_evaluate_obligations` by nnethercote · Pull Request #160479 · rust-lang/rust

nnethercote · 2026-08-04T02:32:28Z

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).

r? @lcnr
cc @jdonszelmann @WaffleLapkin

rustbot · 2026-08-04T02:32:31Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

nnethercote · 2026-08-04T02:36:36Z

On my Linux box this reduced the wall-time for a check full build of nacl-0.5.3 and ijson-0.1.6 by ~50%, and of nvml-wrapper-sys-0.9.1 by ~25%.

Local instruction count results for all the new-solver benchmarks (including those three, which I have added locally but aren't on CI):

nnethercote · 2026-08-04T03:21:48Z

@bors try @rust-timer queue

…try> Optimize `try_evaluate_obligations`

rust-bors · 2026-08-04T05:27:06Z

☀️ Try build successful (CI)
Build commit: 443ade9 (443ade9a155ac75fdadee81c20ae7a9b8576e119)
Base parent: c9ff496 (c9ff496891c278ad660bc0ab85c1f0b72059464a)

rust-timer · 2026-08-04T06:09:28Z

Finished benchmarking commit (443ade9): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never rustc-perf
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-5.1%, -0.2%]	19
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -0.3%, secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.4%	[4.4%, 4.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-1.2%, -0.4%]	16
Improvements ✅ (secondary)	-0.9%	[-2.6%, -0.4%]	26
All ❌✅ (primary)	-0.3%	[-1.2%, 4.4%]	17

Cycles

Results (primary -1.1%, secondary -2.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.4%, 0.7%]	4
Regressions ❌ (secondary)	0.9%	[0.5%, 1.3%]	5
Improvements ✅ (primary)	-1.8%	[-7.0%, -0.5%]	10
Improvements ✅ (secondary)	-3.1%	[-11.1%, -0.4%]	19
All ❌✅ (primary)	-1.1%	[-7.0%, 0.7%]	14

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.838s -> 490.758s (0.19%)
Artifact size: 390.28 MiB -> 390.21 MiB (-0.02%)

nnethercote · 2026-08-04T06:17:58Z

The perf results on CI are a delight:

icount reductions are a bit bigger than I saw locally across all new-solver benchmarks, e.g. -5.12% for wg-grammar on CI vs. -3.96% locally (for a check full build).
cycles and wall-time both show a 10% reduction for a check full build of wg-grammar, and 1-6% across a majority of the other new-solver benchmarks.

jdonszelmann · 2026-08-04T12:02:37Z

r? me

lcnr · 2026-08-04T12:25:09Z

-                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


can throw that check out. There's no use in reinspecting a stalled goal as it hasn't changed since the last time :> inspectors only exist for external tools 😁

View changes since the review

Ok. I have added a second commit for this.

nnethercote · 2026-08-05T08:03:13Z

I have three follow-up changes that will improve these cases more.

jdonszelmann · 2026-08-05T11:19:16Z

+                    return true;
+                }
+
+                let result = delegate.evaluate_root_goal(


note: the first thing evaluate_root_goal does is to check again goal_remains_stalled.

View changes since the review

Yes, there is some repeated work: rerunning_stalled_goal_may_make_progress can be called twice. It's idempotent and the wasted work doesn't matter. Here's some Cachegrind output:

. // Common case: no inspector, still stalled; keep the obligation. This path is . // extremely hot in some cases; there can be thousands of pending obligations. 114,082,625 (0.8%) if !has_inspector 228,165,250 (1.6%) && let Some(stalled_on) = opt_stalled_on . && let Some(certainty) = delegate.goal_remains_stalled(stalled_on) . && matches!(certainty, Certainty::Maybe(_)) . { . return true; . } . 622,248 (0.0%) let result = delegate.evaluate_root_goal( . obligation.as_goal(), 155,562 (0.0%) obligation.cause.span, . opt_stalled_on.take(), . );

The common case is more than 100x hotter than what follows. It would be possible to refactor evaluate_root_goal to avoid this wasted work, but evaluate_root_goal has four call sites and they would all need some changes and I don't think it's worthwhile.

nnethercote · 2026-08-05T13:03:53Z

I updated the code. I added the suggested comments. The other suggestions above were about inefficiencies on the cold path and I don't think anything needs changing there. Good to go?

jdonszelmann · 2026-08-06T07:13:11Z

I love it, lgtm!

@bors r+

lcnr · 2026-08-06T08:02:07Z

+                // 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(_))


we should change stalled_on to contain a MaybeCause instead of a Certainty. It is always Certainty::Maybe

View changes since the review

As a follow-up?

@lcnr

…donszelmann 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). r? @lcnr cc @jdonszelmann @WaffleLapkin

rust-bors · 2026-08-06T17:17:37Z

💔 Test for 84aa78d failed: CI

JonathanBrouwer · 2026-08-06T17:28:25Z

hmmm
@bors retry

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).

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".

rustbot · 2026-08-07T09:35:38Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

nnethercote · 2026-08-07T09:36:17Z

@bors r=jdonszelmann

rust-bors · 2026-08-07T09:36:20Z

📌 Commit 7089725 has been approved by jdonszelmann

It is now in the queue for this repository.

rust-bors · 2026-08-07T13:05:18Z

☀️ Test successful - CI
Approved by: jdonszelmann
Duration: 3h 4m 19s
Pushing 65bcac4 to main...

github-actions · 2026-08-07T13:08:14Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing ae45457 (parent) -> 65bcac4 (this PR)

Test differences

Show 2 test diffs

2 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 65bcac45b3d8a8b2126e5cc844cf6fff5795d32a --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-msvc-2: 1h 43m -> 2h 27m (+41.9%)
x86_64-msvc-ext1: 1h 40m -> 2h 17m (+36.5%)
dist-x86_64-netbsd: 1h 28m -> 56m 32s (-36.0%)
i686-gnu-nopt-1: 1h 36m -> 2h 8m (+34.3%)
x86_64-gnu-llvm-22-3: 1h 44m -> 1h 9m (-33.5%)
dist-powerpc-linux: 1h 9m -> 1h 30m (+31.0%)
dist-various-1: 36m 16s -> 44m 41s (+23.2%)
x86_64-gnu-debug: 1h 32m -> 1h 53m (+22.9%)
dist-ohos-armv7: 1h 11m -> 55m 39s (-22.7%)
dist-x86_64-linux-alt: 2h 15m -> 1h 45m (-22.5%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-08-07T14:40:34Z

Finished benchmarking commit (65bcac4): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.6%	[-5.1%, -0.2%]	20
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.7%	[0.4%, 1.1%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-11.9%, -0.4%]	7
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 2.1%, secondary -2.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	1.0%	[0.4%, 1.9%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-10.4%, -0.4%]	14
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 461.168s -> 458.664s (-0.54%)
Artifact size: 398.56 MiB -> 398.60 MiB (0.01%)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 4, 2026

rustbot assigned lcnr Aug 4, 2026

nnethercote force-pushed the opt-try_evaluate_obligations branch from fb5d400 to c0a9583 Compare August 4, 2026 02:34

nnethercote mentioned this pull request Aug 4, 2026

Manage pending better in try_evaluate_obligations #160418

Closed

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 4, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Aug 4, 2026

Auto merge of #160479 - nnethercote:opt-try_evaluate_obligations, r=<…

443ade9

…try> Optimize `try_evaluate_obligations`

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 4, 2026

rustbot assigned jdonszelmann and unassigned lcnr Aug 4, 2026

lcnr reviewed Aug 4, 2026

View reviewed changes

nnethercote mentioned this pull request Aug 4, 2026

performance regression with -Znext-solver=globally #159933

Open

jdonszelmann reviewed Aug 5, 2026

View reviewed changes

Comment thread compiler/rustc_trait_selection/src/solve/fulfill.rs

Comment thread compiler/rustc_trait_selection/src/solve/fulfill.rs

Comment thread compiler/rustc_trait_selection/src/solve/fulfill.rs

nnethercote force-pushed the opt-try_evaluate_obligations branch from 95c355e to 4b0dd6f Compare August 5, 2026 13:01

This comment has been minimized.

Sign in to view

lcnr reviewed Aug 6, 2026

View reviewed changes

lcnr mentioned this pull request Aug 6, 2026

Stabilize the next-generation trait solver rust-lang/goals#113

Open

8 tasks

This comment has been minimized.

Sign in to view

rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 6, 2026

This comment has been minimized.

Sign in to view

nnethercote added 2 commits August 7, 2026 19:25

nnethercote force-pushed the opt-try_evaluate_obligations branch from 4b0dd6f to 7089725 Compare August 7, 2026 09:35

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 7, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2026

rust-bors Bot merged commit 65bcac4 into rust-lang:main Aug 7, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Aug 7, 2026

rust-bors Bot mentioned this pull request Aug 7, 2026

[perf] tiny fulfillment loop #160296

Draft

Uh oh!

Uh oh!

Conversation

nnethercote commented Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 4, 2026

Uh oh!

nnethercote commented Aug 4, 2026

Uh oh!

nnethercote commented Aug 4, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Aug 4, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Aug 4, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

nnethercote commented Aug 4, 2026

Uh oh!

jdonszelmann commented Aug 4, 2026

Uh oh!

lcnr Aug 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nnethercote commented Aug 5, 2026

Uh oh!

jdonszelmann Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

nnethercote commented Aug 5, 2026

Uh oh!

jdonszelmann commented Aug 6, 2026

Uh oh!

lcnr Aug 6, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rust-bors Bot commented Aug 6, 2026

Uh oh!

JonathanBrouwer commented Aug 6, 2026

Uh oh!

This comment has been minimized.

rustbot commented Aug 7, 2026

Uh oh!

nnethercote commented Aug 7, 2026

Uh oh!

rust-bors Bot commented Aug 7, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Aug 7, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 7, 2026

Test differences

Job duration changes

Uh oh!

rust-timer commented Aug 7, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

nnethercote commented Aug 4, 2026 •

edited by rustbot

Loading

lcnr Aug 4, 2026 •

edited by rustbot

Loading

jdonszelmann Aug 5, 2026 •

edited by rustbot

Loading

lcnr Aug 6, 2026 •

edited by rustbot

Loading