Avoid quadratic growth of functions due to cleanups#31390
Merged
bors merged 1 commit intorust-lang:masterfrom Feb 5, 2016
Merged
Avoid quadratic growth of functions due to cleanups#31390bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
Contributor
|
(rust_highfive has picked a reviewer for you, use r? to override) |
Contributor
|
Awesome stuff @dotdash! 🌴 |
src/librustc_trans/trans/cleanup.rs
Outdated
Member
There was a problem hiding this comment.
Nit: move this a little bit up and reuse len inside take(scope.cleanups.len() - skip).
Member
|
Overall LGTM. |
dotdash
force-pushed
the
fix_quadratic_drop
branch
from
f4f57c0 to
d3c1264
Compare
Contributor
Author
|
Nits addressed. |
Contributor
|
No test? |
dotdash
force-pushed
the
fix_quadratic_drop
branch
from
d3c1264 to
8923122
Compare
Contributor
Author
|
Test added. |
Member
|
@bors r+ |
Collaborator
|
📌 Commit 8923122 has been approved by |
src/test/codegen/drop.rs
Outdated
Contributor
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
Contributor
Author
|
@bors r=nagisa Typo fixed. |
Collaborator
|
📌 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
Collaborator
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
Collaborator
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
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