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

Don't duplicate the extern providers once for each crate #83649

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 29, 2021

This should give a small perf improvement for small crates by avoiding a memcpy of a pretty big struct for each loaded crate. In addition would be useful for replacing the sequential CrateNum everywhere with the hash based StableCrateId introduced in #81635, which would allow avoiding remapping of CrateNum's when loading crate metadata. While this PR is not strictly needed for that, it is necessary to prevent a performance loss due to it.

I think this duplication was done in #40008 (which introduced the query system) to make it possible to compile multiple crates in a single session in the future. I think this is unlikely to be implemented any time soon. In addition this PR can easily be reverted if necessary to implement this.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Mar 29, 2021
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 29, 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 Mar 29, 2021
@bors
Copy link
Contributor

bors commented Mar 29, 2021

⌛ Trying commit cd7a011 with merge 47f4b9cd95f9b17537af3b251c2e5b97369c3acf...

@bors
Copy link
Contributor

bors commented Mar 29, 2021

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

@rust-timer
Copy link
Collaborator

Queued 47f4b9cd95f9b17537af3b251c2e5b97369c3acf with parent 3aedcf0, future comparison URL.

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (47f4b9cd95f9b17537af3b251c2e5b97369c3acf): 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 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 Mar 29, 2021
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 29, 2021

📌 Commit cd7a011 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 Mar 29, 2021
@bors
Copy link
Contributor

bors commented Mar 30, 2021

⌛ Testing commit cd7a011 with merge a0e229a...

@bors
Copy link
Contributor

bors commented Mar 30, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2021
@bors bors merged commit a0e229a into rust-lang:master Mar 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 30, 2021
@bjorn3 bjorn3 deleted the dedup_providers branch March 30, 2021 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants