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.

Sign up for GitHub

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

Jump to bottom

Speed up Token::{ident,lifetime} #96683

Merged
merged 3 commits into from
May 4, 2022
Merged

Speed up Token::{ident,lifetime} #96683

merged 3 commits into from
May 4, 2022

Conversation

nnethercote
Copy link
Contributor

Some speed and cleanliness improvements.

r? @petrochenkov

nnethercote added 3 commits May 4, 2022 06:15
@nnethercote
Speed up Token::{ident,lifetime}.
637a93c 
They're hot enough that the repeated matching done by `uninterpolate`
has a measurable effect.
@nnethercote
Fix spelling of an identifier.
bbef975
@nnethercote
Remove unnecessary NtIdent in Token::is_whole_expr.
1d2e172 
The comment on this function explains that it's a specialized version of
`maybe_whole_expr`. But `maybe_whole_expr` doesn't do anything with
`NtIdent`, so `is_whole_expr` also doesn't need to.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 3, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2022
@nnethercote
Copy link
Contributor Author

Some instruction counts results for check full runs of rustc-perf benchmarks:

Benchmark Profile Scenario % Change Significance Factor?
html5ever-0.26.0 check full -2.97% 14.85x
deep-vector check full -1.39% 6.94x
externs check full -1.07% 5.34x
serde_derive-1.0.136 check full -1.04% 5.20x
libc-0.2.124 check full -0.74% 3.68x
syn-1.0.89 check full -0.64% 3.22x
hyper-0.14.18 check full -0.47% 2.36x
serde-1.0.136 check full -0.35% 1.74x
stm32f4-0.14.0 check full -0.35% 1.73x

And for some benchmarks outside of rustc-perf:

Benchmark Profile Scenario % Change Significance Factor?
async-std-1.10.0 check full -4.92% 24.62x
time-macros-0.2.3 check full -4.10% 20.48x
yansi-0.5.0 check full -3.56% 17.78x
ctor-0.1.21 check full -1.79% 8.95x
scroll_derive-0.11.0 check full -1.79% 8.94x
inotify-0.10.0 check full -1.42% 7.12x
web-sys-0.3.56 check full -1.39% 6.95x
vsdb_derive-0.21.1 check full -1.30% 6.50x
tonic-build-0.7.0 check full -1.18% 5.91x
clap_derive-3.1.7 check full -1.16% 5.79x
wasm-bindgen-backend-0.2.79 check full -1.11% 5.55x
stdweb-derive-0.5.3 check full -1.10% 5.50x
enum-as-inner-0.4.0 check full -1.09% 5.46x
prost-derive-0.10.0 check full -1.07% 5.34x
wayland-scanner-0.29.4 check full -1.07% 5.33x
diesel_derives-1.4.1 check full -1.04% 5.22x
pyo3-macros-backend-0.16.3 check full -1.01% 5.04x

@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 3, 2022
@bors
Copy link
Contributor

bors commented May 3, 2022

⌛ Trying commit 1d2e172 with merge 1def8abfd2755bd91a8833fa7c0e8b0a941c63a2...

@bors
Copy link
Contributor

bors commented May 3, 2022

☀️ Try build successful - checks-actions
Build commit: 1def8abfd2755bd91a8833fa7c0e8b0a941c63a2 (1def8abfd2755bd91a8833fa7c0e8b0a941c63a2)

@rust-timer
Copy link
Collaborator

Queued 1def8abfd2755bd91a8833fa7c0e8b0a941c63a2 with parent 086bf7a, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1def8abfd2755bd91a8833fa7c0e8b0a941c63a2): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 0 148 79 148
mean2 N/A N/A -0.9% -0.7% -0.9%
max N/A N/A -6.8% -4.0% -6.8%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 3, 2022
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 4, 2022

📌 Commit 1d2e172 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2022
@bors
Copy link
Contributor

bors commented May 4, 2022

⌛ Testing commit 1d2e172 with merge 343889b...

@bors
Copy link
Contributor

bors commented May 4, 2022

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 343889b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 4, 2022
@bors bors merged commit 343889b into rust-lang:master May 4, 2022
@rustbot rustbot added this to the 1.62.0 milestone May 4, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (343889b): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 0 151 82 151
mean2 N/A N/A -0.9% -0.7% -0.9%
max N/A N/A -6.9% -4.0% -6.9%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@nnethercote nnethercote deleted the speed-up-Token-ident-lifetime branch May 4, 2022 21:11
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.
6 participants
@nnethercote @rust-timer @bors @petrochenkov @rust-highfive @rustbot