-
Notifications
You must be signed in to change notification settings - Fork 13k
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
add a per-param-env cache to impls_bound
#37152
Conversation
@arielb1 r=me once you finish confirming there do not seem to be adverse performance consequences. |
There used to be only a global cache, which led to uncached calls to trait selection when there were type parameters. I'm running a check that there are no adverse performance effects. Fixes rust-lang#37106 (drop elaboration times are now ~half of borrow checking, so might still be worthy of optimization, but not critical).
@bors r=pnkfelix |
📌 Commit a61d85b has been approved by |
I tried this out on Serde and compile time dropped from 10 seconds using 1.14.0-nightly (a3bc191 2016-10-10) to 5.3 seconds using this commit. Thanks for taking care of this so quickly! |
add a per-param-env cache to `impls_bound` There used to be only a global cache, which led to uncached calls to trait selection when there were type parameters. This causes a 20% decrease in borrow-checking time and an overall 0.5% performance increase during bootstrapping (as borrow-checking tends to be a tiny part of compilation time). Fixes #37106 (drop elaboration times are now ~half of borrow checking, so might still be worthy of optimization, but not critical). r? @pnkfelix
Seems harmless enough. I guess that this is marked for beta-backport because the elaboration code is relatively new? cc @rust-lang/compiler -- should we backport? compile-time only. |
SGTM as this seems to noticeably push back on a compile times regression. |
Marking as beta-accepted. |
There used to be only a global cache, which led to uncached calls to
trait selection when there were type parameters.
This causes a 20% decrease in borrow-checking time and an overall 0.5% performance increase during bootstrapping (as borrow-checking tends to be a tiny part of compilation time).
Fixes #37106 (drop elaboration times are now ~half of borrow checking,
so might still be worthy of optimization, but not critical).
r? @pnkfelix