Add a `x perf` command for integrating bootstrap with `rustc-perf` #126318

Kobzol · 2024-06-12T10:49:40Z

This PR adds a new x perf command to bootstrap. The idea is to let rustc developers profile (profile_local) and benchmark (bench_local) a stage1/stage2 compiler directly from within rust.

Before, if you wanted to use rustc-perf, you had to clone it, set it up, copy the rustc sysroot after every change to rust etc. This is an attempt to automate that.

I opened this PR mostly for discussion. My idea is to offer an interface that looks something like this (a random sample of commands):

x perf --stage 2 profile eprintln x perf --stage1 profile cachegrind x perf benchmark --id baseline x perf benchmark --id after-edit x perf cmp baseline after-edit

In this PR, I'd like to only implement the simplest case (profile_local (eprintln)), because that only requires a single sysroot (you don't compare anything), and it's relatively easy to set up. Also, I'd like to avoid forcing developers to deal with the rustc-perf UI, so more complex use-cases (like benchmarking two sysroots and comparing the results) should probably wait for rust-lang/rustc-perf#1734 (which is hopefully coming along soon-ish).

I'm not sure if it's better to do this in bootstrap directly, or if I should create some shim tool that will receive a rustc sysroot, and offer a simplified CLI on top of rustc-perf.

Why is a separate CLI needed?

We definitely need to add some support to bootstrap to automate preparing rustc-perf and the rustc sysroot, but in theory after that we could just let people invoke rustc-perf manually. While that is definitely possible, you'd need to manually figure out where is your sysroot located, which seems annoying to me. The rustc-perf CLI is also relatively complex, and for this use-case it makes sense to only use a subset of it. So I thought that it would be better to offer a simplified interface on top of it that would make life easier for contributors. But maybe it's not worth it.

CC @onur-ozkan

rustbot · 2024-06-12T10:49:49Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 · 2024-06-12T10:49:49Z

⚠️ Warning ⚠️

These commits modify submodules.

Kobzol · 2024-06-12T10:51:02Z

Damn, forgot to set r? ghost. Well, then:

r? @onur-ozkan

onur-ozkan · 2024-06-12T12:04:18Z

Based on #126306 and #126312.

#126312 is already in the bors queue, you can revert that commit from this PR.

So that it is easier to use `rustc-perf` with `rustc` directly.

rustbot · 2024-06-12T19:06:42Z

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Kobzol · 2024-06-12T19:06:52Z

@rustbot ready

Kobzol · 2024-06-12T19:14:02Z

To clarify: this PR now hardcodes a single command (profile_local eprintln). I'd like to add more commands, but I'm not sure yet which approach to use to prepare the corresponding CLI.

Kobzol · 2024-06-14T07:44:27Z

I realized that having the CLI as a separate tool probably won't work (x run), because we would need to pass it paths from bootstrap, and I'm not sure if x run is prepared for that. So probably having the CLI directly in bootstrap is the easier solution.

onur-ozkan · 2024-06-14T07:58:15Z

I realized that having the CLI as a separate tool probably won't work (x run), because we would need to pass it paths from bootstrap, and I'm not sure if x run is prepared for that. So probably having the CLI directly in bootstrap is the easier solution.

The PR title says x perf. I thought we already implementing this directly in bootstrap without involving x run.

Kobzol · 2024-06-14T07:59:30Z

Yeah, I started with this implementation, since it seemed more natural, but as the PR description said, it was up for discussion :) But I think that indeed x perf is the better option.

onur-ozkan

I'm unsure how the rustc-perf tool works as I haven't used it locally. When I run x perf, I expect it to work, but it only creates empty files currently:

 $ echo -n | sha256sum e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 -  $ sha256sum build/rustc-perf/* e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Check-Full e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Check-IncrFull e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Check-IncrPatched0 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Check-IncrUnchanged e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Debug-Full e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Debug-IncrFull e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Debug-IncrPatched0 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Debug-IncrUnchanged e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Opt-Full e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Opt-IncrFull e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Opt-IncrPatched0 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-Opt-IncrUnchanged e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 build/rustc-perf/eprintln-Id-helloworld-tiny-Opt-Full

src/bootstrap/src/core/build_steps/tool.rs

src/bootstrap/src/core/build_steps/perf.rs

Kobzol · 2024-06-22T07:35:47Z

I'm unsure how the rustc-perf tool works as I haven't used it locally. When I run x perf, I expect it to work, but it only creates empty files currently:

This PR currently uses the eprintln profiler, which is the simplest one. It simply runs the compiler and stores all stderr into a file. Since the compiler didn't print anything to stderr, it is empty. Normally, you would modify the compiler, add some eprintln calls, then run the benchmark and check if some codepaths were taken, or how many times they were taken. This can be then combined with tools like counts to gather statistics. It is described here (you can search for eprintln).

@rustbot ready

onur-ozkan · 2024-06-22T13:14:50Z

@bors r+

bors · 2024-06-22T13:14:52Z

📌 Commit 0bd58d8 has been approved by onur-ozkan

It is now in the queue for this repository.

…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#126140 (Rename `std::fs::try_exists` to `std::fs::exists` and stabilize fs_try_exists) - rust-lang#126318 (Add a `x perf` command for integrating bootstrap with `rustc-perf`) - rust-lang#126552 (Remove use of const traits (and `feature(effects)`) from stdlib) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#126318 - Kobzol:bootstrap-perf, r=onur-ozkan Add a `x perf` command for integrating bootstrap with `rustc-perf` This PR adds a new `x perf` command to bootstrap. The idea is to let rustc developers profile (`profile_local`) and benchmark (`bench_local`) a stage1/stage2 compiler directly from within `rust`. Before, if you wanted to use `rustc-perf`, you had to clone it, set it up, copy the `rustc` sysroot after every change to `rust` etc. This is an attempt to automate that. I opened this PR mostly for discussion. My idea is to offer an interface that looks something like this (a random sample of commands): ```bash x perf --stage 2 profile eprintln x perf --stage1 profile cachegrind x perf benchmark --id baseline x perf benchmark --id after-edit x perf cmp baseline after-edit ``` In this PR, I'd like to only implement the simplest case (`profile_local (eprintln)`), because that only requires a single sysroot (you don't compare anything), and it's relatively easy to set up. Also, I'd like to avoid forcing developers to deal with the rustc-perf UI, so more complex use-cases (like benchmarking two sysroots and comparing the results) should probably wait for rust-lang/rustc-perf#1734 (which is hopefully coming along soon-ish). I'm not sure if it's better to do this in bootstrap directly, or if I should create some shim tool that will receive a `rustc` sysroot, and offer a simplified CLI on top of `rustc-perf`. ## Why is a separate CLI needed? We definitely need to add some support to bootstrap to automate preparing `rustc-perf` and the `rustc` sysroot, but in theory after that we could just let people invoke `rustc-perf` manually. While that is definitely possible, you'd need to manually figure out where is your sysroot located, which seems annoying to me. The `rustc-perf` CLI is also relatively complex, and for this use-case it makes sense to only use a subset of it. So I thought that it would be better to offer a simplified interface on top of it that would make life easier for contributors. But maybe it's not worth it. CC `@onur-ozkan`

…-ozkan Implement `x perf` as a separate tool Continues work from rust-lang#126318, adds a CLI for running `rustc-perf` profiling commands through a new `rustc-perf-wrapper` tool. The CLI is in a separate tool to enable experimentation outside of `bootstrap`. This is probably most of what we can do so far, I'll add support for benchmarking once `rustc-perf` gets a terminal output for comparing benchmark results. r? `@onur-ozkan`

…-ozkan Implement `x perf` as a separate tool Continues work from rust-lang#126318, adds a CLI for running `rustc-perf` profiling commands through a new `rustc-perf-wrapper` tool. The CLI is in a separate tool to enable experimentation outside of `bootstrap`. This is probably most of what we can do so far, I'll add support for benchmarking once `rustc-perf` gets a terminal output for comparing benchmark results. r? ``@onur-ozkan``

Rollup merge of rust-lang#127002 - Kobzol:bootstrap-perf-tool, r=onur-ozkan Implement `x perf` as a separate tool Continues work from rust-lang#126318, adds a CLI for running `rustc-perf` profiling commands through a new `rustc-perf-wrapper` tool. The CLI is in a separate tool to enable experimentation outside of `bootstrap`. This is probably most of what we can do so far, I'll add support for benchmarking once `rustc-perf` gets a terminal output for comparing benchmark results. r? ``@onur-ozkan``

Update Rust toolchain from nightly-2024-06-22 to nightly-2024-06-23 without any other source changes. This is an automatically generated pull request. If any of the CI checks fail, manual intervention is required. In such a case, review the changes at https://github.com/rust-lang/rust from rust-lang@c1b336c up to rust-lang@3cb521a. The log for this commit range is: rust-lang@3cb521a434 Auto merge of rust-lang#126761 - GuillaumeGomez:unsafe_extern_blocks, r=spastorino rust-lang@a0f01c3c10 Auto merge of rust-lang#126838 - matthiaskrgr:rollup-qkop22o, r=matthiaskrgr rust-lang@dc9a08f535 Rollup merge of rust-lang#126552 - fee1-dead-contrib:rmfx, r=compiler-errors rust-lang@162120b4fa Rollup merge of rust-lang#126318 - Kobzol:bootstrap-perf, r=onur-ozkan rust-lang@f3ced9d540 Rollup merge of rust-lang#126140 - eduardosm:stabilize-fs_try_exists, r=Amanieu rust-lang@f944afe380 Auto merge of rust-lang#116113 - kpreid:arcmut, r=dtolnay rust-lang@88c3db57e4 Generalize `{Rc,Arc}::make_mut()` to unsized types. rust-lang@a9a4830d25 Replace `WriteCloneIntoRaw` with `CloneToUninit`. rust-lang@ec201b8650 Add `core::clone::CloneToUninit`. rust-lang@81da6a6d40 Make `effects` an incomplete feature rust-lang@ac47dbad50 Auto merge of rust-lang#126824 - GuillaumeGomez:rollup-sybv8o7, r=GuillaumeGomez rust-lang@d265538016 Rollup merge of rust-lang#126823 - GuillaumeGomez:migrate-run-make-inline-always-many-cgu, r=Kobzol rust-lang@25bcc7d130 Rollup merge of rust-lang#126731 - Kobzol:bootstrap-cmd-refactor, r=onur-ozkan rust-lang@399c5cabdd Rollup merge of rust-lang#126723 - estebank:dot-dot-dot, r=Nadrieril rust-lang@3ed2cd74b5 Rollup merge of rust-lang#126686 - fmease:dump-preds-n-item-bounds, r=compiler-errors rust-lang@07e8b3ac01 Rollup merge of rust-lang#126555 - beetrees:f16-inline-asm-arm, r=Amanieu rust-lang@d03d6c0fea Auto merge of rust-lang#126750 - scottmcm:less-unlikely, r=jhpratt rust-lang@e7dfd4a913 Migrate `run-make/inline-always-many-cgu` to `rmake.rs` rust-lang@d9962bb4d8 Make `read_dir` method take a mutable callback rust-lang@f1b0d54ca9 Auto merge of rust-lang#126816 - weihanglo:update-cargo, r=weihanglo rust-lang@0bd58d8122 Apply review comments. rust-lang@250586cb2e Wrap std `Output` in `CommandOutput` rust-lang@f0aceed540 Auto merge of rust-lang#126817 - workingjubilee:rollup-0rg0k55, r=workingjubilee rust-lang@38bd7a0fcb Add `#[rustc_dump_{predicates,item_bounds}]` rust-lang@1916b3d57f Rollup merge of rust-lang#126811 - compiler-errors:tidy-ftl, r=estebank rust-lang@539090e5cd Rollup merge of rust-lang#126809 - estebank:wording-tweak, r=oli-obk rust-lang@b9ab6c3501 Rollup merge of rust-lang#126798 - miguelfrde:master, r=tmandry rust-lang@9498d5cf2f Rollup merge of rust-lang#126787 - Strophox:get-bytes, r=RalfJung rust-lang@1f9793f1aa Rollup merge of rust-lang#126722 - adwinwhite:ptr_fn_abi, r=celinval rust-lang@84b0922565 Rollup merge of rust-lang#126712 - Oneirical:bootest-constestllation, r=jieyouxu rust-lang@e7956cd994 Rollup merge of rust-lang#126530 - beetrees:f16-inline-asm-riscv, r=Amanieu rust-lang@10e1f5d212 Auto merge of rust-lang#124101 - the8472:pidfd-methods, r=cuviper rust-lang@2c65a24b8c Update cargo rust-lang@fcae62649e Auto merge of rust-lang#126758 - spastorino:avoid-safe-outside-unsafe-blocks, r=compiler-errors rust-lang@ffd72b1700 Fix remaining cases rust-lang@ea681ef281 Add a tidy rule to make sure that diagnostics don't end in periods rust-lang@8abf149bde to extract a pidfd we must consume the child rust-lang@0787c7308c Add PidFd::{kill, wait, try_wait} rust-lang@5d5892e966 Remove stray `.` from error message rust-lang@d94a40516e [fuchsia-test-runner] Remove usage of kw_only rust-lang@771e44ebd3 Add `f16` inline ASM support for RISC-V rust-lang@753fb070bb Add `f16` inline ASM support for 32-bit ARM rust-lang@22831ed117 Do not allow safe usafe on static and fn items rust-lang@a6a83d3d4e bless tests rust-lang@b512bf6f77 add as_ptr to trait AllocBytes, fix 2 impls; add pub fn get_bytes_unchecked_raw in allocation.rs; add pub fn get_alloc_bytes_unchecked_raw[_mut] in memory.rs rust-lang@02aaea1803 update intrinsic const param counting rust-lang@3b14b756d8 Remove `feature(effects)` from the standard library rust-lang@a314f7363a Stop using `unlikely` in `strict_*` methods rust-lang@225796a2df Add method to get `FnAbi` of function pointer rust-lang@630c3adb14 Add regression test for `unsafe_extern_blocks` rust-lang@bb9a3ef90c Implement `unsafe_extern_blocks` feature in rustdoc rust-lang@3c0a4bc915 rewrite crate-name-priority to rmake rust-lang@bc12972bcd Slightly refactor the dumping of HIR analysis data rust-lang@3fe4d134dd Appease `clippy` rust-lang@c15293407f Remove unused import rust-lang@5c4318d02c Implement `run_cmd` in terms of `run_tracked` rust-lang@0de7b92cc6 Remove `run_delaying_failure` rust-lang@e933cfb13c Remove `run_quiet_delaying_failure` rust-lang@949e667d3f Remove `run_quiet` rust-lang@a12f541a18 Implement new command execution logic rust-lang@9fd7784b97 Fix `...` in multline code-skips in suggestions rust-lang@f22b5afa6a rewrite error-writing-dependencies to rmake rust-lang@75ee1d74a9 rewrite relocation-model to rmake rust-lang@87d2e61428 Add `x perf` command for profiling the compiler using `rustc-perf` rust-lang@fd44aca2aa Copy `rustc-fake` binary when building the `rustc-perf` tool rust-lang@9e0b76201b Add `RustcPerf` bootstrap tool rust-lang@9ec178df0b Add `cargo_args` to `ToolBuild` rust-lang@6a04dfe78c Rename `std::fs::try_exists` to `std::fs::exists` and stabilize fs_try_exists Co-authored-by: celinval <35149715+celinval@users.noreply.github.com>

rustbot assigned Mark-Simulacrum Jun 12, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 12, 2024

rustbot assigned onur-ozkan and unassigned Mark-Simulacrum Jun 12, 2024

Kobzol added 3 commits June 12, 2024 20:21

Add cargo_args to ToolBuild

9ec178d

Add RustcPerf bootstrap tool

9e0b762

So that it is easier to use `rustc-perf` with `rustc` directly.

Copy rustc-fake binary when building the rustc-perf tool

fd44aca

onur-ozkan mentioned this pull request Jun 12, 2024

Add rustc-perf as a stage0 tool #126306

Closed

Add x perf command for profiling the compiler using rustc-perf

87d2e61

Kobzol force-pushed the bootstrap-perf branch from f51a04b to 87d2e61 Compare June 12, 2024 19:06

Kobzol marked this pull request as ready for review June 12, 2024 19:06

onur-ozkan reviewed Jun 22, 2024

View reviewed changes

src/bootstrap/src/core/build_steps/tool.rs Outdated Show resolved Hide resolved

src/bootstrap/src/core/build_steps/perf.rs Outdated Show resolved Hide resolved

onur-ozkan 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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2024

Apply review comments.

0bd58d8

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2024

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 Jun 22, 2024

matthiaskrgr mentioned this pull request Jun 22, 2024

Rollup of 3 pull requests #126838

Merged

bors merged commit 162120b into rust-lang:master Jun 22, 2024

rustbot added this to the 1.81.0 milestone Jun 22, 2024

Kobzol deleted the bootstrap-perf branch June 22, 2024 20:56

This was referenced Jun 23, 2024

Add more arguments to x perf #126853

Closed

Implement x perf as a separate tool #127002

Merged

Kobzol mentioned this pull request Jun 30, 2024

Document bootstrap integration with rustc-perf rust-lang/rustc-dev-guide#2005

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a `x perf` command for integrating bootstrap with `rustc-perf` #126318

Add a `x perf` command for integrating bootstrap with `rustc-perf` #126318

Uh oh!

Kobzol commented Jun 12, 2024 •

edited

Loading

rustbot commented Jun 12, 2024

rustbot commented Jun 12, 2024

Kobzol commented Jun 12, 2024

onur-ozkan commented Jun 12, 2024

rustbot commented Jun 12, 2024

Kobzol commented Jun 12, 2024

Kobzol commented Jun 12, 2024

Kobzol commented Jun 14, 2024

onur-ozkan commented Jun 14, 2024

Kobzol commented Jun 14, 2024

onur-ozkan left a comment

Uh oh!

Uh oh!

Kobzol commented Jun 22, 2024

onur-ozkan commented Jun 22, 2024

bors commented Jun 22, 2024

Labels

5 participants

Add a x perf command for integrating bootstrap with rustc-perf #126318

Add a x perf command for integrating bootstrap with rustc-perf #126318

Uh oh!

Conversation

Kobzol commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is a separate CLI needed?

rustbot commented Jun 12, 2024

rustbot commented Jun 12, 2024

Kobzol commented Jun 12, 2024

onur-ozkan commented Jun 12, 2024

rustbot commented Jun 12, 2024

Kobzol commented Jun 12, 2024

Kobzol commented Jun 12, 2024

Kobzol commented Jun 14, 2024

onur-ozkan commented Jun 14, 2024

Kobzol commented Jun 14, 2024

onur-ozkan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kobzol commented Jun 22, 2024

onur-ozkan commented Jun 22, 2024

bors commented Jun 22, 2024

Labels

5 participants

Add a `x perf` command for integrating bootstrap with `rustc-perf` #126318

Add a `x perf` command for integrating bootstrap with `rustc-perf` #126318

Kobzol commented Jun 12, 2024 •

edited

Loading