Add a dedicated debug-logging option to config.toml

[guswynn]

@Mark-Simulacrum and I were talking in zulip and we found that turning on debug/trace logging in rustc is fairly confusing, as it effectively depends on debug-assertions and is not documented as such. @Mark-Simulacrum mentioned that we should probably have a separate option for logging anyways.

this diff adds that, having the option follow debug-assertions (so everyone's existing config.toml should be fine) and if the option is false

to test I ran ./x.py test twice, once with debug-logging = false and once with debug-logging = true and made sure i only saw trace's when it was true

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Mark-Simulacrum]

Hm, so this brings up two interesting questions:

• Should we default to this being on? If you're doing a lot of rustc development, it can certainly be helpful to have debug logs, but it can also be a pretty annoying slowdown (IIRC, benchmarks show 5-10%).
• Show we tie to to the old way of turning it on, i.e., debug-assertions?

cc @jyn514, we might want an MCP or similar here to figure out what people think good defaults would be.

[jyn514]

I think it should start as off-by-default. There was a lot of discussion in #76588 about enabling logging and it was almost a 10% perf hit on some benchmarks: https://perf.rust-lang.org/compare.html?start=22e6099330cde0e7b1529774fe27874f8326de7a&end=ededceef5651050bda1a2058af270da498bb1333.

That said, I would love to get the perf impact low enough that this can be on by default (or even always on).

[jyn514]

That said, I would love to get the perf impact low enough that this can be on by default (or even always on).

One possible way to do this is to separate debug! from trace! and have that be opt-in instead. AFAIK most of the compiler uses debug! currently, but I separate the two in rustdoc and it's worked very well. In particular, collect_intra_doc_links prints ludicrous amounts of info with trace but only ~15 lines or so per module with debug: https://github.com/rust-lang/rust/blob/master/src/librustdoc/passes/collect_intra_doc_links.rs#L647

[guswynn]

As it is implemented now, it should be off by default, and on wit debug-assertions, which matched exactly the semantics of the current config.toml

[Mark-Simulacrum]

The implementation looks great here, left a few questions/concerns.

[Mark-Simulacrum]

I'm going to re-assign to @jyn514 who can hopefully drive the defaults discussion here, but r=me from a pure implementation perspective (with the comment nit fixed).

[jyn514]

r=me with documentation updated, I think any changes to the defaults should be separate from the implementation. Having this default to debug-assertions is a good idea though IMO :)

[jyn514]

@bors r=jyn514,Mark-Simulacrum

Thanks for working on this! I'll open a separate issue for turning on debug logging by default (#76588 (comment)), but that's a long-term project and shouldn't block this improvement.

[bors]

📌 Commit 0be66d7 has been approved by jyn514,Mark-Simulacrum

[guswynn]

@jyn514 I might see if I have time for some work on #76588 (comment)

[bors]

⌛ Testing commit 0be66d7 with merge 4e48010...

[rust-log-analyzer]

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: jyn514,Mark-Simulacrum
Pushing 4e48010 to master...