Skip to content

Add profiler to bootstrap command #143525

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

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Shourya742
Copy link
Contributor

This PR adds command profiling to the bootstrap command. It tracks the total execution time and records cache hits for each command. It also provides the ability to export execution result to a JSON file. Integrating this with Chrome tracing could further enhance observability.

r? @Kobzol

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 6, 2025
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thank you! I don't think that we'll need JSON output for now, I think that the main output should be some lightly formatted text that will be human interpretable.

@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-06-add-profiler branch from 2730ce9 to aeec9c4 Compare July 8, 2025 18:27
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Gonna try it tomorrow.

@rust-log-analyzer
Copy link
Collaborator

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] build_helper test:false 0.683
error: variables can be used directly in the `format!` string
   --> src/bootstrap/src/utils/exec.rs:132:25
    |
132 |                         writeln!(writer, "  - Cache hit at:   {:?}", timestamp).unwrap();
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
    = note: `-D clippy::uninlined-format-args` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::uninlined_format_args)]`
help: change this to
    |
132 -                         writeln!(writer, "  - Cache hit at:   {:?}", timestamp).unwrap();
132 +                         writeln!(writer, "  - Cache hit at:   {timestamp:?}").unwrap();
    |

error: variables can be used directly in the `format!` string
   --> src/bootstrap/src/utils/exec.rs:139:25
    |
139 | /                         writeln!(
140 | |                             writer,
141 | |                             "  - Executed at:    {:?}, duration: {:.2?}",
142 | |                             timestamp, duration
143 | |                         )
    | |_________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

error: variables can be used directly in the `format!` string
   --> src/bootstrap/src/utils/exec.rs:150:28
    |
150 |                 Some(d) => format!("{:.2?}", d),
    |                            ^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
help: change this to
    |
150 -                 Some(d) => format!("{:.2?}", d),
150 +                 Some(d) => format!("{d:.2?}"),
    |

[RUSTC-TIMING] bootstrap test:false 9.553
error: could not compile `bootstrap` (lib) due to 3 previous errors
Build completed unsuccessfully in 0:10:19

@@ -350,6 +351,7 @@ impl<'a> BootstrapCommand {
.map(|(k, v)| (k.to_os_string(), v.map(|val| val.to_os_string())))
.collect(),
cwd: command.get_current_dir().map(Path::to_path_buf),
short_cmd: command.format_short_cmd(),
Copy link
Member

Choose a reason for hiding this comment

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

Can you please instead implement FormatShortCmd on the fingerprint, and then use that implementation in the FomatShortCmd for the commands? To derive the short cmd from the fingerprint, instead of storing duplicated data in it. It is supposed to be a cache key, so I don't want to include additional human-readable data in it.

@@ -102,3 +102,4 @@ flake.lock
/default.nix

# Before adding new lines, see the comment at the top.
*.txt
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants