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 `hygiene_data` lookups #61253

Merged
merged 5 commits into from May 30, 2019

Conversation

@nnethercote
Copy link
Contributor

commented May 28, 2019

These commits mostly introduce compound operations that allow two close adjacent hygiene_data lookups to be combined.

r? @petrochenkov

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

Local results were very good, let's hope the CI results are similar. Instruction counts:

packed-simd-check
        avg: -6.4%      min: -10.2%     max: -0.5%
helloworld-check
        avg: -2.4%      min: -2.8%      max: -2.2%
unify-linearly-check
        avg: -1.9%      min: -2.7%      max: -1.3%
deeply-nested-check
        avg: -1.6%      min: -2.7%      max: -1.1%
script-servo-check
        avg: -2.0%      min: -2.6%      max: -1.6%
webrender-check
        avg: -1.8%      min: -2.4%      max: -1.2%
cranelift-codegen-check
        avg: -1.8%      min: -2.3%      max: -1.4%
style-servo-check
        avg: -1.8%      min: -2.1%      max: -1.5%
issue-46449-check
        avg: -1.5%      min: -1.9%      max: -1.3%
inflate-check
        avg: -0.5%      min: -1.6%      max: -0.1%
crates.io-check
        avg: -1.1%      min: -1.5%      max: -0.6%
regression-31157-check
        avg: -1.0%      min: -1.5%      max: -0.6%
sentry-cli-check
        avg: -1.2%      min: -1.5%      max: -0.9%
regex-check
        avg: -1.1%      min: -1.4%      max: -1.0%
tokio-webpush-simple-check
        avg: -1.0%      min: -1.4%      max: -0.7%
ripgrep-check
        avg: -1.0%      min: -1.2%      max: -0.8%
wg-grammar-check
        avg: -0.6%      min: -1.2%      max: -0.2%
futures-check
        avg: -0.7%      min: -1.1%      max: -0.4%
serde-check
        avg: -0.7%      min: -1.0%      max: -0.5%
piston-image-check
        avg: -0.8%      min: -1.0%      max: -0.6%
encoding-check
        avg: -0.8%      min: -1.0%      max: -0.5%
syn-check
        avg: -0.7%      min: -1.0%      max: -0.5%
clap-rs-check
        avg: -0.6%      min: -0.9%      max: -0.5%
hyper-check
        avg: -0.7%      min: -0.9%      max: -0.5%
unused-warnings-check 
        avg: -0.6%      min: -0.8%      max: -0.5%

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

⌛️ Trying commit 265ab64 with merge 9f4ea3d...

bors added a commit that referenced this pull request May 28, 2019

Auto merge of #61253 - nnethercote:avoid-hygiene_data-lookups, r=<try>
Avoid `hygiene_data` lookups

These commits mostly introduce compound operations that allow two close adjacent `hygiene_data` lookups to be combined.

r? @petrochenkov
@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@Manishearth: the new outer_expn_info() function introduced in the second commit can be used in Clippy -- every single occurence of outer() in Clippy is immediately followed by expn_info(). (Note that some of these occurrences span two lines, and won't be found with a simple grep.) It's possible that using the new function might speed up Clippy.

@bors

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

☀️ Try build successful - checks-travis
Build commit: 9f4ea3d

@Manishearth

This comment has been minimized.

Copy link
Member

commented May 28, 2019

Thanks for the heads up!

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Success: Queued 9f4ea3d with parent b711179, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

19 similar comments
@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented May 28, 2019

Finished benchmarking try commit 9f4ea3d, comparison URL.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

Suggestion: make outer(), expn_info() and is_descendant_of() private methods on HygieneData and call them from hte Mark/SyntaxCtxt methods.

(Everything else looks good, but I didn't look at the last commit yet, will look later.)

Show resolved Hide resolved src/librustc/ty/mod.rs Outdated
Show resolved Hide resolved src/librustc/ty/mod.rs Outdated

nnethercote added some commits May 27, 2019

Optimize `TyCtxt::adjust_ident`.
It's a hot function that returns a 2-tuple, but the hottest call site
(`hygienic_eq`) discards the second element.

This commit renames `adjust_ident` as `adjust_ident_and_get_scope`, and
then introduces a new `adjust_ident` that only computes the first
element. This change also avoids the need to pass in an unused
`DUMMY_HIR_ID` argument in a couple of places, which is nice.
Add `HygieneData::{outer,expn_info,is_descendant_of}` methods.
This commit factors out some repeated code.

@nnethercote nnethercote force-pushed the nnethercote:avoid-hygiene_data-lookups branch from 265ab64 to 95ea7fd May 29, 2019

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

@petrochenkov: New code is up. I have addressed your comments. The new functions are in a new commit at the end of the series.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Thanks!
@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

📌 Commit 95ea7fd has been approved by petrochenkov

@Centril

This comment has been minimized.

Copy link
Member

commented May 30, 2019

@bors rollup=never

for perf.

@Centril

This comment has been minimized.

Copy link
Member

commented May 30, 2019

@bors p=1

because never.

bors added a commit that referenced this pull request May 30, 2019

Auto merge of #61253 - nnethercote:avoid-hygiene_data-lookups, r=petr…
…ochenkov

Avoid `hygiene_data` lookups

These commits mostly introduce compound operations that allow two close adjacent `hygiene_data` lookups to be combined.

r? @petrochenkov

@rust-lang rust-lang deleted a comment from bors May 30, 2019

@rust-lang rust-lang deleted a comment from rust-highfive May 30, 2019

@bors

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

⌛️ Testing commit 95ea7fd with merge 0bfbaa6...

bors added a commit that referenced this pull request May 30, 2019

Auto merge of #61253 - nnethercote:avoid-hygiene_data-lookups, r=petr…
…ochenkov

Avoid `hygiene_data` lookups

These commits mostly introduce compound operations that allow two close adjacent `hygiene_data` lookups to be combined.

r? @petrochenkov
@bors

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing 0bfbaa6 to master...

@bors bors added the merged-by-bors label May 30, 2019

@bors bors merged commit 95ea7fd into rust-lang:master May 30, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@nnethercote nnethercote deleted the nnethercote:avoid-hygiene_data-lookups branch May 30, 2019

@nnethercote

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

Final perf improvements are here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.