save subobligations in the projection cache #43546
Conversation
|
I've "minimized" a test case which at least doesn't depend on the futures crate, but I wouldn't necessarily call it small yet. |
|
I've managed to make @alexcrichton 's example a bit smaller (118 lines -> 60 lines). |
arielb1
added
the
beta-nominated
|
beta-nominating because this is a regression - I shuffled things around and made evaluation easier to reach. r=me with a test. |
|
☔ The latest upstream changes (presumably #43543) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
issue-43132
branch
from
6b3f4c7
to
2574f31
Compare
|
@bor: r=arielb1 |
|
@bors: r=arielb1 |
|
📌 Commit 2574f31 has been approved by |
save subobligations in the projection cache The projection cache explicitly chose not to "preserve" subobligations for projections, since the fulfillment context ought to have been doing so. But for the trait evaluation scheme that causes problems. This PR reproduces subobligations. This has the potential to slow down compilation, but minimal investigation suggests it does not do so. One hesitation about this PR: I could not find a way to make a standalone test case for #43132 (but admittedly I did not try very hard). Fixes #43132. r? @arielb1
|
☀️ Test successful - status-appveyor, status-travis |
nikomatsakis
added
the
beta-accepted
|
I'm marking this as beta-accepted -- it's a small and harmless patch. |
|
The author of #43132 complains that this caused an exponential case in his code. I can believe that - need to investigate. |
|
It looks like at least one test also regressed on perf.rust-lang.org |
alexcrichton
removed
the
beta-nominated
This effectively reverts rust-lang#43546 as it seems that it does affect performance more than the PR has anticipated. Follow-up changes from rust-lang#44269 are also reverted. This also removes the deduplication code from rust-lang#48296 as duplications were primarily coming from rust-lang#43546 and after removing that code it probably doesn't worth paying the cost of using a hash map.
The projection cache explicitly chose not to "preserve" subobligations for projections, since the fulfillment context ought to have been doing so. But for the trait evaluation scheme that causes problems. This PR reproduces subobligations. This has the potential to slow down compilation, but minimal investigation suggests it does not do so.
One hesitation about this PR: I could not find a way to make a standalone test case for #43132 (but admittedly I did not try very hard).
Fixes #43132.
r? @arielb1