Skip to content
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

Avoid closures to improve compile times. #183

Merged
merged 1 commit into from
Aug 7, 2020

Conversation

nnethercote
Copy link
Contributor

HashMap and HashSet are used widely, and often instantiated many
times. As a result, small differences in how the code is written can
have a significant effect on how much LLVM IR is generated, which
affects compile times.

This commit avoids a lot of small closures by replacing calls to
Option::map, Option::ok_or_else, Option::unwrap_or_else and
Result::unwrap_or_elsewithmatch` expressions. Although this makes
the code less concise, it improves compile times. For example, several
of the benchmarks in rustc-perf compile up to 3.5% faster after this
change is incorporated into std.

Every change is accompanied by a short comment to explain why a match
is used. This may seem excessive, but without these comments it would be
easy for a well-meaning person in the future to change some or all of
these back to the original versions without understanding the
consequences.

@nnethercote
Copy link
Contributor Author

Here are some measurements from my Linux box, measuring full (non-incremental) builds for the "real-world" benchmarks in rustc-perf.

cargo-debug
        avg: -3.5%      min: -3.5%      max: -3.5%
webrender-wrench-debug
        avg: -2.8%      min: -2.8%      max: -2.8%
regex-debug
        avg: -2.7%      min: -2.7%      max: -2.7%
webrender-debug
        avg: -2.7%      min: -2.7%      max: -2.7%
tokio-webpush-simple-debug
        avg: -1.9%      min: -1.9%      max: -1.9%
webrender-opt
        avg: -1.9%      min: -1.9%      max: -1.9%
piston-image-debug
        avg: -1.4%      min: -1.4%      max: -1.4%
cranelift-codegen-debug
        avg: -1.3%      min: -1.3%      max: -1.3%
cargo-opt
        avg: -1.3%      min: -1.3%      max: -1.3%
ripgrep-debug
        avg: -1.3%      min: -1.3%      max: -1.3%
clap-rs-debug
        avg: -1.2%      min: -1.2%      max: -1.2%
clap-rs-opt
        avg: -1.2%      min: -1.2%      max: -1.2%
html5ever-debug
        avg: -1.1%      min: -1.1%      max: -1.1%
html5ever-opt
        avg: -1.0%      min: -1.0%      max: -1.0%
syn-debug
        avg: -1.0%      min: -1.0%      max: -1.0%
regex-opt
        avg: 1.0%       min: 1.0%       max: 1.0%
cranelift-codegen-opt
        avg: -0.9%      min: -0.9%      max: -0.9%
hyper-2-debug
        avg: -0.8%      min: -0.8%      max: -0.8%
serde-check
        avg: -0.8%      min: -0.8%      max: -0.8%
futures-debug
        avg: -0.8%      min: -0.8%      max: -0.8%
serde-debug
        avg: -0.8%      min: -0.8%      max: -0.8%
serde-opt
        avg: -0.7%      min: -0.7%      max: -0.7%
futures-check
        avg: -0.6%      min: -0.6%      max: -0.6%
clap-rs-check
        avg: -0.6%      min: -0.6%      max: -0.6%
ripgrep-opt
        avg: -0.6%      min: -0.6%      max: -0.6%
tokio-webpush-simple-opt
        avg: -0.6%      min: -0.6%      max: -0.6%
piston-image-opt
        avg: -0.5%      min: -0.5%      max: -0.5%
cargo-check
        avg: -0.5%      min: -0.5%      max: -0.5%
syn-opt
        avg: -0.5%?     min: -0.5%?     max: -0.5%?
hyper-2-check
        avg: -0.5%      min: -0.5%      max: -0.5%
webrender-wrench-opt
        avg: -0.4%      min: -0.4%      max: -0.4%
html5ever-check
        avg: -0.4%      min: -0.4%      max: -0.4%
hyper-2-opt
        avg: -0.4%      min: -0.4%      max: -0.4%
piston-image-check
        avg: -0.4%      min: -0.4%      max: -0.4%
futures-opt
        avg: -0.4%      min: -0.4%      max: -0.4%
webrender-check
        avg: -0.4%      min: -0.4%      max: -0.4%
cranelift-codegen-check
        avg: -0.4%      min: -0.4%      max: -0.4%
ripgrep-check
        avg: -0.4%      min: -0.4%      max: -0.4%
regex-check
        avg: -0.4%      min: -0.4%      max: -0.4%
webrender-wrench-check
        avg: -0.4%      min: -0.4%      max: -0.4%
tokio-webpush-simple-check
        avg: -0.3%      min: -0.3%      max: -0.3%
syn-check
        avg: -0.3%      min: -0.3%      max: -0.3%

`HashMap` and `HashSet` are used widely, and often instantiated many
times. As a result, small differences in how the code is written can
have a significant effect on how much LLVM IR is generated, which
affects compile times.

This commit avoids a lot of small closures by replacing calls to
`Option::map`, `Option::ok_or_else`, `Option::unwrap_or_else` and
`Result::unwrap_or_else` with `match` expressions. Although this makes
the code less concise, it improves compile times. For example, several
of the benchmarks in rustc-perf compile up to 3.5% faster after this
change is incorporated into std.

Every change is accompanied by a short comment to explain why a `match`
is used. This may seem excessive, but without these comments it would be
easy for a well-meaning person in the future to change some or all of
these back to the original versions without understanding the
consequences.
@Amanieu
Copy link
Member

Amanieu commented Aug 7, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Aug 7, 2020

📌 Commit e47bec1 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Aug 7, 2020

⌛ Testing commit e47bec1 with merge 09e43a8...

@bors
Copy link
Contributor

bors commented Aug 7, 2020

☀️ Test successful - checks-travis
Approved by: Amanieu
Pushing 09e43a8 to master...

@bors bors merged commit 09e43a8 into rust-lang:master Aug 7, 2020
@nnethercote nnethercote deleted the avoid-closures branch August 7, 2020 23:57
@nnethercote
Copy link
Contributor Author

@Amanieu: I know that hashbrown was just upgraded in std to 0.8.1, but I think it would be worth doing a 0.8.2 release with this change so that the compile time benefits can be spread widely?

@Amanieu
Copy link
Member

Amanieu commented Aug 8, 2020

Sure, I'll do that after #184 is merged.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2020
Update hashbrown to 0.8.2

Includes:
- Avoid closures to improve compile times (rust-lang/hashbrown#183)
- Do not iterate to drop if empty (rust-lang/hashbrown#182)

r? @Mark-Simulacrum
jonhoo pushed a commit to jonhoo/griddle that referenced this pull request Aug 9, 2020
Amanieu added a commit to Amanieu/hashbrown that referenced this pull request Mar 4, 2021
We manually expand these to reduce the amount of LLVM IR generated. (rust-lang#183)
@Amanieu Amanieu mentioned this pull request Mar 4, 2021
bors added a commit that referenced this pull request Mar 4, 2021
Allow clippy::manual_map

We manually expand these to reduce the amount of LLVM IR generated. (#183)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants