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

Box all fields of ast::*ItemKind #81683

Closed
wants to merge 7 commits into from
Closed

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Feb 2, 2021

Follow up to #81405, this PR boxes all fields, making ast::ItemKind, ast::AssocItemKind and ast::ForeignItemKind all 16 bytes. This results in less wasted memory when the smaller variants are instantiated a lot.

@rust-highfive
Copy link
Collaborator

r? @lcnr

(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 2, 2021
@bugadani
Copy link
Contributor Author

bugadani commented Feb 2, 2021

Some variants have boxed fields which can now be inlined. I intend to clean those up where possible in a future PR.

@bugadani bugadani changed the title Box all fields of ast::*Item Box all fields of ast::*ItemKind Feb 2, 2021
@tmiasko
Copy link
Contributor

tmiasko commented Feb 2, 2021

@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 2, 2021
@bors
Copy link
Contributor

bors commented Feb 2, 2021

⌛ Trying commit c12a8ca with merge cfa444729c2a1a0ea8ba7a394e8bed7d5fad6011...

@bors
Copy link
Contributor

bors commented Feb 2, 2021

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

@rust-timer
Copy link
Collaborator

Queued cfa444729c2a1a0ea8ba7a394e8bed7d5fad6011 with parent 3182375, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (cfa444729c2a1a0ea8ba7a394e8bed7d5fad6011): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 3, 2021
@bugadani
Copy link
Contributor Author

bugadani commented Feb 3, 2021

Around .5% regressions in several doc benchmarks. max-rss shows varied results.

@bugadani bugadani marked this pull request as draft February 3, 2021 20:17
@bugadani bugadani closed this Feb 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants