Skip to content

rustc: Preallocate when building the dep graph #44586

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 17, 2017

Conversation

alexcrichton
Copy link
Member

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 #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 #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 #44142 but I figured it's a good start.

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
Copy link
Member Author

cc @michaelwoerister

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@michaelwoerister
Copy link
Member

@bors r+ rollup

Nice find. Hopefully though DepGraphQuery will completely go away soon.

@bors
Copy link
Collaborator

bors commented Sep 15, 2017

📌 Commit a7817dd has been approved by michaelwoerister

@alexcrichton
Copy link
Member Author

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 alexcrichton added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 15, 2017
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
@bors bors merged commit a7817dd into rust-lang:master Sep 17, 2017
@alexcrichton alexcrichton deleted the smaller-query branch September 17, 2017 16:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants