rustc: Preallocate when building the dep graph #44586
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
This commit alters the `query` function in the dep graph module to preallocate memory using `with_capacity` instead of relying on automatic growth. Discovered in rust-lang#44576 it was found that for the syntex_syntax clean incremental benchmark the peak memory usage was found when the dep graph was being saved, particularly the `DepGraphQuery` data structure itself. PRs like rust-lang#44142 which add more queries end up just making this much larger! I didn't see an immediately obvious way to reduce the size of the `DepGraphQuery` object, but it turns out that `with_capacity` helps quite a bit! Locally 831 MB was used [before] this commit, and 770 MB is in use at the peak of the compiler [after] this commit. That's a nice 7.5% improvement! This won't quite make up for the losses in rust-lang#44142 but I figured it's a good start. [before]: https://gist.github.com/alexcrichton/2d2b9c7a65503761925c5a0bcfeb0d1e [before]: https://gist.github.com/alexcrichton/6da51f2a6184bfb81694cc44f06deb5b
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors r+ rollup Nice find. Hopefully though |
|
📌 Commit a7817dd has been approved by |
|
Heh oops, probably shouldn't have tried so hard to investigate.. |
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Sep 15, 2017
…elwoerister rustc: Preallocate when building the dep graph This commit alters the `query` function in the dep graph module to preallocate memory using `with_capacity` instead of relying on automatic growth. Discovered in rust-lang#44576 it was found that for the syntex_syntax clean incremental benchmark the peak memory usage was found when the dep graph was being saved, particularly the `DepGraphQuery` data structure itself. PRs like rust-lang#44142 which add more queries end up just making this much larger! I didn't see an immediately obvious way to reduce the size of the `DepGraphQuery` object, but it turns out that `with_capacity` helps quite a bit! Locally 831 MB was used [before] this commit, and 770 MB is in use at the peak of the compiler [after] this commit. That's a nice 7.5% improvement! This won't quite make up for the losses in rust-lang#44142 but I figured it's a good start. [before]: https://gist.github.com/alexcrichton/2d2b9c7a65503761925c5a0bcfeb0d1e [before]: https://gist.github.com/alexcrichton/6da51f2a6184bfb81694cc44f06deb5b
alexcrichton
added
the
S-waiting-on-bors
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Sep 16, 2017
…elwoerister rustc: Preallocate when building the dep graph This commit alters the `query` function in the dep graph module to preallocate memory using `with_capacity` instead of relying on automatic growth. Discovered in rust-lang#44576 it was found that for the syntex_syntax clean incremental benchmark the peak memory usage was found when the dep graph was being saved, particularly the `DepGraphQuery` data structure itself. PRs like rust-lang#44142 which add more queries end up just making this much larger! I didn't see an immediately obvious way to reduce the size of the `DepGraphQuery` object, but it turns out that `with_capacity` helps quite a bit! Locally 831 MB was used [before] this commit, and 770 MB is in use at the peak of the compiler [after] this commit. That's a nice 7.5% improvement! This won't quite make up for the losses in rust-lang#44142 but I figured it's a good start. [before]: https://gist.github.com/alexcrichton/2d2b9c7a65503761925c5a0bcfeb0d1e [before]: https://gist.github.com/alexcrichton/6da51f2a6184bfb81694cc44f06deb5b
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this pull request
Sep 16, 2017
…elwoerister rustc: Preallocate when building the dep graph This commit alters the `query` function in the dep graph module to preallocate memory using `with_capacity` instead of relying on automatic growth. Discovered in rust-lang#44576 it was found that for the syntex_syntax clean incremental benchmark the peak memory usage was found when the dep graph was being saved, particularly the `DepGraphQuery` data structure itself. PRs like rust-lang#44142 which add more queries end up just making this much larger! I didn't see an immediately obvious way to reduce the size of the `DepGraphQuery` object, but it turns out that `with_capacity` helps quite a bit! Locally 831 MB was used [before] this commit, and 770 MB is in use at the peak of the compiler [after] this commit. That's a nice 7.5% improvement! This won't quite make up for the losses in rust-lang#44142 but I figured it's a good start. [before]: https://gist.github.com/alexcrichton/2d2b9c7a65503761925c5a0bcfeb0d1e [before]: https://gist.github.com/alexcrichton/6da51f2a6184bfb81694cc44f06deb5b
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Sep 17, 2017
…elwoerister rustc: Preallocate when building the dep graph This commit alters the `query` function in the dep graph module to preallocate memory using `with_capacity` instead of relying on automatic growth. Discovered in rust-lang#44576 it was found that for the syntex_syntax clean incremental benchmark the peak memory usage was found when the dep graph was being saved, particularly the `DepGraphQuery` data structure itself. PRs like rust-lang#44142 which add more queries end up just making this much larger! I didn't see an immediately obvious way to reduce the size of the `DepGraphQuery` object, but it turns out that `with_capacity` helps quite a bit! Locally 831 MB was used [before] this commit, and 770 MB is in use at the peak of the compiler [after] this commit. That's a nice 7.5% improvement! This won't quite make up for the losses in rust-lang#44142 but I figured it's a good start. [before]: https://gist.github.com/alexcrichton/2d2b9c7a65503761925c5a0bcfeb0d1e [before]: https://gist.github.com/alexcrichton/6da51f2a6184bfb81694cc44f06deb5b
bors
added a commit
that referenced
this pull request
Sep 17, 2017
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.
This commit alters the
queryfunction in the dep graph module to preallocatememory using
with_capacityinstead of relying on automatic growth. Discoveredin #44576 it was found that for the syntex_syntax clean incremental benchmark
the peak memory usage was found when the dep graph was being saved, particularly
the
DepGraphQuerydata structure itself. PRs like #44142 which add morequeries end up just making this much larger!
I didn't see an immediately obvious way to reduce the size of the
DepGraphQueryobject, but it turns out thatwith_capacityhelps quite a bit!Locally 831 MB was used before this commit, and 770 MB is in use at the peak
of the compiler [after] this commit. That's a nice 7.5% improvement! This won't
quite make up for the losses in #44142 but I figured it's a good start.