search graph: improve rebasing and add forced ambiguity support #143054

lcnr · 2025-06-26T11:53:59Z

This slightly strengthens rebasing and actually checks for the property we want to maintain. There are two additional optimizations we can and should do here:

we should be able to just always rebase if cycle heads already have a provisional result from a previous iteration
we currently only apply provisional cache entries if the path_to_entry matches exactly. We should be able to extend this e.g. if you have an entry for B in ABA where the path BA is coinductive, then we can use this entry even if the current path from A to B is inductive.

I've also added support for PathKind::ForcedAmbiguity which always forced the initial provisional result to be ambiguous. A am using this for cycles involving negative reasons, which is currently only used by the fuzzer in https://github.com/lcnr/search_graph_fuzz. Consider the following setup: A goal A which only holds if B does not hold, and B which only holds if A does not hold.

A only holds if B does not hold, results in X
- B only holds if A does not hold, results in !X
  - A cycle, provisional result X
B only holds if A does not hold, results in X
- A only holds if B does not hold, results in !X
  - B cycle, provisional result X

With negative reasoning, the result of cycle participants depends on their position in the cycle. This means using cache entries while other entries are on the stack/have been popped is wrong. It's also generally just kinda iffy. By always forcing the initial provisional result of such cycles to be ambiguity, we can avoid this, as "not maybe" is just "maybe" again.

Rust kind of has negative reasoning due to incompleteness, consider the following setup:

T::Foo eq u32
- normalize T::Foo
  - via impl -> u32
  - via param_env -> T
    - nested goals...

T::Foo eq u32 holds exactly if the nested goals of the param_env candidate do not hold, as preferring that candidate over the impl causes the alias-relate to fail. This means the current provisional cache may cause us to ignore param_env preference in rare cases. This is not unsound and I don't care about it, as we already have this behavior when rerunning on changed fixpoint results:

T: Trait
- via impl ok
- via env
  - T: Trait non-productive cycle
result OK, rerun changed provisional result
T: Trait
- via impl ok
- via env
  - T: Trait using the provisional result, can be thought of as recursively expanding the proof tree
    - via impl ok
    - via env <don't care>
prefer the env candidate, reached fixpoint

One could imaging changing ParamEnv candidates or the impl shadowing check to use PathKind::ForcedAmbiguity to make the search graph less observable instead of only using it for fuzzing. However, incomplete candidate preference isn't really negative reasoning and doing this is a breaking change rust-lang/trait-system-refactor-initiative#114

r? @compiler-errors

rustbot · 2025-06-26T11:54:07Z

Some changes occurred to the core trait solver

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

lcnr · 2025-06-26T13:03:01Z

@bors2 try @rust-timer queue

rust-bors · 2025-06-26T13:03:05Z

⌛ Trying commit ff3254b with merge a07e3de…

To cancel the try build, run the command @bors2 try cancel.

search graph: improve rebasing and add forced ambiguity support This slightly strengthens rebasing and actually checks for the property we want to maintain. Consider the following very minor changes in benchmarks: | | dropped entries old | new | compute_goal old | new | |---|----|----|---|----| | diesel | 20412 | 4533 | 5144336 | 5128470 | | nalgebra | 2570 | 112 | 779257 | 778571 | | `./x.py b --stage 2`¹ | 17234 | 7680 |14242763 | 142375634 | ¹ with the alias-relate fast-path to avoid the hang in rayon 😅 There are two additional optimizations we can and should do here: - we should be able to just always rebase if cycle heads already have a provisional result from a previous iteration - we currently only apply provisional cache entries if the `path_to_entry` matches exactly. We should be able to extend this e.g. if you have an entry for `B` in `ABA` where the path `BA` is coinductive, then we can use this entry even if the current path from `A` to `B` is inductive. --- Finally, I've added support for `PathKind::ForcedAmbiguity` which always forced the initial provisional result to be ambiguous. A am using this for cycles involving negative reasons, which is currently only used by the fuzzer in https://github.com/lcnr/search_graph_fuzz. Consider the following setup: A goal `A` which only holds if `B` does not hold, and `B` which only holds if `A` does not hold. - A only holds if B does not hold, results in X - B only holds if A does not hold, results in !X - A cycle, provisional result X - B only holds if A does not hold, results in X - A only holds if B does not hold, results in !X - B cycle, provisional result X With negative reasoning, the result of cycle participants depends on their position in the cycle. This means using cache entries while other entries are on the stack/have been popped is wrong. It's also generally just kinda iffy. By always forcing the initial provisional result of such cycles to be ambiguity, we can avoid this, as "not maybe" is just "maybe" again. Rust kind of has negative reasoning due to incompleteness, consider the following setup: - `T::Foo eq u32` - normalize `T::Foo` - via impl -> `u32` - via param_env -> `T` - nested goals... `T::Foo eq u32` holds exactly if the nested goals of the `param_env` candidate do not hold, as preferring that candidate over the impl causes the alias-relate to fail. This means the current provisional cache may cause us to ignore `param_env` preference in rare cases. This is not unsound and I don't care about it, as we already have this behavior when rerunning on changed fixpoint results: - `T: Trait` - via impl ok - via env - `T: Trait` non-productive cycle - result OK, rerun changed provisional result - `T: Trait` - via impl ok - via env - `T: Trait` using the provisional result, can be thought of as recursively expanding the proof tree - via impl ok - via env <don't care> - prefer the env candidate, reached fixpoint --- One could imaging changing `ParamEnv` candidates or the impl shadowing check to use `PathKind::ForcedAmbiguity` to make the search graph less observable instead of only using it for fuzzing. However, incomplete candidate preference isn't really negative reasoning and doing this is a breaking change rust-lang/trait-system-refactor-initiative#114 r? `@compiler-errors`

rust-bors · 2025-06-26T15:33:34Z

☀️ Try build successful (CI)
Build commit: a07e3de (a07e3de733ec6c6ec4f2988cbd06487ff09094c4, parent: 8f21a5c92ea55c348c275a1bc4fedbdf181e0d64)

bors · 2025-06-26T21:06:56Z

☔ The latest upstream changes (presumably #142774) made this pull request unmergeable. Please resolve the merge conflicts.

rust-timer · 2025-06-27T08:20:04Z

Finished benchmarking commit (a07e3de): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@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)	-0.3%	[-0.5%, -0.2%]	11
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 5.1%, secondary 2.7%)

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

	mean	range	count
Regressions ❌ (primary)	5.1%	[2.6%, 9.4%]	3
Regressions ❌ (secondary)	2.7%	[2.6%, 2.8%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	5.1%	[2.6%, 9.4%]	3

Cycles

Results (secondary -8.0%)

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
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-8.0%	[-9.8%, -3.3%]	7
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 691.142s -> 688.194s (-0.43%)
Artifact size: 372.05 MiB -> 372.03 MiB (-0.01%)

compiler/rustc_type_ir/src/search_graph/mod.rs

BoxyUwU · 2025-08-11T14:03:30Z

@bors r+ rollup=never

BoxyUwU · 2025-08-11T14:03:35Z

r? BoxyUwU

bors · 2025-08-11T14:03:38Z

📌 Commit 733aea5 has been approved by BoxyUwU

It is now in the queue for this repository.

bors · 2025-08-11T22:51:10Z

⌛ Testing commit 733aea5 with merge a153133...

bors · 2025-08-12T01:54:40Z

☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing a153133 to master...

github-actions · 2025-08-12T01:57:32Z

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 1ebbd87 (parent) -> a153133 (this PR)

Test differences

Show 3 test diffs

3 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 a1531335fe2807715fff569904d99602022643a7 --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-aarch64-apple: 9397.3s -> 5315.7s (-43.4%)
dist-apple-various: 4335.1s -> 6065.6s (39.9%)
x86_64-apple-1: 7129.5s -> 9683.6s (35.8%)
aarch64-apple: 5489.5s -> 7238.5s (31.9%)
x86_64-apple-2: 4549.4s -> 5998.2s (31.8%)
dist-aarch64-linux: 8580.7s -> 5956.3s (-30.6%)
x86_64-rust-for-linux: 2662.9s -> 3044.2s (14.3%)
x86_64-gnu-llvm-19-1: 3279.2s -> 3698.2s (12.8%)
i686-gnu-2: 5438.7s -> 6087.6s (11.9%)
dist-x86_64-apple: 11465.9s -> 10209.7s (-11.0%)

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-08-12T06:59:33Z

Finished benchmarking commit (a153133): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (primary -1.4%, secondary 3.2%)

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

	mean	range	count
Regressions ❌ (primary)	2.7%	[2.7%, 2.7%]	1
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-2.8%	[-3.3%, -2.3%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.4%	[-3.3%, 2.7%]	4

Binary size

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

Bootstrap: 465.929s -> 465.439s (-0.11%)
Artifact size: 377.31 MiB -> 377.34 MiB (0.01%)

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? `@compiler-errors` `@BoxyUwU`

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ``@compiler-errors`` ``@BoxyUwU``

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ```@compiler-errors``` ```@BoxyUwU```

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ````@compiler-errors```` ````@BoxyUwU````

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? `````@compiler-errors````` `````@BoxyUwU`````

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ``````@compiler-errors`````` ``````@BoxyUwU``````

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ```````@compiler-errors``````` ```````@BoxyUwU```````

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? ````````@compiler-errors```````` ````````@BoxyUwU````````

…s, r=BoxyUwU search graph: lazily update parent goals Based on top of rust-lang#143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? `````````@compiler-errors````````` `````````@BoxyUwU`````````

Rollup merge of #144955 - lcnr:lazily-update-non-parent-goals, r=BoxyUwU search graph: lazily update parent goals Based on top of #143054. In the search graph only the last entry is actually mutable and all other entries get lazily mutated when popping child goals. This simplifies a bunch of possible future optimizations: - We can try evaluating nested goals and entirely ignore discard their evaluation by simply not calling `fn update_parent_goal` - Because we only lazily update, tracking the "impact" of a nested goal is easy. The necessary information *has to be* integrated in the `StackEntry` of the current goal, as there is otherwise no way to influence its parents. This makes it easier to avoid rerunning cycle heads if they have only been used in candidates which don't impact the final result of a goal. r? `````````@compiler-errors````````` `````````@BoxyUwU`````````

rustbot assigned compiler-errors Jun 26, 2025

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 Jun 26, 2025

This comment has been minimized.

Sign in to view

lcnr force-pushed the search_graph-3 branch from bc43f11 to ff3254b Compare June 26, 2025 12:51

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 Jun 26, 2025

This comment has been minimized.

Sign in to view

lcnr force-pushed the search_graph-3 branch from ff3254b to 9c5a110 Compare June 27, 2025 07:55

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 27, 2025

lcnr force-pushed the search_graph-3 branch 2 times, most recently from 7f567d7 to 544afbb Compare June 27, 2025 09:45

lcnr commented Jul 1, 2025

View reviewed changes

compiler/rustc_type_ir/src/search_graph/mod.rs Show resolved Hide resolved

lcnr mentioned this pull request Jul 8, 2025

vibe: negative reasoning and solver cycles rust-lang/trait-system-refactor-initiative#122

Open

lcnr force-pushed the search_graph-3 branch 2 times, most recently from efafedb to ba96f4a Compare August 5, 2025 11:57

This comment has been minimized.

Sign in to view

lcnr force-pushed the search_graph-3 branch from ba96f4a to 578e15d Compare August 5, 2025 12:05

lcnr mentioned this pull request Aug 5, 2025

search graph: lazily update parent goals #144955

Merged

significantly improve provisional cache rebasing

733aea5

lcnr force-pushed the search_graph-3 branch from 578e15d to 733aea5 Compare August 11, 2025 13:51

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 Aug 11, 2025

rustbot assigned BoxyUwU and unassigned compiler-errors Aug 11, 2025

bors mentioned this pull request Aug 12, 2025

Fix parallel rustc not being reproducible due to unstable sorts of items #144722

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 12, 2025

bors merged commit a153133 into rust-lang:master Aug 12, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Aug 12, 2025

bors mentioned this pull request Aug 12, 2025

ignore head usages from ignored candidates #144991

Merged

lcnr deleted the search_graph-3 branch August 12, 2025 07:52

search graph: improve rebasing and add forced ambiguity support #143054

search graph: improve rebasing and add forced ambiguity support #143054

Conversation

lcnr commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 26, 2025

Uh oh!

This comment has been minimized.

lcnr commented Jun 26, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 26, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 26, 2025

Uh oh!

This comment has been minimized.

bors commented Jun 26, 2025

Uh oh!

rust-timer commented Jun 27, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

Uh oh!

This comment has been minimized.

BoxyUwU commented Aug 11, 2025

Uh oh!

BoxyUwU commented Aug 11, 2025

Uh oh!

bors commented Aug 11, 2025

Uh oh!

bors commented Aug 11, 2025

Uh oh!

bors commented Aug 12, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 12, 2025

Test differences

Job duration changes

Uh oh!

rust-timer commented Aug 12, 2025

Overall result: no relevant changes - no action needed

Uh oh!

Uh oh!

lcnr commented Jun 26, 2025 •

edited

Loading