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

Tell rustc about unused bits in Span. #93747

Closed
wants to merge 1 commit into from
Closed

Tell rustc about unused bits in Span. #93747

wants to merge 1 commit into from

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Feb 7, 2022

The maximum value for len_or_tag in encoded Span is LEN_TAG, which is less than u32::MAX.
#84290 determined the extra room was not really useful for perf.

This PR tells rustc about the extra room at the top, hoping for more aggressive layout optimizations.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 7, 2022
@rust-highfive
Copy link
Collaborator

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 7, 2022
@cjgillot
Copy link
Contributor Author

cjgillot commented Feb 7, 2022

@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 Feb 7, 2022
@bors
Copy link
Contributor

bors commented Feb 7, 2022

⌛ Trying commit 4749961 with merge 71f1cf816a84020c13ca2ee1a5c2a88314d44b42...

@bors
Copy link
Contributor

bors commented Feb 8, 2022

☀️ Try build successful - checks-actions
Build commit: 71f1cf816a84020c13ca2ee1a5c2a88314d44b42 (71f1cf816a84020c13ca2ee1a5c2a88314d44b42)

@rust-timer
Copy link
Collaborator

Queued 71f1cf816a84020c13ca2ee1a5c2a88314d44b42 with parent f52c318, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (71f1cf816a84020c13ca2ee1a5c2a88314d44b42): comparison url.

Summary: This benchmark run shows 20 relevant regressions 😿 to instruction counts.

  • Average relevant regression: 0.4%
  • Largest regression in instruction counts: 1.3% on incr-full builds of ctfe-stress-4 check

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 8, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2022
@apiraino
Copy link
Contributor

@cjgillot checking review status: are the perf regressions triaged and should this PR wait a little bit more before review? I'll assign it's waiting on the author, but please feel free to flip the switch if it's the case, thanks!

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2022
@cjgillot cjgillot closed this Mar 26, 2022
@cjgillot cjgillot deleted the spanopt branch March 26, 2022 13:25
@nnethercote nnethercote mentioned this pull request Sep 20, 2022
Closed
nnethercote added a commit to nnethercote/rust that referenced this pull request Oct 17, 2022
@nnethercote @cjgillot
Tell rustc about unused bits in Span.
273fb2a 
This shrinks the size of `Option<Span>` and similar types, which shrinks
several AST and HIR nodes. The most important of these are `hir::Expr`
and `ast::Ty`.

This change was first attempted in rust-lang#93747 where it had little effect.
But the improved niche-filling implemented by rust-lang#94075 means this change
now has a much bigger effect.

Co-authored-by: Camille Gillot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@wesleywiser wesleywiser

Labels
perf-regression Performance regression. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@cjgillot @rust-highfive @rust-timer @bors @apiraino @wesleywiser @JohnCSimon @rustbot