-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
25% regression in serde compile time #47628
Comments
A not-too-deep check reveals that trait selection might have slowed down. Seriously? (I didn't track it down to that commit, should probably bisect it to a single commit in that range). |
triage: P-high Good to know what happened. |
I'll try to bisect. |
Curious. I was not able to reproduce when building those commits from scratch, though I did see the perf difference with the nightly builds. I'll double-check. It could have to do with some of my config.toml options (e.g., debuginfo or debug-assertions), but that seems a bit surprising. |
I tried with no config.toml at all and still got basically the same timing for those two commits: 11.6s in both cases. |
cc @rust-lang/compiler -- can someone else try to reproduce from source? |
I also built these two revisions from source. Both of my builds have the same timing of ~40 seconds for |
@Zoxc thanks! Not entirely sure what to make of that. |
cc @rust-lang/infra -- can we try using the PR-by-PR builds to narrow this down further or gain some insight? I'm not sure how to access those. |
@nikomatsakis The problem isn't in rustc, but cargo. Note that starting from Dec 25 cargo passes ~/serde ((v1.0.27)):160$ cargo clean && time cargo +nightly-2017-12-24 build -vv
Compiling serde v1.0.27 (file:///~/serde/serde)
Running `rustc --crate-name serde src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=7c335e5d4a9ee66d -C extra-filename=-7c335e5d4a9ee66d --out-dir /~/serde/target/debug/deps -L dependency=/~/serde/target/debug/deps`
Finished dev [unoptimized + debuginfo] target(s) in 9.50 secs
real 0m9.752s
user 0m9.463s
sys 0m0.409s
~/serde ((v1.0.27)):161$ cargo clean && time cargo +nightly-2017-12-25 build -vv
Compiling serde v1.0.27 (file:///~/serde/serde)
Running `rustc --crate-name serde serde/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=633312819f55a634 -C extra-filename=-633312819f55a634 --out-dir /~/serde/target/debug/deps -C incremental=/~/serde/target/debug/incremental -L dependency=/~/serde/target/debug/deps`
Finished dev [unoptimized + debuginfo] target(s) in 12.42 secs
real 0m13.098s
user 0m11.421s
sys 0m0.814s
~/serde ((v1.0.27)):162$ cargo clean && time cargo +16992930835ce3376a4aaed42307726e1fc78e45 build -vv
Compiling serde v1.0.27 (file:///~/serde/serde)
Running `rustc --crate-name serde serde/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=7a2f2aaec6829696 -C extra-filename=-7a2f2aaec6829696 --out-dir /~/serde/target/debug/deps -C incremental=/~/serde/target/debug/incremental -L dependency=/~/serde/target/debug/deps`
Finished dev [unoptimized + debuginfo] target(s) in 12.82 secs
real 0m13.442s
user 0m11.809s
sys 0m0.781s Edit: Disabling incremental with |
Is 25% within the expected range of slowdown from incremental compilation on a clean build? |
@dtolnay unknown, but it seems worth optimizing! Question: what happens on subsequent compiles? |
OK, so, here are latest measurements:
Seems like something changed in between 2018-02-04 and 2018-02-05. |
Could be disabling ThinLTO? |
In any case, the regression appears to be gone, in an absolute sense. A quick glance at the profiles suggested we are spending a lot of time hashing, but I'd have to do a custom build with framepointers to get real data. I'm inclined to close this, though. |
(It could well be that ThinLTO was making hashing etc add a lot more overhead than it otherwise should.) |
triage: P-medium Priority is not so hot. @michaelwoerister is going to do some final investigation and decide whether to close or what. |
triage: @michaelwoerister will confirm that build times are back to normal. If so, he will close as "not a regression" (or at least no longer a regression...) |
I did some measurements with the current nightly and the versions from the OP:
This confirms that a default build has gotten slower between the first two nightlies and then faster again. However, I think, the slowdown is due to incremental compilation becoming the default between the two nightlies. Forcing incremental compilation to off shows that there little difference between them, and that the compiler has actually become a bit faster since:
Vice versa, forcing incr. comp. to on shows little difference between the December nightlies and that incr. comp. especially has become faster:
Hopefully you'll also see a benefit from incr. comp.:
I think this explains the build time regression between the two nightlies. It is expected that clean incremental builds are slower. For the Dec 25 nightly the overhead is ~24% in my measurements, for the current nightly its ~16%. Hopefully we can bring this down further in the future. |
I narrowed this down to 1699293...c284f88. To reproduce:
For me nightly-2017-12-24 takes 7.4 seconds and nightly-2017-12-25 takes 9.2 seconds.
The text was updated successfully, but these errors were encountered: