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

Put crate metadata first in the rlib #93816

Merged
merged 1 commit into from
Feb 20, 2022
Merged

Put crate metadata first in the rlib #93816

merged 1 commit into from
Feb 20, 2022

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Feb 9, 2022 •
edited
Loading

This should make metadata lookup faster

Fixes #93806

@bjorn3 bjorn3 added I-slow Issue: Problems and improvements with respect to performance of generated code. A-metadata Area: Crate metadata T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-compiler-performance Working group: Compiler Performance labels Feb 9, 2022
@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 9, 2022
@bjorn3
Copy link
Member Author

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

bors commented Feb 9, 2022

⌛ Trying commit d0402c7b74a80e9236c8118717da4ec6ce2b2b47 with merge a98a13ec95bd42fbd8ab118f08091d9c1d7ad9dc...

@bjorn3 bjorn3 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 Feb 9, 2022
@petrochenkov petrochenkov self-assigned this Feb 9, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Feb 9, 2022

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

@rust-timer
Copy link
Collaborator

Queued a98a13ec95bd42fbd8ab118f08091d9c1d7ad9dc with parent b7cd0f7, future comparison URL.

@bjorn3 bjorn3 added I-compiletime Issue: Problems and improvements with respect to compile times. and removed I-slow Issue: Problems and improvements with respect to performance of generated code. labels Feb 9, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a98a13ec95bd42fbd8ab118f08091d9c1d7ad9dc): comparison url.

Summary: This benchmark run did not return any relevant results.

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.

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Feb 9, 2022
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 9, 2022

It seems like this slightly reduces cycle count overall, a bigger inprovement in fault count and a significant up to 6% reduction in max-rss. Wall time is too noisy to notice any effect.

@petrochenkov petrochenkov removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 10, 2022
@petrochenkov petrochenkov removed their assignment Feb 10, 2022
@bjorn3 bjorn3 changed the title [PERF] Put crate metadata first in the rlib Put crate metadata first in the rlib Feb 10, 2022
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 10, 2022

This now doesn't place the metadata first if rustc can't wrap the metadata in an object file for the respective target.

Blocked on #93864

@bjorn3 bjorn3 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2022
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 11, 2022

#93864 has landed.

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 11, 2022
@bjorn3 bjorn3 removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Feb 11, 2022
@lcnr
Copy link
Contributor

lcnr commented Feb 14, 2022

r? @nagisa maybe

@rust-highfive rust-highfive assigned nagisa and unassigned lcnr Feb 14, 2022
@nagisa
Copy link
Member

nagisa commented Feb 17, 2022

r=me, probably want to squash away the fix review comments commit.

@bjorn3
Copy link
Member Author

bjorn3 commented Feb 17, 2022

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Feb 17, 2022

📌 Commit 9dc0c09c9f01671f6604eb9a3892fc3d1790c66e has been approved by nagisa

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

bors commented Feb 18, 2022

☔ The latest upstream changes (presumably #94103) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 18, 2022
@bjorn3
Put crate metadata first in the rlib when possible
932559c 
This should make metadata lookup faster
@bjorn3
Copy link
Member Author

bjorn3 commented Feb 18, 2022

Rebased

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Feb 18, 2022

📌 Commit 932559c has been approved by nagisa

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2022
@bors
Copy link
Contributor

bors commented Feb 20, 2022

⌛ Testing commit 932559c with merge 3b18651...

@bors
Copy link
Contributor

bors commented Feb 20, 2022

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 3b18651 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 20, 2022
@bors bors merged commit 3b18651 into rust-lang:master Feb 20, 2022
@rustbot rustbot added this to the 1.61.0 milestone Feb 20, 2022
@bjorn3 bjorn3 deleted the rlib_metadata_first branch February 20, 2022 14:44
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3b18651): comparison url.

Summary: This benchmark run did not return any relevant results. 7 results were found to be statistically significant but too small to be relevant.

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

@rustbot label: -perf-regression

@bjorn3
Copy link
Member Author

bjorn3 commented Feb 20, 2022

Unlike the previous perf run this slightly increases instruction count. It does still result in a decent max-rss reduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nagisa nagisa nagisa left review comments

Assignees

@nagisa nagisa

Labels
A-metadata Area: Crate metadata I-compiletime Issue: Problems and improvements with respect to compile times. 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. WG-compiler-performance Working group: Compiler Performance
Projects
None yet
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.

Put crate metadata first in rlibs whenever possible
9 participants
@bjorn3 @rust-highfive @rust-timer @bors @rust-log-analyzer @lcnr @nagisa @petrochenkov @rustbot