Up to 8% RSS regression because of #44142 #44576
Labels
regression-from-stable-to-nightly
Performance or correctness regression from stable to nightly.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
arielb1
added
regression-from-stable-to-nightly
|
Interestingly if you widen the view a bit you'll see:
May be worth looking into #44275 as well? |
|
Specifically before/after #44142, massif snapshots -- I've only included the peak snapshot in the output there. |
|
Looks like basically all of the memory is here, aka we have a bigger dep graph. Turns out adding more queries makes a bigger dep graph :) |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this issue
Sep 15, 2017
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
|
I cashed in some easy savings in #44586, but it doesn't actually fix the "regression", but we likely shouldn't call it a regression as it also fixed a number of would-be bugs! |
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
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 a commit
to alexcrichton/rust
that referenced
this issue
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 issue
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 issue
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
|
Looks like we've recovered the losses and this particular peak may also go away soon, so closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
http://perf.rust-lang.org/?start=2017-08-15T00%3A00%3A00%2B00%3A00&end=2017-09-14T00%3A00%3A00%2B00%3A00&crates=%5B%22futures-rs-test-all%22%2C%22helloworld%22%2C%22html5ever-2016-08-25%22%2C%22hyper.0.5.0%22%2C%22inflate-0.1.0%22%2C%22issue-20936-deep-vector%22%2C%22issue-32062-equality-relations-complexity%22%2C%22issue-32278-big-array-of-strings%22%2C%22issue-43572-unused-uses%22%2C%22jld-day15-parser%22%2C%22piston-image-0.10.3%22%2C%22regex-0.1.80%40010-baseline%22%2C%22regex-0.1.80%40020-incr-from-scratch%22%2C%22regex-0.1.80%40030-compile_one%22%2C%22regex-0.1.80%40040-is_valid_cap_letter%22%2C%22regex-0.1.80%40050-expand%22%2C%22regex-0.1.80%40060-Compiler-new%22%2C%22regex-0.1.80%40070-BYTE_FREQUENCIES%22%2C%22regex-0.1.80%40080-SparseSet%22%2C%22regex-0.1.80%40090-Job%22%2C%22regex-0.1.80%40100-incr-no-change%22%2C%22regex.0.1.30%22%2C%22rust-encoding-0.3.0%22%2C%22syntex-0.42.2%40000-base%22%2C%22syntex-0.42.2%40010-incr%22%2C%22syntex-0.42.2%40020-clean%22%2C%22tokio-webpush-simple%40000-base%22%2C%22tokio-webpush-simple%40010-incr%22%2C%22tokio-webpush-simple%40020-clean%22%2C%22tokio-webpush-simple%40030-minor-change%22%2C%22tuple-stress%22%5D&stat=max-rss
This is an 8% regression with incremental enabled, 1% without it. #44142
The text was updated successfully, but these errors were encountered: