-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add initial integration for --json=timings
behing -Zsection-timings
#15780
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
Conversation
r? @weihanglo rustbot has assigned @weihanglo. Use |
src/cargo/core/compiler/timings.rs
Outdated
@@ -64,8 +64,9 @@ pub struct Timings<'gctx> { | |||
cpu_usage: Vec<(f64, f64)>, | |||
} | |||
|
|||
/// Section of compilation. | |||
struct TimingSection { |
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.
Mind moving the rename into the commit that added it so this commit can be focused just on json output?
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.
Why was this marked as resolved? I'm not seeing it resolved as requested nor an explanation for why it shouldn't happen
Ok, so based on feedback I'm fine with moving this to a fully dynamic approach. Before I do that though, do you have any suggestions on how to detect whether the timings flag should be passed to Cargo? Is there any precedent for doing something in Cargo only on the nightly channel, or more precisely only with nightly rustc? Or would you prefer that we add an unstable Cargo flag for this? |
We discussed this in today's cargo team meeting. We should not enable an unstable rustc feature implicitly but require an explicit opt-in.
|
Fair enough, I'll add an explicit feature flag then. It's true that if it was implicit, if the rustc format changes in the future, it could break nightly cargo users of |
--json=timings
--json=timings
behing -Zsection-timings
Ok, I rewrote the PR and updated the PR description. It is now gated behind an unstable |
Restructured the commits |
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.
Thanks!
What does this PR try to resolve?
This PR adds initial support into Cargo for JSON timing sections, implemented in rustc in rust-lang/rust#142123. This allows Cargo to read frontend/codegen/linking time from rustc, and thus reporting slightly more detailed data in the
cargo build --timings
output.The PR modifies Cargo to tell rustc to emit the section messages (
--json=...,timings
), and it adds the section timings data to the HTML table output and the JSON output. It does not yet integration different sections in the HTML unit chart (I want to do that as a follow-up).Note that the JSON timings are currently only supported on the nightly compiler (they are not stabilized). The new behavior is thus gated behing an unstable Cargo flag (
-Zsection-timings
). When the flag is unused, the HTML table should look more or less the same as before, just that the code now supports both options.How to test and review this PR?
You can run e.g. this to generate the timing report with a nightly compiler:
on some crate, e.g. ripgrep.
Tracking issue: #15817