Windows: cache the stdio write mode for the duration of a lock session - #159562
Windows: cache the stdio write mode for the duration of a lock session#159562Joel-Wwalker wants to merge 2 commits into
Conversation
Every write to stdout/stderr re-queried GetStdHandle and GetConsoleMode (and GetConsoleOutputCP for consoles) to decide how to write, which dominates bulk writes through a locked handle. Cache the handle and the console verdict in the sys-level Stdout/Stderr and re-query when a new lock session begins, so SetStdHandle calls made between lock sessions keep working.
rustbot
added
S-waiting-on-review
|
r? @aapoalas rustbot has assigned @aapoalas. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| // stream state at the next write. Skipped if the cell is already | ||
| // borrowed (a nested lock during an ongoing write); such a lock is | ||
| // part of the outer session anyway. | ||
| if let Ok(mut w) = lock.inner.try_borrow_mut() { |
There was a problem hiding this comment.
issue: Does the inner try-borrow-mut optimise out on Linux and other targets where the refresh is a no-op?
There was a problem hiding this comment.
Probably, since after inlining the whole thing is just borrow-flag manipulation with no observable effect, but I did not want to rely on that. Gated the block with #[cfg(windows)] at the call site instead (and made the refresh impls windows-only), so on other targets lock() compiles to exactly the pre-PR code. Checked the change against x86_64-unknown-linux-gnu as well as Windows.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
On targets where refresh is a no-op, lock() now compiles to exactly the code it had before, instead of relying on the borrow-flag dance optimizing out.
|
@rustbot ready |
rustbot
added
S-waiting-on-review
3 participants
On Windows, every write to stdout/stderr calls
GetStdHandleandGetConsoleMode(andGetConsoleOutputCPfor consoles) to decide how to write. For bulk writes through a locked handle that is most of the work; #154071 measured 14% of CPU time inis_consolealone.This caches the handle and the console verdict in the sys-level
Stdout/Stderrand re-queries when a new lock session begins, as suggested in the issue. Holding aStdoutLockpins the stream; unlocked writes acquire the lock per call, so they re-query per write exactly as before, andSetStdHandlecalls made between lock sessions keep working (the #40490 behavior is preserved).Numbers from the issue's workload shape (16 KiB chunks to
NUL, 4 GiB total, Windows 11):stdout().lock()The unlocked case also improves because one implicit lock session can contain several sys-level writes when
LineWritersplits a chunk at newlines.The console path cannot run in CI, so it was verified under a pseudoconsole: several writes through one lock session on a CP437 console came through
WriteConsoleWintact (non-ASCII included), and afterSetConsoleOutputCPplus a new lock session the refresh picked up the change.The
refreshhook is a small#[cfg(windows)]shim onStdoutRaw/StderrRaw; happy to change it to a method on every platform's sys type instead if that is preferred.Fixes #154071