Don't rerun goals if none of their vars have changed #141500

compiler-errors · 2025-05-24T12:38:03Z

Alternative to #141488. I'm pretty sure that we don't need to re-run the goal at all if the inputs don't change... 🤔

rustbot · 2025-05-24T12:38:07Z

Some changes occurred to the core trait solver

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

lqd · 2025-05-24T15:30:21Z

perf perf PERF (to the tune of teeth teeth TEETH) @bors try @rust-timer queue

[PERF] Don't rerun goals if none of their vars have changed r? `@ghost` Alternative to #141488. I'm pretty sure that we don't need to re-run the goal at all if the inputs don't change... 🤔

bors · 2025-05-24T15:31:31Z

⌛ Trying commit 2d718c1 with merge 4c02404...

bors · 2025-05-24T17:26:36Z

☀️ Try build successful - checks-actions
Build commit: 4c02404 (4c02404f6a3e21d721602e21942b9ee0e3175b14)

rust-timer · 2025-05-24T20:37:20Z

Finished benchmarking commit (4c02404): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.6%	[0.3%, 0.9%]	6
Improvements ✅ (primary)	-0.2%	[-0.3%, -0.1%]	13
Improvements ✅ (secondary)	-1.8%	[-17.6%, -0.2%]	23
All ❌✅ (primary)	-0.2%	[-0.3%, -0.1%]	13

Max RSS (memory usage)

Results (primary 0.8%, secondary 5.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.9%	[1.8%, 2.0%]	2
Regressions ❌ (secondary)	6.7%	[3.2%, 9.6%]	7
Improvements ✅ (primary)	-1.2%	[-1.2%, -1.2%]	1
Improvements ✅ (secondary)	-2.8%	[-2.8%, -2.8%]	1
All ❌✅ (primary)	0.8%	[-1.2%, 2.0%]	3

Cycles

Results (secondary -5.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.9%	[0.9%, 0.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-8.6%	[-11.7%, -5.6%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 777.23s -> 776.477s (-0.10%)
Artifact size: 366.32 MiB -> 366.38 MiB (0.02%)

compiler-errors · 2025-05-25T10:19:24Z

@bors try @rust-timer queue

bors · 2025-05-25T10:20:33Z

⌛ Trying commit ca34794 with merge 7e9d4c6...

[PERF] Don't rerun goals if none of their vars have changed r? `@ghost` Alternative to #141488. I'm pretty sure that we don't need to re-run the goal at all if the inputs don't change... 🤔

bors · 2025-05-25T12:17:11Z

☀️ Try build successful - checks-actions
Build commit: 7e9d4c6 (7e9d4c67ad1d4d4bcd517d72759241344f1854e5)

rust-timer · 2025-05-25T13:46:39Z

Finished benchmarking commit (7e9d4c6): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.2%, 0.4%]	5
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.1%]	5
Improvements ✅ (secondary)	-6.1%	[-18.1%, -2.2%]	6
All ❌✅ (primary)	-0.1%	[-0.2%, -0.1%]	5

Max RSS (memory usage)

Results (primary 0.8%, secondary -3.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.6%, 2.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-3.3%	[-4.2%, -2.0%]	14
All ❌✅ (primary)	0.8%	[-1.3%, 2.0%]	3

Cycles

Results (secondary -0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.5%	[1.2%, 4.7%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.2%	[-12.3%, -6.1%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.592s -> 776.167s (0.07%)
Artifact size: 366.33 MiB -> 366.31 MiB (-0.01%)

compiler-errors · 2025-05-25T10:19:57Z

compiler/rustc_infer/src/infer/opaque_types/table.rs

@@ -24,6 +24,12 @@ pub struct OpaqueTypeStorageEntries {
    duplicate_entries: usize,
 }

+impl rustc_type_ir::inherent::OpaqueTypeStorageEntries for OpaqueTypeStorageEntries {
+    fn needs_reevaluation(self, canonicalized: usize) -> bool {


ew but also i dont think we need a better API For now.

compiler-errors · 2025-05-25T10:20:04Z

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs

@@ -53,10 +53,10 @@ where
 {
    /// Canonicalizes the goal remembering the original values
    /// for each bound variable.
-    pub(super) fn canonicalize_goal<T: TypeFoldable<I>>(
+    pub(super) fn canonicalize_goal(


didnt need to be this generic :>

compiler-errors · 2025-05-25T10:20:20Z

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs

+                    (
+                        Certainty::Yes,
+                        NestedNormalizationGoals(
+                            goals.into_iter().map(|(s, g, _)| (s, g)).collect(),


we could actually canonicalize these keys i think, which may prevent a rerun in the parent query

compiler-errors · 2025-05-25T12:30:49Z

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

@@ -660,6 +733,8 @@ where
                // looking at the "has changed" return from evaluate_goal,
                // because we expect the `unconstrained_rhs` part of the predicate
                // to have changed -- that means we actually normalized successfully!
+                // FIXME: Do we need to eagerly resolve here? Or should we check
+                // if the cache key has any changed vars?
                let with_resolved_vars = self.resolve_vars_if_possible(goal);


this resolve call feels strange -- we should probably be checking if any of the vars changed at all?

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

compiler-errors · 2025-05-25T14:11:11Z

Minor improvement in syn. Some of the non new-solver scenarios are back to neutral; not sure what to make of those, but whatever. I think this is fine to be reviewed now.

lcnr

👍

naming-wise I am not totally happy yet 🤔 CanonicalGoalCacheKey seems somewhat... misleading/unhelpful

GoalStalledOn maybe? also, add a comment to that struct :3

lcnr · 2025-05-25T14:10:19Z

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

    ) -> (
-        Result<(HasChanged, Certainty), NoSolution>,
+        Result<(HasChanged, Certainty, CanonicalGoalCacheKey<Self::Interner>), NoSolution>,


can you move (HasChanged, Certainty, CanonicalGoalCacheKey<Self::Interner>) into a struct.

Unlike the one below, this one actually does clean up a lot 👍

compiler/rustc_type_ir/src/solve/mod.rs

lcnr · 2025-05-25T14:25:02Z

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

@@ -115,7 +116,7 @@ where

    pub(super) search_graph: &'a mut SearchGraph<D>,

-    nested_goals: Vec<(GoalSource, Goal<I, I::Predicate>)>,
+    nested_goals: Vec<(GoalSource, Goal<I, I::Predicate>, Option<CanonicalGoalCacheKey<I>>)>,


same with this tuple

For a single use? I don't really feel like that's really beneficial. Notably, this isn't the same tuple of (Goal, Option<CacheKey>) that's stored in the fulfillment context.

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

compiler-errors · 2025-05-25T19:00:03Z

I introduced a new GoalEvaluation<'tcx> struct that holds the response + stalled vars from the evaluate_goal_* functions, and renamed the "cache key" struct (whose name stopped being descriptive when I changed the approach) to GoalStalledOn<'tcx>.

Threw another random commit into the PR (5893b58) b/c I didn't want to open a totally separate PR for just a one liner.

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

… new solver is more complete Just a totally unrelated nitpick I'm folding into the PR, since it's code I'd like for us to prune when the new solver lands.

compiler/rustc_trait_selection/src/solve/fulfill.rs

compiler-errors · 2025-05-26T08:49:30Z

@bors r=lcnr

bors · 2025-05-26T08:49:33Z

📌 Commit f0bcb0e has been approved by lcnr

It is now in the queue for this repository.

compiler-errors · 2025-05-26T08:49:34Z

@bors rollup=never

compiler-errors · 2025-05-26T10:12:08Z

@bors r=lcnr

bors · 2025-05-26T10:12:12Z

📌 Commit e2215a8 has been approved by lcnr

It is now in the queue for this repository.

bors · 2025-05-26T10:12:12Z

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

add additional `TypeFlags` fast paths Some crates, e.g. `diesel`, have items with a lot of where-clauses (more than 150). In these cases checking the `TypeFlags` of the whole `param_env` can be very beneficial. This adds `fn fold_clauses` to mirror the existing `fn visit_clauses` and then uses this in folders which fold `ParamEnv`s. Split out from #141451, depends on #141500. r? `@compiler-errors`

bors · 2025-05-26T17:02:45Z

⌛ Testing commit e2215a8 with merge 40d2563...

bors · 2025-05-26T20:10:27Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 40d2563 to master...

github-actions · 2025-05-26T20:13:37Z

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 9c0bcb5 (parent) -> 40d2563 (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 40d2563ea200f9327a8cb8b99a0fb82f75a7365c --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

dist-x86_64-apple: 11397.7s -> 8025.7s (-29.6%)
aarch64-gnu-debug: 4901.5s -> 4063.4s (-17.1%)
aarch64-gnu: 6806.1s -> 7655.1s (12.5%)
x86_64-apple-2: 5442.5s -> 4830.9s (-11.2%)
aarch64-apple: 4627.1s -> 4125.6s (-10.8%)
x86_64-msvc-ext1: 7401.4s -> 6954.9s (-6.0%)
dist-x86_64-msvc-alt: 7797.8s -> 7331.5s (-6.0%)
x86_64-msvc-1: 8126.4s -> 8587.5s (5.7%)
dist-x86_64-musl: 7411.5s -> 7756.6s (4.7%)
dist-aarch64-apple: 6152.9s -> 5875.9s (-4.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 · 2025-05-26T22:24:27Z

Finished benchmarking commit (40d2563): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.1%]	2
Improvements ✅ (secondary)	-4.1%	[-17.9%, -0.3%]	9
All ❌✅ (primary)	-0.1%	[-0.2%, -0.1%]	2

Max RSS (memory usage)

Results (primary -0.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.0%	[1.0%, 1.0%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.5%	[-1.6%, -1.4%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-1.6%, 1.0%]	3

Cycles

Results (secondary -9.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-9.1%	[-12.3%, -6.0%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 777.056s -> 778.13s (0.14%)
Artifact size: 366.33 MiB -> 366.42 MiB (0.02%)

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 May 24, 2025

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 May 24, 2025

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 24, 2025

compiler-errors mentioned this pull request May 25, 2025

Cache subsequent re-runs of goals #141488

Closed

compiler-errors force-pushed the rerun-cache-2 branch from 2d718c1 to ca34794 Compare May 25, 2025 10:17

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 May 25, 2025

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 May 25, 2025

compiler-errors commented May 25, 2025

View reviewed changes

compiler-errors changed the title ~~[PERF] Don't rerun goals if none of their vars have changed~~ Don't rerun goals if none of their vars have changed May 25, 2025

lcnr reviewed May 25, 2025

View reviewed changes

compiler-errors force-pushed the rerun-cache-2 branch from fec620e to 21ba021 Compare May 25, 2025 18:53

compiler-errors commented May 25, 2025

View reviewed changes

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs Show resolved Hide resolved

compiler-errors force-pushed the rerun-cache-2 branch from 5893b58 to 24a77c5 Compare May 25, 2025 20:07

lcnr self-assigned this May 26, 2025

lcnr reviewed May 26, 2025

View reviewed changes

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs Outdated Show resolved Hide resolved

Don't retry in pred_known_to_hold_modulo_regions in new solver, since…

4d783c3

… new solver is more complete Just a totally unrelated nitpick I'm folding into the PR, since it's code I'd like for us to prune when the new solver lands.

lcnr reviewed May 26, 2025

View reviewed changes

compiler/rustc_trait_selection/src/solve/fulfill.rs Show resolved Hide resolved

compiler-errors force-pushed the rerun-cache-2 branch from 24a77c5 to f0bcb0e Compare May 26, 2025 08:46

bors 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-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2025

Don't rerun goals if none of its vars have changed

e2215a8

compiler-errors force-pushed the rerun-cache-2 branch from f0bcb0e to e2215a8 Compare May 26, 2025 10:11

lcnr mentioned this pull request May 26, 2025

add additional TypeFlags fast paths #141581

Merged

bors mentioned this pull request May 26, 2025

Remove an unnecessary use of Box::into_inner. #141599

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label May 26, 2025

bors merged commit 40d2563 into rust-lang:master May 26, 2025
8 checks passed

rustbot added this to the 1.89.0 milestone May 26, 2025

rustbot removed the perf-regression Performance regression. label May 26, 2025

lcnr mentioned this pull request May 29, 2025

Next-generation trait solver rust-lang/rust-project-goals#113

Open

4 tasks

Don't rerun goals if none of their vars have changed #141500

Don't rerun goals if none of their vars have changed #141500

Uh oh!

Conversation

compiler-errors commented May 24, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 24, 2025

Uh oh!

lqd commented May 24, 2025

Uh oh!

This comment has been minimized.

bors commented May 24, 2025

Uh oh!

bors commented May 24, 2025

Uh oh!

This comment has been minimized.

rust-timer commented May 24, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

Uh oh!

compiler-errors commented May 25, 2025

Uh oh!

This comment has been minimized.

bors commented May 25, 2025

Uh oh!

bors commented May 25, 2025

Uh oh!

This comment has been minimized.

rust-timer commented May 25, 2025

Overall result: ❌✅ regressions and improvements - please read the text below

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

compiler-errors commented May 25, 2025

Uh oh!

lcnr left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

compiler-errors commented May 25, 2025

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

compiler-errors commented May 26, 2025

Uh oh!

compiler-errors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

bors commented May 26, 2025

Uh oh!

Uh oh!

github-actions bot commented May 26, 2025

Test differences

Job duration changes

Uh oh!

compiler-errors commented May 24, 2025 •

edited by rustbot

Loading