Skip to content

Use box syntax instead of Box::new in Mutex::remutex on Windows #49646

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 16, 2018

Conversation

glandium
Copy link
Contributor

@glandium glandium commented Apr 4, 2018

The Box::new(mem::uninitialized()) pattern actually actively copies
uninitialized bytes from the stack into the box, which is a waste of
time. Using the box syntax instead avoids the useless copy.

The Box::new(mem::uninitialized()) pattern actually actively copies
uninitialized bytes from the stack into the box, which is a waste of
time. Using the box syntax instead avoids the useless copy.
@pietroalbini pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 4, 2018
@aidanhs
Copy link
Member

aidanhs commented Apr 6, 2018

It surprises me that this would make a difference, given that Box::new is an #[inline(always)] wrapper around box syntax - https://github.com/rust-lang/rust/blob/83669ec/src/liballoc/boxed.rs#L92-L95.

Just thinking aloud, I wonder if there's a way to fix Box::new instead?

@pietroalbini pietroalbini added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 16, 2018
@pietroalbini
Copy link
Member

Ping from triage! This PR needs a review. Can @withoutboats or someone else from @rust-lang/libs review this?

@withoutboats
Copy link
Contributor

@bors r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 16, 2018

📌 Commit 4577da7 has been approved by alexcrichton

@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 Apr 16, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 16, 2018
…richton

Use box syntax instead of Box::new in Mutex::remutex on Windows

The Box::new(mem::uninitialized()) pattern actually actively copies
uninitialized bytes from the stack into the box, which is a waste of
time. Using the box syntax instead avoids the useless copy.
bors added a commit that referenced this pull request Apr 16, 2018
Rollup of 8 pull requests

Successful merges:

 - #49555 (Inline most of the code paths for conversions with boxed slices)
 - #49606 (Prevent broken pipes causing ICEs)
 - #49646 (Use box syntax instead of Box::new in Mutex::remutex on Windows)
 - #49647 (Remove `underscore_lifetimes` and `match_default_bindings` from active feature list)
 - #49931 (Fix incorrect span in `&mut` suggestion)
 - #49959 (rustbuild: allow building tools with debuginfo)
 - #49965 (Remove warning about f64->f32 cast being potential UB)
 - #49994 (Remove unnecessary indentation in rustdoc book codeblock.)

Failed merges:
@bors bors merged commit 4577da7 into rust-lang:master Apr 16, 2018
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 18, 2021
Remove box syntax from compiler and tools

Removes box syntax from the compiler and tools. In rust-lang#49733, the future of box syntax is uncertain and the use in the compiler was listed as one of the reasons to keep it. Removal of box syntax [might affect the code generated](rust-lang#49646 (comment)) and slow down the compiler so I'd recommend doing a perf run on this.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants