Add -Z time-passes-format to allow specifying a JSON output for -Z time-passes
#107718
Conversation
|
r? @jackh726 (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-query-system
Zoxc
force-pushed
the
z-time
branch
2 times, most recently
from
4191f45
to
58afd39
Compare
|
I'm going too r? @nnethercote here, since they removed this originally. Also going to cc @davidtwco as previous reviewer. |
| unsafety::check_item(tcx, impl_def_id); | ||
| tcx.ensure().orphan_check_impl(impl_def_id); |
There was a problem hiding this comment.
drive by question: why stop timing these ?
There was a problem hiding this comment.
They're done per item and are not passes.
| pub fn verbose_generic_activity_with_arg<A>( | ||
| /// Start profiling an extra verbose generic activity. Profiling continues until the | ||
| /// VerboseTimingGuard returned from this call is dropped. In addition to recording | ||
| /// a measureme event, "extra verbose" generic activities also print a timing entry to |
There was a problem hiding this comment.
| /// a measureme event, "extra verbose" generic activities also print a timing entry to | |
| /// a measurement event, "extra verbose" generic activities also print a timing entry to |
There was a problem hiding this comment.
measureme is the profiling library ;)
|
The commit log from the removal has some important details on problems with the old Also, to review this I need a better understanding of the motivation. Can you give examples of the new output, and how it compares with Removing |
|
My currently use case is mostly as output to Here's Here's |
nnethercote
added
S-waiting-on-author
|
I think changing |
|
Maybe |
|
☔ The latest upstream changes (presumably #108079) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Previously the output of |
|
☔ The latest upstream changes (presumably #108127) made this pull request unmergeable. Please resolve the merge conflicts. |
|
JSON would make sense. It might need the |
|
These commits modify the If this was intentional then you can ignore this comment. |
Zoxc
changed the title
-Z time as -Z time-precise-Z time-passes-format to allow specifying a JSON output for -Z time-passes
|
I've changed this to add a |
| ) | ||
| } | ||
|
|
||
| /// Like `verbose_generic_activity`, but `event_label` must be unique for a rustc session. |
There was a problem hiding this comment.
More detail in this comment would be good. How is uniqueness ensured -- must the caller guarantee it? What happens if it's not unique?
| @@ -705,15 +731,21 @@ impl<'a> TimingGuard<'a> { | |||
|
|
|||
| #[must_use] | |||
| pub struct VerboseTimingGuard<'a> { | |||
| start_and_message: Option<(Instant, Option<usize>, String)>, | |||
| start_and_message: Option<(Instant, Option<usize>, String, TimePassesFormat, bool)>, | |||
There was a problem hiding this comment.
With five elements, I think this tuple is now big enough that it should have its own type. That would allow some comments, particularly for the bool field. I see it's called unique, but at this point in the review it's not clear to me what that means.
There was a problem hiding this comment.
Also, is_unique might be a better name? Or a whole new 2-value enum might be nicer than just bool.
| ) { | ||
| if format == TimePassesFormat::Json { |
There was a problem hiding this comment.
It would be nice to have protection here against the addition of other formats. Either by adding assert_eq!(format, TimePassesFormat::Text) after the return, or by having a match here, which could fall through to the code below in the Text case.
|
Unique is false for |
|
@bors r+ |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#106964 (Clarify `Error::last_os_error` can be weird) - rust-lang#107718 (Add `-Z time-passes-format` to allow specifying a JSON output for `-Z time-passes`) - rust-lang#107880 (Lint ambiguous glob re-exports) - rust-lang#108549 (Remove issue number for `link_cfg`) - rust-lang#108588 (Fix the ffi_unwind_calls lint documentation) - rust-lang#109231 (Add `try_canonicalize` to `rustc_fs_util` and use it over `fs::canonicalize`) - rust-lang#109472 (Add parentheses properly for method calls) - rust-lang#109487 (Move useless_anynous_reexport lint into unused_imports) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This adds back the
-Z timeoption as that is useful for my rustc benchmark tool, reverting #102725. It now uses nanoseconds and bytes as the units so it is renamed totime-precise.