Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate identical region constraints in new solver #112442

Conversation

compiler-errors
Copy link
Member

the new solver doesn't track whether we've already proven a goal like the fulfillment context's obligation forest does, so we may be instantiating a canonical response (and specifically, its nested region obligations) quite a few times.

This may lead to exponentially gathering up identical region constraints for things like auto traits, so let's deduplicate region constraints when in compute_external_query_constraints.

r? @lcnr

@rustbot 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 9, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 9, 2023

Some changes occurred to the core trait solver

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

@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 9, 2023

the new solver doesn't track whether we've already proven a goal like the fulfillment context's obligation forest does

What do you mean by this, the new solver has both the provisional and global cache

@compiler-errors
Copy link
Member Author

What do you mean by this, the new solver has both the provisional and global cache

Yeah, but in the new solver these results are canonicalized, and as a consequence we always end up instantiating the region constraints from these goals every time we evaluate it in the new solver even if we're not doing any deep goal solving or anything. In contrast, if you register a goal in an old-style obligation forest that's already been processed, it won't re-register it at all and we never end up instantiating its region obligations once again:

if self.done_cache.contains(&cache_key) {
debug!("register_obligation_at: ignoring already done obligation: {:?}", obligation);
return Ok(());
}

In the example, I'm essentially creating a case where we end up instantiating a pathologically large number of outlives constraints that wouldn't be registered in the old solver because of the way it doesn't reprocess (and therefore re-register region constraints for) a goal that it already knows it has solved.

@lcnr
Copy link
Contributor

lcnr commented Jun 9, 2023

In the example, I'm essentially creating a case where we end up instantiating a pathologically large number of outlives constraints that wouldn't be registered in the old solver because of the way it doesn't reprocess (and therefore re-register region constraints for) a goal that it already knows it has solved.

I think this is also kinda inevitable unless you have a fulfillment context which adds nested obligations into the root

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 9, 2023

📌 Commit d5e25d4 has been approved by lcnr

It is now in the queue for this repository.

@bors 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 Jun 9, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 9, 2023
…icate-region-constraints, r=lcnr

Deduplicate identical region constraints in new solver

the new solver doesn't track whether we've already proven a goal like the fulfillment context's obligation forest does, so we may be instantiating a canonical response (and specifically, its nested region obligations) quite a few times.

This may lead to exponentially gathering up identical region constraints for things like auto traits, so let's deduplicate region constraints when in `compute_external_query_constraints`.

r? `@lcnr`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 9, 2023
…llaumeGomez

Rollup of 3 pull requests

Successful merges:

 - rust-lang#112260 (Improve document of `unsafe_code` lint)
 - rust-lang#112429 ([rustdoc] List matching impls on type aliases)
 - rust-lang#112442 (Deduplicate identical region constraints in new solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4ef7257 into rust-lang:master Jun 9, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 9, 2023
@compiler-errors compiler-errors deleted the next-solver-deduplicate-region-constraints branch August 11, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants