Avoid quadratic growth of functions due to cleanups #31390
Merged
Conversation
This file contains 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
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
Awesome stuff @dotdash! 🌴 |
| exit_label.branch(bcx_out, prev_llbb); | ||
| prev_llbb = bcx_in.llbb; | ||
|
|
||
| scope.add_cached_early_exit(exit_label, prev_llbb); | ||
| let len = scope.cleanups.len(); |
There was a problem hiding this comment.
Nit: move this a little bit up and reuse len inside take(scope.cleanups.len() - skip).
|
Overall LGTM. |
dotdash
force-pushed
the
fix_quadratic_drop
branch
from
f4f57c0
to
d3c1264
Compare
|
Nits addressed. |
|
No test? |
dotdash
force-pushed
the
fix_quadratic_drop
branch
from
d3c1264
to
8923122
Compare
|
Test added. |
|
@bors r+ |
|
📌 Commit 8923122 has been approved by |
| // CHECK: call{{.*}}SomeUniqueName{{.*}}drop | ||
| // CHECK: call{{.*}}SomeUniqueName{{.*}}drop | ||
| // CHECK-NOT: call{{.*}}SomeUniqueName{{.*}}drop | ||
| // The next line checks for the 0 that ends the function definition |
There was a problem hiding this comment.
The 0 is a typo'd }, right?
If a new cleanup is added to a cleanup scope, the cached exits for that scope are cleared, so all previous cleanups have to be translated again. In the worst case this means that we get N distinct landing pads where the last one has N cleanups, then N-1 and so on. As new cleanups are to be executed before older ones, we can instead cache the number of already translated cleanups in addition to the block that contains them, and then only translate new ones, if any and then jump to the cached ones, getting away with linear growth instead. For the crate in rust-lang#31381 this reduces the compile time for an optimized build from >20 minutes (I cancelled the build at that point) to about 11 seconds. Testing a few crates that come with rustc show compile time improvements somewhere between 1 and 8%. The "big" winner being rustc_platform_intrinsics which features code similar to that in rust-lang#31381. Fixes rust-lang#31381
dotdash
force-pushed
the
fix_quadratic_drop
branch
from
8923122
to
8c0f4f5
Compare
|
@bors r=nagisa Typo fixed. |
|
📌 Commit 8c0f4f5 has been approved by |
bluss
added
the
relnotes
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Feb 5, 2016
If a new cleanup is added to a cleanup scope, the cached exits for that scope are cleared, so all previous cleanups have to be translated again. In the worst case this means that we get N distinct landing pads where the last one has N cleanups, then N-1 and so on. As new cleanups are to be executed before older ones, we can instead cache the number of already translated cleanups in addition to the block that contains them, and then only translate new ones, if any and then jump to the cached ones, getting away with linear growth instead. For the crate in rust-lang#31381 this reduces the compile time for an optimized build from >20 minutes (I cancelled the build at that point) to about 11 seconds. Testing a few crates that come with rustc show compile time improvements somewhere between 1 and 8%. The "big" winner being rustc_platform_intrinsics which features code similar to that in rust-lang#31381. Fixes rust-lang#31381
bors
added a commit
that referenced
this pull request
Feb 5, 2016
If a new cleanup is added to a cleanup scope, the cached exits for that scope are cleared, so all previous cleanups have to be translated again. In the worst case this means that we get N distinct landing pads where the last one has N cleanups, then N-1 and so on. As new cleanups are to be executed before older ones, we can instead cache the number of already translated cleanups in addition to the block that contains them, and then only translate new ones, if any and then jump to the cached ones, getting away with linear growth instead. For the crate in #31381 this reduces the compile time for an optimized build from >20 minutes (I cancelled the build at that point) to about 11 seconds. Testing a few crates that come with rustc show compile time improvements somewhere between 1 and 8%. The "big" winner being rustc_platform_intrinsics which features code similar to that in #31381. Fixes #31381
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a new cleanup is added to a cleanup scope, the cached exits for that
scope are cleared, so all previous cleanups have to be translated
again. In the worst case this means that we get N distinct landing pads
where the last one has N cleanups, then N-1 and so on.
As new cleanups are to be executed before older ones, we can instead
cache the number of already translated cleanups in addition to the
block that contains them, and then only translate new ones, if any and
then jump to the cached ones, getting away with linear growth instead.
For the crate in #31381 this reduces the compile time for an optimized
build from >20 minutes (I cancelled the build at that point) to about 11
seconds. Testing a few crates that come with rustc show compile time
improvements somewhere between 1 and 8%. The "big" winner being
rustc_platform_intrinsics which features code similar to that in #31381.
Fixes #31381