-
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
Always register a tracing subscriber, even if no env var is set #89623
Conversation
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 79eed33 with merge 7c00e2ea53db975b00c9c2eba31f0e522ec5c284... |
☀️ Try build successful - checks-actions |
1 similar comment
☀️ Try build successful - checks-actions |
Queued 7c00e2ea53db975b00c9c2eba31f0e522ec5c284 with parent ca8078d, future comparison URL. |
Why register a subscriber that won't emit anything? Are you trying to emit logs programmatically or something? |
Sorry, should have written something in the main post, even if I just wanted to gauge the effect of this. If nothing is specified, the log crate emits at least error! messages, tracing should probably, too, but if we skip the subscriber, it can't do that. This was discovered in #76824 |
Finished benchmarking commit (7c00e2ea53db975b00c9c2eba31f0e522ec5c284): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
r? rust-lang/compiler |
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.
looks good to me!
// Don't register a dispatcher if there's no filter to print anything | ||
match std::env::var(env) { | ||
Err(_) => return, | ||
Ok(s) if s.is_empty() => return, | ||
Ok(_) => {} | ||
} |
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.
if we wanted to have a way to completely disable logging, we could consider special-casing RUSTC_LOG=off
, instead of "no value", but, since the perf run indicates that the performance difference between "no subscriber" and "levels disabled" is negligable, it's probably not worth messing with...
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.
it may be more noticeable if debug assertions are turned on, but I have to test that locally. Since I often run with RUSTC_LOG and without, and don't notice something, it can't be too bad though.
As a side note, this does not actually fix #76824; only the |
Thanks for actually fixing it |
This commit changes the `tracing_subscriber` initialization in `rustc_driver` so that the `WARN` verbosity level is enabled by default when the `RUSTC_LOG` env variable is empty. If the `RUSTC_LOG` env variable is set, the filter string in the environment variable is honored, instead. Fixes rust-lang#76824 Closes rust-lang#89623 cc @eddyb, @oli-obk
rustc_driver: Enable the `WARN` log level by default This commit changes the `tracing_subscriber` initialization in `rustc_driver` so that the `WARN` verbosity level is enabled by default when the `RUSTC_LOG` env variable is empty. If the `RUSTC_LOG` env variable is set, the filter string in the environment variable is honored, instead. Fixes rust-lang#76824 Closes rust-lang#89623 cc `@eddyb,` `@oli-obk`
rustc_driver: Enable the `WARN` log level by default This commit changes the `tracing_subscriber` initialization in `rustc_driver` so that the `WARN` verbosity level is enabled by default when the `RUSTC_LOG` env variable is empty. If the `RUSTC_LOG` env variable is set, the filter string in the environment variable is honored, instead. Fixes rust-lang#76824 Closes rust-lang#89623 cc ``@eddyb,`` ``@oli-obk``
If nothing is specified, the log crate emits at least error! messages, tracing would, too, but if we skip the subscriber, it can't do that.
fixes #76824
cc @hawkw @eddyb