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

Avoid zeroing large stack buffers in stdio on Windows #101193

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

thomcc
Copy link
Member

@thomcc thomcc commented Aug 30, 2022

Does what it says on the tin, using [MaybeUninit<u16>; N] instead of [0u16; N]. These buffers seem to be around 8kb, which is big enough that this is likely to be a very nice perf boost to stdio-heavy windows code.

r? @ChrisDenton

(Note: this PR also has a commit that adds windows to CI, but as it mentions I'll revert that after it comes out green -- I can only do a check build on the machine I'm typing this on)

@rustbot rustbot added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 30, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 30, 2022
@thomcc
Copy link
Member Author

thomcc commented Aug 30, 2022

CI is green on windows (https://github.com/rust-lang/rust/runs/8089709575?check_suite_focus=true and https://github.com/rust-lang/rust/runs/8089709693?check_suite_focus=true), so I dropped the CI edit commit, and this should be reviewable.

(edit: this is why I removed the t-infra label -- it was only here because I wanted to add windows to the CI temporarily)

Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of things

library/std/src/sys/windows/stdio.rs Outdated Show resolved Hide resolved
library/std/src/sys/windows/stdio.rs Outdated Show resolved Hide resolved
library/std/src/sys/windows/stdio.rs Outdated Show resolved Hide resolved
@thomcc thomcc added O-windows Operating system: Windows and removed T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 30, 2022
@ChrisDenton
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 30, 2022

📌 Commit 1b8b2dc has been approved by ChrisDenton

It is now in the queue for this repository.

@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 Aug 30, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 30, 2022
…nton

Avoid zeroing large stack buffers in stdio on Windows

Does what it says on the tin, using `[MaybeUninit<u16>; N]` instead of `[0u16; N]`. These buffers seem to be around 8kb, which is big enough that this is likely to be a very nice perf boost to stdio-heavy windows code.

r? `@ChrisDenton`

*(Note: this PR also has a commit that adds windows to CI, but as it mentions I'll revert that after it comes out green -- I can only do a check build on the machine I'm typing this on)*
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2022
Rollup of 10 pull requests

Successful merges:

 - rust-lang#100804 (Fix search results color on hover for ayu theme)
 - rust-lang#100892 (Add `AsFd` implementations for stdio types on WASI.)
 - rust-lang#100927 (Adding new Fuchsia rustup docs... reworking walkthrough)
 - rust-lang#101088 (Set DebuginfoKind::Pdb in msvc_base)
 - rust-lang#101159 (add tracking issue number to const_slice_split_at_not_mut)
 - rust-lang#101192 (Remove path string)
 - rust-lang#101193 (Avoid zeroing large stack buffers in stdio on Windows)
 - rust-lang#101197 (:arrow_up: rust-analyzer)
 - rust-lang#101200 (Add test for issue rust-lang#85872)
 - rust-lang#101219 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f19783d into rust-lang:master Aug 31, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 31, 2022
@thomcc thomcc deleted the win-stdio-nozero branch August 31, 2022 11:25
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
Avoid needless buffer zeroing in `std::sys::windows::fs`

Followup to rust-lang#101171 and rust-lang#101193. This finishes up avoiding buffer zeroing pointed out in rust-lang#100729 (comment) (thanks!)

r? `@ChrisDenton`
workingjubilee pushed a commit to tcdi/postgrestd that referenced this pull request Sep 15, 2022
Avoid needless buffer zeroing in `std::sys::windows::fs`

Followup to rust-lang/rust#101171 and rust-lang/rust#101193. This finishes up avoiding buffer zeroing pointed out in rust-lang/rust#100729 (comment) (thanks!)

r? `@ChrisDenton`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants