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.

Sign up for GitHub

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

Don't print thread ids and names in tracing logs #78933

Merged
merged 2 commits into from
Nov 12, 2020
Merged

Don't print thread ids and names in tracing logs #78933

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cf8a49
Don't print thread ids and names in `tracing` logs
jyn514 Nov 10, 2020
6e9ed8b
Enable thread names in debug logging only for `parallel_compiler`
jyn514 Nov 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_driver/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,12 +1288,12 @@ pub fn init_env_logger(env: &str) {
.with_indent_lines(true)
.with_ansi(true)
.with_targets(true)
.with_thread_ids(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I did turn these on on purpose, because in a parallel rustc we'll be unable to pick apart the tracing messages otherwise. One alternative would be to only turn them on if there's a RUSTC_LOG_THREADS env var set. So by default we would not emit these for now. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way to tell if the parallel compiler is enabled? Maybe with a cfg? It would be nice for this to be enabled/disabled automatically.

Copy link
Contributor

@oli-obk oli-obk Nov 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

it's a command line flag, which we don't have access to at this stage. I think we can cross that bridge when parallel compiling and debugging parallel compilers becomes more common.

Copy link
Member Author

Choose a reason for hiding this comment

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

@oli-obk are you sure? I see a lot of cfg(parallel_compiler) around the code base - do you have to both build the parallel compiler and opt into it with a command line flag?

Copy link
Member

Choose a reason for hiding this comment

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

There's two bits of parallel compiler: query-level support in compiled code (e.g. using a mutex), and separately how many threads we actually end up using (-Zthreads). The latter only has any effect if the first is enabled, which is indeed cfg paralllel_compiler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, it sounds like a) the question only matters when parallel_compiler is set, and then separately b) it might be nice to disable thread-based logging when cfg(parallel_compiler) is set but -Z threads isn't passed. I implemented a) and I think I'll leave b) for when parallel_compiler is better maintained - how does that sound?

.with_thread_names(true)
.with_wraparound(10)
.with_verbose_exit(true)
.with_verbose_entry(true)
.with_indent_amount(2);
#[cfg(parallel_compiler)]
let layer = layer.with_thread_ids(true).with_thread_names(true);

use tracing_subscriber::layer::SubscriberExt;
let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer);
Expand Down