Make compare_predicate_entailment no longer a query by compiler-errors · Pull Request #101615 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Make compare_predicate_entailment so it's no longer a query (again), and splits out the new logic (that equates the return types to infer RPITITs) into its own query. This means that this new query (now called collect_trait_impl_trait_tys) is no longer executed for non-RPITIT cases.
This should improve perf (#101224 (comment)), though in practice we see that these some crates remain from the primary regressions list on the original report... They are all <= 0.43% regression and seemingly only on the incr-full scenario for all of them.
I am at a loss for what might be causing this regression other than what I fixed here, since we don't introduce much new non-RPITIT logic except for some def_kind query calls in some places, for example, like projection. Maybe that's it?
Originally this PR was opened to test enabling cache_on_disk (62164aaaa11) but that didn't turn out to be very useful (#101615 (comment)), so that led me to just split the query (and rename the PR).
Relevant to the compiler team, which will review and decide on the PR/issue.
label
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
⌛ Trying commit 112458bd85c9e8d2692233dce436912c88f35521 with merge 89b3f54f5de96e6fd11b7d992d9b64f335997aa5...
Status: This is awaiting some action (such as code changes or more information) from the author.
label
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
label
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
⌛ Trying commit d2cd1b7c1d284dbf502e09f666c64150ce2abad2 with merge ee778fd79f1ba1246047b1f652c6ef0a135c1e49...
This comment was marked as outdated.
This comment has been minimized.
☀️ Try build successful - checks-actions
Build commit: ee778fd79f1ba1246047b1f652c6ef0a135c1e49 (ee778fd79f1ba1246047b1f652c6ef0a135c1e49)
Finished benchmarking commit (ee778fd79f1ba1246047b1f652c6ef0a135c1e49): comparison URL.
Overall result: ❌✅ regressions and improvements - no action needed
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.
@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 0.2% | [0.2%, 0.3%] | 3 |
| Improvements ✅ (primary) | -0.3% | [-0.3%, -0.3%] | 1 |
| Improvements ✅ (secondary) | -0.4% | [-0.4%, -0.4%] | 1 |
| All ❌✅ (primary) | -0.3% | [-0.3%, -0.3%] | 1 |
Max RSS (memory usage)
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 0.8% | [0.8%, 0.8%] | 1 |
| Improvements ✅ (primary) | -1.5% | [-2.0%, -0.9%] | 4 |
| Improvements ✅ (secondary) | -2.0% | [-2.9%, -1.3%] | 10 |
| All ❌✅ (primary) | -1.5% | [-2.0%, -0.9%] | 4 |
Cycles
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | -2.2% | [-2.2%, -2.2%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Footnotes
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
⌛ Trying commit 2e1da580dd11d4fdf8763638f771e41dfcfebd58 with merge 46ec702e9dd4258388a00180e4de0d629dfdad33...
Finished benchmarking commit (0531a017ad453e9f1bc2ab9193c17642e1ece6f2): comparison URL.
Overall result: ✅ improvements - no action needed
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.
@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | -0.5% | [-0.6%, -0.3%] | 7 |
| Improvements ✅ (secondary) | -0.4% | [-0.4%, -0.4%] | 5 |
| All ❌✅ (primary) | -0.5% | [-0.6%, -0.3%] | 7 |
Max RSS (memory usage)
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 1.5% | [1.1%, 1.8%] | 2 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | 2.8% | [2.7%, 2.8%] | 2 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | -2.2% | [-2.2%, -2.2%] | 1 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | 1.1% | [-2.2%, 2.8%] | 3 |
Footnotes
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
⌛ Trying commit bec079d with merge 58c417521d7df5a75c6abc60cbba95b4e4b49f2b...
cache Make compare_predicates_and_trait_impl_trait_tyscompare_predicate_entailment no longer a query
☀️ Try build successful - checks-actions
Build commit: 58c417521d7df5a75c6abc60cbba95b4e4b49f2b (58c417521d7df5a75c6abc60cbba95b4e4b49f2b)
Finished benchmarking commit (58c417521d7df5a75c6abc60cbba95b4e4b49f2b): comparison URL.
Overall result: ✅ improvements - no action needed
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.
@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | -0.4% | [-0.5%, -0.4%] | 6 |
| Improvements ✅ (secondary) | -0.4% | [-0.6%, -0.3%] | 5 |
| All ❌✅ (primary) | -0.4% | [-0.5%, -0.4%] | 6 |
Max RSS (memory usage)
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 2.2% | [2.2%, 2.2%] | 1 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | -1.2% | [-1.2%, -1.2%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
This benchmark run did not return any relevant results for this metric.
Footnotes
📌 Commit bec079d has been approved by oli-obk
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
label
Finished benchmarking commit (1ce5198): comparison URL.
Overall result: ✅ improvements - no action needed
@rustbot label: -perf-regression
Instruction count
This is a highly reliable metric that was used to determine the overall result at the top of this comment.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | - | - | 0 |
| Improvements ✅ (primary) | -0.4% | [-0.5%, -0.4%] | 6 |
| Improvements ✅ (secondary) | -0.4% | [-0.5%, -0.3%] | 5 |
| All ❌✅ (primary) | -0.4% | [-0.5%, -0.4%] | 6 |
Max RSS (memory usage)
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 3.0% | [2.7%, 3.3%] | 2 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | -2.6% | [-2.6%, -2.6%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Cycles
Results
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.
| mean1 | range | count2 | |
|---|---|---|---|
| Regressions ❌ (primary) | 2.7% | [1.7%, 3.8%] | 2 |
| Regressions ❌ (secondary) | 3.3% | [2.8%, 3.7%] | 2 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | 2.7% | [1.7%, 3.8%] | 2 |