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

Revert most of MaybeUninit, except for the new API itself #54554

Merged
merged 2 commits into from
Sep 29, 2018
Merged

Revert most of MaybeUninit, except for the new API itself #54554

merged 2 commits into from
Sep 29, 2018

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 25, 2018 •
edited
Loading

This reverts most of #53508 for perf reasons (first commit reverts that entire PR), except for the new API itself (added back in 2nd commit).

@rust-highfive
Copy link
Collaborator

r? @withoutboats

(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 Sep 25, 2018
@RalfJung RalfJung mentioned this pull request Sep 25, 2018
Merged
@RalfJung
Copy link
Member Author

@rust-lang/wg-compiler-performance I'd prefer to also include 96572cb but wasn't sure if you'd be happy with that.

@nagisa
Copy link
Member

nagisa commented Sep 25, 2018

Do we know why the regression occurred?

@withoutboats
Copy link
Contributor

@r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned withoutboats Sep 25, 2018
@RalfJung
Copy link
Member Author

@nagisa No. Don't have time to do any research right now, I am on a conference.

But my preliminary plan was to then re-submit "half" of the remaining commits and see if perf complains, etc. My guess is that it's the BTreeMap changes. Though I'd also be happy if someone else does all of that, ;)

@nagisa
Copy link
Member

nagisa commented Sep 25, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Sep 25, 2018

📌 Commit b595e4f82377bb2c50201b3fc8b1f528cdc49b93 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 Sep 25, 2018
@bors
Copy link
Contributor

bors commented Sep 26, 2018

☔ The latest upstream changes (presumably #54575) 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 Sep 26, 2018
@nagisa
Copy link
Member

nagisa commented Sep 27, 2018

@RalfJung this wants a rebase.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 27, 2018 via email

@RalfJung
Copy link
Member Author

Rebase done.

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Sep 29, 2018

📌 Commit 546e45a 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 Sep 29, 2018
@kennytm
Copy link
Member

kennytm commented Sep 29, 2018

@bors p=15

rollup fairness

@bors
Copy link
Contributor

bors commented Sep 29, 2018

⌛ Testing commit 546e45a with merge 7e7bc06...

bors added a commit that referenced this pull request Sep 29, 2018
@bors
Auto merge of #54554 - RalfJung:maybe-uninit, r=nagisa
7e7bc06 
Revert most of MaybeUninit, except for the new API itself

This reverts most of #53508 for perf reasons (first commit reverts that entire PR), except for the new API itself (added back in 2nd commit).
@bors
Copy link
Contributor

bors commented Sep 29, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing 7e7bc06 to master...

@bors bors merged commit 546e45a into rust-lang:master Sep 29, 2018
This was referenced Sep 29, 2018
Closed
Merged
Merged
@nnethercote
Copy link
Contributor

The final perf improvements from this landing are here.

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

@eddyb eddyb

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@RalfJung @rust-highfive @nagisa @withoutboats @bors @kennytm @nnethercote @eddyb @japaric