Skip to content

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Sep 4, 2025

What does this PR try to resolve?

This PR expands the support for --json-timings (added in #15780), by rendering the individual compilation sections in the pipeline graph of the --timings page.

Before, the linking section was only shown in the table at the bottom of the page, now it should also be clearly visible in the compilation graph, which should help more quickly understand how much time is spent in linking.

image

I also added a legend to the pipeline graph, to explain what do the colors mean.
image

One wart is that the linking time actually ends a bit before the unit ends, so there is some "vacuum" at the end where rustc does cleanup, persists files to disk, deallocates things, etc. That's why I marked the blue section "Frontend/rest" in the legend.

How to test and review this PR?

Same as for #15780, e.g.:

export RUSTC=`rustup +nightly which rustc`
target/debug/cargo build -Zsection-timings --timings

on some crate, e.g. ripgrep.

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-timings Area: timings S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2025
@Kobzol Kobzol changed the title Cargo timings linker Render individual compilation sections in --timings pipeline graph Sep 4, 2025
@epage
Copy link
Contributor

epage commented Sep 4, 2025

One wart is that the linking time actually ends a bit before the unit ends, so there is some "vacuum" at the end where rustc does cleanup, persists files to disk, deallocates things, etc. That's why I marked the blue section "Frontend/rest" in the legend.

Should we just have this as a separate "Other" category?

@Kobzol
Copy link
Member Author

Kobzol commented Sep 4, 2025

Could be. We could create an artificial "Other" section, which would be from the end of the last known section to the end of the compilation of the unit (as long as this duration is non-empty, of course). Does that work?

@epage
Copy link
Contributor

epage commented Sep 4, 2025

Could be. We could create an artificial "Other" section, which would be from the end of the last known section to the end of the compilation of the unit (as long as this duration is non-empty, of course). Does that work?

Works for me!

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

This is nice. Do you have a sense of when the rustc flag will be stabilized?

@Kobzol
Copy link
Member Author

Kobzol commented Sep 5, 2025

I mean, I could propose stabilization immediately, but I'm not sure if it's a good idea yet. For example, @bjorn3 is currently doing some changes of how LTO works, and while doing that, we found out that you can have multiple codegen + linking sections during a single rustc invocation. That kind of breaks the assumption that --timings makes (even without taking sections into account).

@Kobzol Kobzol force-pushed the cargo-timings-linker branch from eb9a5f6 to 80912b6 Compare September 5, 2025 15:33
@Kobzol
Copy link
Member Author

Kobzol commented Sep 5, 2025

Fixed the missing else and added the "other" section. I can imagine improving this page further (e.g. adding a tooltip on hover on a crate that would show the names and durations of the individual sections), but it's kinda bothersome with the raw JavaScript and the canvas :/

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Thanks!

@epage epage enabled auto-merge September 5, 2025 15:37
@epage epage added this pull request to the merge queue Sep 5, 2025
Merged via the queue into rust-lang:master with commit fa10d65 Sep 5, 2025
25 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2025
@Kobzol Kobzol deleted the cargo-timings-linker branch September 5, 2025 17:19
Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

This is very nice!

bors added a commit to rust-lang/rust that referenced this pull request Sep 10, 2025
Update cargo submodule

10 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..98402ac7a41dd0f564d3e56d58180d325d0417a0
2025-09-04 01:25:01 +0000 to 2025-09-09 20:19:39 +0000
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)

r? ghost
bors added a commit to rust-lang/rust that referenced this pull request Sep 11, 2025
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
@rustbot rustbot added this to the 1.91.0 milestone Sep 11, 2025
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 12, 2025
Update cargo submodule

13 commits in 761c4658d0079d607e6d33cf0c060e61a617cad3..24bb93c388fb8c211a37986539f24a819dc669d3
2025-09-04 01:25:01 +0000 to 2025-09-10 23:16:07 +0000
- Bump miow to 0.60.1 (rust-lang/cargo#15950)
- test(help): Ensure consistent behavior regardless of rustup use (rust-lang/cargo#15949)
- docs(changelog): Clarify how manifest paths are used (rust-lang/cargo#15946)
- fix(flock): check if they are marked unsupported in libstd (rust-lang/cargo#15941)
- test(manifest): Fix test output order (rust-lang/cargo#15940)
- refactor(shell): Simplify some code (rust-lang/cargo#15937)
- fix(manifest): Report script manifest errors for the right line number (rust-lang/cargo#15927)
- refactor: replace flock with std flock (rust-lang/cargo#15935)
- fix(cli): Adjust messages to match rustc  (rust-lang/cargo#15928)
- fix: Switch from --nocapture to --no-capture (rust-lang/cargo#15930)
- Render individual compilation sections in `--timings` pipeline graph (rust-lang/cargo#15923)
- test(credential): Switch more expected results to snapshots (rust-lang/cargo#15929)
- refactor(cli): Pull out error chain iteration (rust-lang/cargo#15926)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-timings Area: timings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants