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

Remove -Ztime #102725

Merged
merged 3 commits into from
Oct 6, 2022
Merged

Remove -Ztime #102725

merged 3 commits into from
Oct 6, 2022

Conversation

nnethercote
Copy link
Contributor

Because it has a lot of overlap with -Ztime-passes but is generally less useful. Plus some related cleanups.

Best reviewed one commit at a time.

r? @davidtwco

- It's `--print`, not `--prints`.
- `-Ztime` and `-Ztime-passes` print to stderr, not stdout.
The compiler currently has `-Ztime` and `-Ztime-passes`. I've used
`-Ztime-passes` for years but only recently learned about `-Ztime`.

What's the difference? Let's look at the `-Zhelp` output:
```
  -Z        time=val -- measure time of rustc processes (default: no)
  -Z time-passes=val -- measure time of each rustc pass (default: no)
```
The `-Ztime-passes` description is clear, but the `-Ztime` one is less so.
Sounds like it measures the time for the entire process?

No. The real difference is that `-Ztime-passes` prints out info about passes,
and `-Ztime` does the same, but only for a subset of those passes. More
specifically, there is a distinction in the profiling code between a "verbose
generic activity" and an "extra verbose generic activity". `-Ztime-passes`
prints both kinds, while `-Ztime` only prints the first one. (It took me
a close reading of the source code to determine this difference.)

In practice this distinction has low value. Perhaps in the past the "extra
verbose" output was more voluminous, but now that we only print stats for a
pass if it exceeds 5ms or alters the RSS, `-Ztime-passes` is less spammy. Also,
a lot of the "extra verbose" cases are for individual lint passes, and you need
to also use `-Zno-interleave-lints` to see those anyway.

Therefore, this commit removes `-Ztime` and the associated machinery. One thing
to note is that the existing "extra verbose" activities all have an extra
string argument, so the commit adds the ability to accept an extra argument to
the "verbose" activities.
`print_time_passes_entry` unconditionally prints data about a pass. The
most commonly used call site, in `VerboseTimingGuard::drop`, guards it
with a `should_print_passes` test. But there are a couple of other call
sites that don't do that test.

This commit moves the `should_print_passes` test within
`print_time_passes_entry` so that all passes are treated equally.
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 6, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2022
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 6, 2022

📌 Commit 4e8faff has been approved by davidtwco

It is now in the queue for this repository.

@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 Oct 6, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 6, 2022
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#98496 (make `compare_const_impl` a query and use it in `instance.rs`)
 - rust-lang#102680 (Fix overconstrained Send impls in btree internals)
 - rust-lang#102718 (Fix `opaque_hidden_inferred_bound` lint ICE)
 - rust-lang#102725 (Remove `-Ztime`)
 - rust-lang#102736 (Migrate search input color to CSS variable)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@nnethercote
Copy link
Contributor Author

@bors rollup=always

@bors bors merged commit 42df0a5 into rust-lang:master Oct 6, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 6, 2022
@nnethercote nnethercote deleted the rm-Z-time branch October 6, 2022 23:26
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 23, 2023
Add `-Z time-passes-format` to allow specifying a JSON output for `-Z time-passes`

This adds back the `-Z time` option as that is useful for [my rustc benchmark tool](https://github.com/Zoxc/rcb), reverting rust-lang#102725. It now uses nanoseconds and bytes as the units so it is renamed to `time-precise`.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Mar 24, 2023
Add `-Z time-passes-format` to allow specifying a JSON output for `-Z time-passes`

This adds back the `-Z time` option as that is useful for [my rustc benchmark tool](https://github.com/Zoxc/rcb), reverting rust-lang/rust#102725. It now uses nanoseconds and bytes as the units so it is renamed to `time-precise`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants