-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Only use LOCAL_{STDOUT,STDERR} when set_{print/panic} is used. #77264
Only use LOCAL_{STDOUT,STDERR} when set_{print/panic} is used. #77264
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
The thread local LOCAL_STDOUT and LOCAL_STDERR are only used by the test crate to capture output from tests when running them in the same process in differen threads. However, every program will check these variables on every print, even outside of testing. This involves allocating a thread local key, and registering a thread local destructor. This can be somewhat expensive. This change keeps a global flag (LOCAL_STREAMS) which will be set to true when either of these local streams is used. (So, effectively only in test and benchmark runs.) When this flag is off, these thread locals are not even looked at and therefore will not be initialized on the first output on every thread, which also means no thread local destructors will be registered.
d132cf6
to
de597fc
Compare
cc @fasterthanlime whose blogpost inspired this change |
@rustbot modify labels: +T-libs +A-io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
@bors r+ |
📌 Commit de597fc has been approved by |
You could use a thread-local |
@eddyb Considering that these LOCAL_* streams are only used in tests/benchmarks, I think a global flag makes sense here. In all regular programs, this flag will be permanently off. In tests/benchmarks, the local streams are used on pretty much every thread. And a global flag is efficient on all platforms, not only those supporting |
…as-schievink Rollup of 8 pull requests Successful merges: - rust-lang#75377 (Fix Debug implementations of some of the HashMap and BTreeMap iterator types) - rust-lang#76107 (Write manifest for MAJOR.MINOR channel to enable rustup convenience) - rust-lang#76745 (Move Wrapping<T> ui tests into library) - rust-lang#77182 (Add missing examples for Fd traits) - rust-lang#77251 (Bypass const_item_mutation if const's type has Drop impl) - rust-lang#77264 (Only use LOCAL_{STDOUT,STDERR} when set_{print/panic} is used. ) - rust-lang#77421 (Revert "resolve: Avoid "self-confirming" import resolutions in one more case") - rust-lang#77452 (Permit ty::Bool in const generics for v0 mangling) Failed merges: r? `@ghost`
The thread local
LOCAL_STDOUT
andLOCAL_STDERR
are only used by thetest
crate to capture output from tests when running them in the same process in differen threads. However, every program will check these variables on every print, even outside of testing.This involves allocating a thread local key, and registering a thread local destructor. This can be somewhat expensive.
This change keeps a global flag (
LOCAL_STREAMS
) which will be set totrue
when either of these local streams is used. (So, effectively only in test and benchmark runs.) When this flag is off, these thread locals are not even looked at and therefore will not be initialized on the first output on every thread, which also means no thread local destructors will be registered.Together with #77154, this should make output a little bit more efficient.