Skip to content

datastructures: replace once_cell crate with an impl from std - #76075

Merged
bors merged 1 commit into
rust-lang:masterfrom
marmeladema:remove-once-cell-crate
Aug 31, 2020
Merged

datastructures: replace once_cell crate with an impl from std#76075
bors merged 1 commit into
rust-lang:masterfrom
marmeladema:remove-once-cell-crate

Conversation

@marmeladema

@marmelademamarmeladema commented Aug 29, 2020

Copy link
Copy Markdown
Contributor

Fixes#75700

r? @matklad

We might need a perf run for this change.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2020
Mark-Simulacrum
Mark-Simulacrum reviewed Aug 29, 2020
View reviewed changes
Comment threadsrc/librustc_interface/lib.rs Outdated

@Mark-SimulacrumMark-SimulacrumAug 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've expected import changes as well - is this coming from a macro or something like that? If so that should get a allow_internal_unstable annotation instead, probably

@marmelademamarmelademaAug 29, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not exactly sure what you are referring to, but in src/librustc_interface/passes.rs and src/librustc_interface/queries.rs the code imports OnceCell re-exported from librustc_datastructures.

Side note: there is also an explicit import (and thus a dependency on the crate) of once_cell::sync::OnceCell in src/librustc_interface/util.rs though and I don't know if that should be updated to use the impl from std.

ecstatic-morse
ecstatic-morse reviewed Aug 29, 2020
View reviewed changes
Comment threadsrc/librustc_data_structures/sync.rs Outdated
@jyn514jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 29, 2020
@marmeladema
marmelademaforce-pushed the remove-once-cell-crate branch from b8e4b77 to 2ced222CompareAugust 29, 2020 23:11
@jyn514

jyn514 commented Aug 30, 2020

Copy link
Copy Markdown
Member

@bors try @rust-timer queue

@rust-timer

rust-timer commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 2ced2226acb1f6fd027e87e5918e81db8320505b with merge e1c8714b3f420be20f3c670b7ad84eccbe252b26...

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions, checks-azure
Build commit: e1c8714b3f420be20f3c670b7ad84eccbe252b26 (e1c8714b3f420be20f3c670b7ad84eccbe252b26)

@rust-timer

rust-timer commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

Queued e1c8714b3f420be20f3c670b7ad84eccbe252b26 with parent 5c27700, future comparison URL.

@rust-timer

rust-timer commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

Finished benchmarking try commit (e1c8714b3f420be20f3c670b7ad84eccbe252b26): 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

@jyn514

jyn514 commented Aug 30, 2020

Copy link
Copy Markdown
Member

Nice, basically no change in instructions but significantly less max-rss :)

@andjo403

andjo403 commented Aug 30, 2020

Copy link
Copy Markdown
Contributor

@jyn514 think that it is to little data to say anything about the max-rss due to how big the variance is if you look at perf graphs you will see that eg. keccak-debug have varied between -10% to +20%.

@matklad

matklad commented Aug 30, 2020

Copy link
Copy Markdown
Contributor

Yeah, I would be very surprised if the perf turned out to be non-neutral on this one !

@bors r+ rollup

Let’s change direct usages of once_cell and lazy_static crates to std::lazy in a separate PR.

I am not sure about the policy of using librustc_datastructures re-export vs direct use, but it should always be safe to not change this particular aspect.

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 2ced2226acb1f6fd027e87e5918e81db8320505b has been approved by matklad

@borsbors 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 Aug 30, 2020
@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

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

@borsbors 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 Aug 30, 2020
@marmeladema
marmelademaforce-pushed the remove-once-cell-crate branch from 2ced222 to 68500ffCompareAugust 30, 2020 19:09
@marmeladema

marmeladema commented Aug 30, 2020

Copy link
Copy Markdown
ContributorAuthor

@bors r=matklad

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

@marmeladema: 🔑 Insufficient privileges: Not in reviewers

@marmeladema

marmeladema commented Aug 30, 2020

Copy link
Copy Markdown
ContributorAuthor

@matklad I need another approval from your part I believe 👍

@ecstatic-morse

ecstatic-morse commented Aug 30, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

@bors delegate+

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

✌️ @marmeladema can now approve this pull request

@marmeladema

marmeladema commented Aug 30, 2020

Copy link
Copy Markdown
ContributorAuthor

@bors r=matklad

@bors

bors commented Aug 30, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 68500ff has been approved by matklad

@borsbors 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 Aug 30, 2020
@matkladmatklad mentioned this pull request Aug 31, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2020
@bors
Auto merge ofrust-lang#76159- matklad:rollup-8jydjg3, r=matklad
1fd8636
Rollup of 5 pull requests
Successful merges:
- rust-lang#75938 (Added some `min_const_generics` revisions into `const_generics` tests)
- rust-lang#76050 (Remove unused function)
- rust-lang#76075 (datastructures: replace `once_cell` crate with an impl from std)
- rust-lang#76115 (Restore public visibility on some parsing functions for rustfmt)
- rust-lang#76127 (rustbuild: Remove one LLD workaround)
Failed merges:
r? @ghost
@bors
bors merged commit 67f1643 into rust-lang:masterAug 31, 2020
@marmeladema
marmeladema deleted the remove-once-cell-crate branch April 24, 2021 09:11
@cuvipercuviper added this to the 1.48.0 milestone Nov 17, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ecstatic-morseecstatic-morseAwaiting requested review from ecstatic-morse
@Mark-SimulacrumMark-SimulacrumAwaiting requested review from Mark-Simulacrum

Assignees

@matkladmatklad

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.48.0

Development

Successfully merging this pull request may close these issues.

Replace once_cell crate from librustc_datastructures with an (unstable) impl from std

10 participants

@marmeladema@jyn514@rust-timer@bors@andjo403@matklad@ecstatic-morse@Mark-Simulacrum@cuviper@rust-highfive