`-Znext-solver` caching #128828

lcnr · 2024-08-08T14:34:47Z

This PR has two major changes while also fixing multiple issues found via fuzzing.

The main optimization is the ability to not discard provisional cache entries when popping the highest cycle head the entry depends on. This fixes the hang in Fuchsia with -Znext-solver=coherence.

It also bails if the result of a fixpoint iteration is ambiguous, even without reaching a fixpoint. This is necessary to avoid exponential blowup if a coinductive cycle results in ambiguity, e.g. due to unknowable candidates in coherence.

Updating stack entries pretty much exclusively happens lazily now, so fn check_invariants ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it.

For a general overview, see the in-process update of the relevant rustc-dev-guide chapter: https://hackmd.io/1ALkSjKlSCyQG-dVb_PUHw

r? @compiler-errors

rustbot · 2024-08-08T14:34:58Z

Some changes occurred to the core trait solver

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

lcnr · 2024-08-08T14:40:26Z

@bors try

bors · 2024-08-08T14:41:39Z

⌛ Trying commit 2de329c with merge fed0c12...

[try-me] `-Znext-solver` caching try-job: x86_64-fuchsia r? `@compiler-errors`

bors · 2024-08-08T16:29:07Z

☀️ Try build successful - checks-actions
Build commit: fed0c12 (fed0c120f38587c2d4d8e939ec312391feca77c6)

lcnr · 2024-08-09T10:44:23Z

@bors try

[try-me] `-Znext-solver` caching try-job: x86_64-fuchsia r? `@compiler-errors`

bors · 2024-08-09T10:45:36Z

⌛ Trying commit ad501b4 with merge f3e5a95...

bors · 2024-08-09T12:34:27Z

☀️ Try build successful - checks-actions
Build commit: f3e5a95 (f3e5a95dfcefbf5dcbdcf0bd252d6da661e9cb64)

lcnr · 2024-08-12T08:28:59Z

@bors try

doing so requires overwriting global cache entries and generally adds significant complexity to the solver. This is also only ever done for root goals, so it feels easier to wrap the `evaluate_canonical_goal` in an ordinary query if necessary.

this allows us to only sometimes disable the global cache.

this makes it easier to maintain and modify going forward. There may be a small performance cost as we now need to access the provisional cache *and* walk through the stack to detect cycles. However, the provisional cache should be mostly empty and the stack should only have a few elements so the performance impact is likely minimal. Given the complexity of the search graph maintainability trumps linear performance improvements.

lcnr · 2024-08-12T08:34:08Z

@bors try

`-Znext-solver` caching Updating stack entries pretty much exclusively happens lazily now, so `fn check_invariants` ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it. try-job: x86_64-fuchsia r? `@compiler-errors`

bors · 2024-08-12T08:35:33Z

⌛ Trying commit 21f7a72 with merge cb8409d...

[try-run] Search graph 11 test rust-lang#128828 except that it enables `-Znext-solver=coherence` 😅 try-job: x86_64-fuchsia r? `@ghost`

compiler-errors

One nit

compiler-errors · 2024-08-13T14:37:45Z

compiler/rustc_type_ir/src/search_graph/mod.rs

+            let step_kind = Self::step_kind(cx, parent.input);
+            parent.nested_goals.extend_from_child(step_kind, nested_goals);
+            if !nested_goals.is_empty() {
+                parent.nested_goals.insert(parent.input, UsageKind::Single(PathKind::Coinductive))


What is this for?

good catch. deserves a comment

We have to disable a cache entry if it's the root of a cycle and another cycle participant
is already on the stack.

ABA cycle

BA ❌

As the root of a cycle can change its result, we have to also be careful with global cache
entries which depend on a cycle, even if they aren't directly involved.

ABCB cycle

CB

C cycle

A ❌

Something definitely is not clicking. I don't understand how either that, or the comment you left, translates to:

if !nested_goals.is_empty() { parent.nested_goals.insert(parent.input, UsageKind::Single(PathKind::Coinductive))

I'm missing something, probably a lot of things.

did you also see the inline comment i've added?

edit: the actual relevant example is https://gist.github.com/lcnr/291c3a7e1491ea33c1333c83cb594ca0#provisional-cache-entries

compiler/rustc_type_ir/src/search_graph/mod.rs

compiler-errors · 2024-08-13T16:32:36Z

@bors r+

bors · 2024-08-13T16:32:39Z

📌 Commit 0aa17a4 has been approved by compiler-errors

It is now in the queue for this repository.

…rrors `-Znext-solver` caching This PR has two major changes while also fixing multiple issues found via fuzzing. The main optimization is the ability to not discard provisional cache entries when popping the highest cycle head the entry depends on. This fixes the hang in Fuchsia with `-Znext-solver=coherence`. It also bails if the result of a fixpoint iteration is ambiguous, even without reaching a fixpoint. This is necessary to avoid exponential blowup if a coinductive cycle results in ambiguity, e.g. due to unknowable candidates in coherence. Updating stack entries pretty much exclusively happens lazily now, so `fn check_invariants` ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it. For a general overview, see the in-process update of the relevant rustc-dev-guide chapter: https://hackmd.io/1ALkSjKlSCyQG-dVb_PUHw r? `@compiler-errors`

…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#127857 (Allow to customize `// TODO:` comment for deprecated safe autofix) - rust-lang#128410 (Migrate `remap-path-prefix-dwarf` `run-make` test to rmake) - rust-lang#128828 (`-Znext-solver` caching) - rust-lang#128873 (Add windows-targets crate to std's sysroot) - rust-lang#129034 (Add `#[must_use]` attribute to `Coroutine` trait) - rust-lang#129049 (compiletest: Don't panic on unknown JSON-like output lines) - rust-lang#129050 (Emit a warning instead of an error if `--generate-link-to-definition` is used with other output formats than HTML) r? `@ghost` `@rustbot` modify labels: rollup

…rrors `-Znext-solver` caching This PR has two major changes while also fixing multiple issues found via fuzzing. The main optimization is the ability to not discard provisional cache entries when popping the highest cycle head the entry depends on. This fixes the hang in Fuchsia with `-Znext-solver=coherence`. It also bails if the result of a fixpoint iteration is ambiguous, even without reaching a fixpoint. This is necessary to avoid exponential blowup if a coinductive cycle results in ambiguity, e.g. due to unknowable candidates in coherence. Updating stack entries pretty much exclusively happens lazily now, so `fn check_invariants` ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it. For a general overview, see the in-process update of the relevant rustc-dev-guide chapter: https://hackmd.io/1ALkSjKlSCyQG-dVb_PUHw r? ``@compiler-errors``

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#128828 (`-Znext-solver` caching) - rust-lang#128954 (Explicitly specify type parameter on FromResidual for Option and ControlFlow.) - rust-lang#129054 (Subtree update of `rust-analyzer`) - rust-lang#129059 (Record the correct target type when coercing fn items/closures to pointers) - rust-lang#129071 (Port `run-make/sysroot-crates-are-unstable` to rmake) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 6 pull requests Successful merges: - rust-lang#128570 (Stabilize `asm_const`) - rust-lang#128828 (`-Znext-solver` caching) - rust-lang#128954 (Explicitly specify type parameter on FromResidual for Option and ControlFlow.) - rust-lang#129059 (Record the correct target type when coercing fn items/closures to pointers) - rust-lang#129071 (Port `run-make/sysroot-crates-are-unstable` to rmake) - rust-lang#129088 (Make the rendered html doc for rustc better) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#128828 - lcnr:search-graph-11, r=compiler-errors `-Znext-solver` caching This PR has two major changes while also fixing multiple issues found via fuzzing. The main optimization is the ability to not discard provisional cache entries when popping the highest cycle head the entry depends on. This fixes the hang in Fuchsia with `-Znext-solver=coherence`. It also bails if the result of a fixpoint iteration is ambiguous, even without reaching a fixpoint. This is necessary to avoid exponential blowup if a coinductive cycle results in ambiguity, e.g. due to unknowable candidates in coherence. Updating stack entries pretty much exclusively happens lazily now, so `fn check_invariants` ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it. For a general overview, see the in-process update of the relevant rustc-dev-guide chapter: https://hackmd.io/1ALkSjKlSCyQG-dVb_PUHw r? ```@compiler-errors```

rustbot assigned compiler-errors Aug 8, 2024

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 8, 2024

This comment has been minimized.

Sign in to view

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2024

Auto merge of rust-lang#128828 - lcnr:search-graph-11, r=<try>

fed0c12

[try-me] `-Znext-solver` caching try-job: x86_64-fuchsia r? `@compiler-errors`

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2024

Auto merge of rust-lang#128828 - lcnr:search-graph-11, r=<try>

f3e5a95

[try-me] `-Znext-solver` caching try-job: x86_64-fuchsia r? `@compiler-errors`

compiler-errors added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2024

lcnr added 7 commits August 12, 2024 10:33

expand fuzzing support

51338ca

this allows us to only sometimes disable the global cache.

simplify match + move debug! call

e87157b

tracing: debug to trace

9308401

move behavior out of shared fn

e83eacd

merge impl blocks

0c75c08

lcnr force-pushed the search-graph-11 branch from 73f0823 to 21f7a72 Compare August 12, 2024 08:33

lcnr changed the title ~~[try-me] -Znext-solver caching~~ -Znext-solver caching Aug 12, 2024

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the search-graph-11 branch from 036d2de to a634209 Compare August 13, 2024 10:40

compiler-errors mentioned this pull request Aug 13, 2024

refactor search graph even more! #128115

Closed

compiler-errors reviewed Aug 13, 2024

View reviewed changes

lcnr force-pushed the search-graph-11 branch 2 times, most recently from e354c73 to 5798271 Compare August 13, 2024 15:24

implement a performant and fuzzed solver cache

0aa17a4

lcnr force-pushed the search-graph-11 branch from 5798271 to 0aa17a4 Compare August 13, 2024 15:33

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 13, 2024

GuillaumeGomez mentioned this pull request Aug 13, 2024

Rollup of 7 pull requests #129057

Closed

matthiaskrgr mentioned this pull request Aug 14, 2024

Rollup of 5 pull requests #129082

Closed

jieyouxu mentioned this pull request Aug 14, 2024

Rollup of 6 pull requests #129092

Merged

bors merged commit 59ad2ae into rust-lang:master Aug 14, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Aug 14, 2024

lcnr mentioned this pull request Aug 15, 2024

do not rerun trait solver cycles on ambiguity #125981

Closed

lcnr deleted the search-graph-11 branch August 15, 2024 10:08

lcnr mentioned this pull request Aug 15, 2024

stabilize -Znext-solver=coherence #121848

Merged

This was referenced Sep 2, 2024

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

Open

privacy: normalize associated types before visiting #126076

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`-Znext-solver` caching #128828

`-Znext-solver` caching #128828

lcnr commented Aug 8, 2024 •

edited

Loading

rustbot commented Aug 8, 2024

lcnr commented Aug 8, 2024

This comment has been minimized.

bors commented Aug 8, 2024

bors commented Aug 8, 2024

lcnr commented Aug 9, 2024

bors commented Aug 9, 2024

bors commented Aug 9, 2024

lcnr commented Aug 12, 2024

lcnr commented Aug 12, 2024

bors commented Aug 12, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors left a comment

compiler-errors Aug 13, 2024

lcnr Aug 13, 2024

lcnr Aug 13, 2024

compiler-errors Aug 13, 2024

compiler-errors Aug 13, 2024

lcnr Aug 13, 2024 •

edited

Loading

compiler-errors commented Aug 13, 2024

bors commented Aug 13, 2024

-Znext-solver caching #128828

-Znext-solver caching #128828

Conversation

lcnr commented Aug 8, 2024 • edited Loading

rustbot commented Aug 8, 2024

lcnr commented Aug 8, 2024

This comment has been minimized.

bors commented Aug 8, 2024

bors commented Aug 8, 2024

lcnr commented Aug 9, 2024

bors commented Aug 9, 2024

bors commented Aug 9, 2024

lcnr commented Aug 12, 2024

lcnr commented Aug 12, 2024

bors commented Aug 12, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors Aug 13, 2024

Choose a reason for hiding this comment

lcnr Aug 13, 2024

Choose a reason for hiding this comment

lcnr Aug 13, 2024

Choose a reason for hiding this comment

compiler-errors Aug 13, 2024

Choose a reason for hiding this comment

compiler-errors Aug 13, 2024

Choose a reason for hiding this comment

lcnr Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Aug 13, 2024

bors commented Aug 13, 2024

`-Znext-solver` caching #128828

`-Znext-solver` caching #128828

lcnr commented Aug 8, 2024 •

edited

Loading

lcnr Aug 13, 2024 •

edited

Loading