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.

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

Use debug log level for developer oriented logs #82029

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Feb 12, 2021

The information logged here is of limited general interest, while at the
same times makes it impractical to simply enable logging and share the
resulting logs due to the amount of the output produced.

Reduce log level from info to debug for developer oriented information.

For example, when building cargo, this reduces the amount of logs
generated by RUSTC_LOG=info cargo build from 265 MB to 79 MB.

Continuation of changes from 81350.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 12, 2021
@rust-log-analyzer

This comment has been minimized.

@tmiasko
Copy link
Contributor Author

tmiasko commented Feb 12, 2021

cc @Aaron1011, @Nadrieril, @oli-obk in the case you would like to keep the existing log level for some of those.

@Aaron1011
Copy link
Member

Looks good to me!

The information logged here is of limited general interest, while at the
same times makes it impractical to simply enable logging and share the
resulting logs due to the amount of the output produced.

Reduce log level from info to debug for developer oriented information.

For example, when building cargo, this reduces the amount of logs
generated by `RUSTC_LOG=info cargo build` from 265 MB to 79 MB.

Continuation of changes from 81350.
@oli-obk
Copy link
Contributor

oli-obk commented Feb 13, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 13, 2021

📌 Commit 06381ea660af2fe18dd62317f43df0c3b4860e61 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Feb 13, 2021

cc @RalfJung the logging levels for the miri-engine got lowered from info! to debug!

@RalfJung
Copy link
Member

Which ones? I think the logging that says which MIR statement will be executed should remain at info!. I do use this for debugging fairly regularly, and having to build a local rustc just for that would be quite painful.

@@ -77,7 +77,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Runs the interpretation logic for the given `mir::Statement` at the current frame and
/// statement counter. This also moves the statement counter forward.
crate fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> {
info!("{:?}", stmt);
Copy link
Member

@RalfJung RalfJung Feb 13, 2021

Choose a reason for hiding this comment

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

I would really like to keep this (and the other Miri changes here) at info! so that this level of logging is available for regular rustc builds.

Well really, I don't care what level it is at, as long as I can make rustc emit this without having to build a rustc with debug assertions enabled. AFAIK debug! and trace! are compiled out entirely for release builds, so info! is the lowest it can be at for that.

@RalfJung
Copy link
Member

@bors r-
I'd like to discuss this a bit more; this change would be quite painful for some of the debugging I do.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 13, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Feb 13, 2021

we could lower the logging level cutoff for release mode to debug and run that through perf.

@RalfJung
Copy link
Member

That would resolve my concern.

Or maybe there's a way to sidestep the debug!-removal for just these few cases (by using something more verbose than debug!)?

@tmiasko
Copy link
Contributor Author

tmiasko commented Feb 13, 2021

Reverted changes to interpreter. Please take another look.

@RalfJung
Copy link
Member

RalfJung commented Feb 13, 2021

I have no stake in the remaining changes and thus no objection.

However, I do concede that emitting a log line per CTFE instruction can be quite verbose. I was not aware that RUSTC_LOG=info is something people do (I only ever enable logging in select parts of rustc). So it would still be good to find some compromise that both makes the info log something one can work with (or some other log level suited for this purpose), and enables debugging of CTFE/Miri issues without having to do a local rustc build.

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 13, 2021

📌 Commit 361dcd5 has been approved by matthewjasper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 13, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#80523 (#[doc(inline)] sym_generated)
 - rust-lang#80920 (Visit more targets when validating attributes)
 - rust-lang#81720 (Updated smallvec version due to RUSTSEC-2021-0003)
 - rust-lang#81891 ([rustdoc-json] Make `header` a vec of modifiers, and FunctionPointer consistent)
 - rust-lang#81912 (Implement the precise analysis pass for lint `disjoint_capture_drop_reorder`)
 - rust-lang#81914 (Fixing bad suggestion for `_` in `const` type when a function rust-lang#81885)
 - rust-lang#81919 (BTreeMap: fix internal comments)
 - rust-lang#81927 (Add a regression test for rust-lang#32498)
 - rust-lang#81965 (Fix MIR pretty printer for non-local DefIds)
 - rust-lang#82029 (Use debug log level for developer oriented logs)
 - rust-lang#82056 (fix ice (rust-lang#82032))

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c8dacf9 into rust-lang:master Feb 15, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 15, 2021
@tmiasko tmiasko deleted the debug branch February 15, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants