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

Try to evalate obligation in fulfillment when we have no inference vars #86748

Closed
wants to merge 2 commits into from

Conversation

Aaron1011
Copy link
Member

Previously, we required that the predicate be 'global', which also
ruled out type parameters. As long as we don't have any inference
variables (meaning that that the fulfillment will never need to
constrain any inferene variables), we can use the evaluate_obligation
query.

Previously, we required that the predicate be 'global', which also
ruled out type parameters. As long as we don't have any inference
variables (meaning that that the fulfillment will never need to
constrain any inferene variables), we can use the `evaluate_obligation`
query.
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2021
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 30, 2021
@bors
Copy link
Contributor

bors commented Jun 30, 2021

⌛ Trying commit 121faca with merge 175a9cc1cf3acd27382e67c38be0fd0c341382d3...

@bors
Copy link
Contributor

bors commented Jun 30, 2021

☀️ Try build successful - checks-actions
Build commit: 175a9cc1cf3acd27382e67c38be0fd0c341382d3 (175a9cc1cf3acd27382e67c38be0fd0c341382d3)

@rust-timer
Copy link
Collaborator

Queued 175a9cc1cf3acd27382e67c38be0fd0c341382d3 with parent 5d34076, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (175a9cc1cf3acd27382e67c38be0fd0c341382d3): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

  • Very large regression in instruction counts (up to 17.6% on full builds of wf-projection-stress-65510-check)
  • Moderate improvement in instruction counts (up to -1.6% on full builds of unify-linearly-check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 30, 2021
@Aaron1011
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 30, 2021
@bors
Copy link
Contributor

bors commented Jun 30, 2021

⌛ Trying commit 74ee70c with merge 0d390589eed781c1e3aa3f8f045bf7f759912646...

@bors
Copy link
Contributor

bors commented Jun 30, 2021

☀️ Try build successful - checks-actions
Build commit: 0d390589eed781c1e3aa3f8f045bf7f759912646 (0d390589eed781c1e3aa3f8f045bf7f759912646)

@rust-timer
Copy link
Collaborator

Queued 0d390589eed781c1e3aa3f8f045bf7f759912646 with parent 868c702, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (0d390589eed781c1e3aa3f8f045bf7f759912646): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

  • Very large regression in instruction counts (up to 17.6% on full builds of wf-projection-stress-65510-check)
  • Moderate improvement in instruction counts (up to -1.6% on full builds of unify-linearly-check)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 1, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 9, 2021
@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
@jackh726
Copy link
Member

jackh726 commented Sep 9, 2021

@Aaron1011 was this an alternative to #85868?

@Aaron1011
Copy link
Member Author

This was an attempt at speeding up that PR - by itself, this PR doesn't fix any bugs. However, it appears that this approach actually hurts performance.

@Aaron1011 Aaron1011 closed this Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf-regression Performance regression. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants