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 -Z time-passes and error with "-Z self-profile should be used instead". #53631

Closed
3 tasks
eddyb opened this issue Aug 23, 2018 · 18 comments
Closed
3 tasks
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler A-self-profile Area: Self-profiling feature of the compiler C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Aug 23, 2018

Remaining issues with -Z self-profile:

cc @steveklabnik @wesleywiser @alexcrichton @rust-lang/compiler

@eddyb eddyb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-driver Area: rustc_driver that ties everything together into the `rustc` compiler I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 23, 2018
@nikomatsakis
Copy link
Contributor

I'm in favor!

@Zoxc
Copy link
Contributor

Zoxc commented Aug 23, 2018

I'm against. I use -Z time-passes all the time when doing any performance work.

@bjorn3
Copy link
Member

bjorn3 commented Aug 23, 2018

-Z self-profile provides a much less detailed output

@kennytm
Copy link
Member

kennytm commented Aug 23, 2018

I'm mildly against if the replacement can't print some compilation progress at real time.

@eddyb
Copy link
Member Author

eddyb commented Aug 23, 2018

@bjorn3 How so, does it not have a per-query breakdown? I'll shamefully admit I opened this issue away from my laptop when I remembered -Z self-profile landed, and I haven't played with it yet.

Either way, we should track the remaining problems with -Z self-profile in this issue, and remove -Z time-passes when they're resolved.

@Zoxc also mentioned on IRC that it doesn't work when there are errors.

@kennytm Ah, great point, I had forgotten. Would #53630 solve the "real time" problem for you?
I'd add that maybe we should catch Ctrl+C and also print the profile summary then too.

EDIT: I've added these to the issue description.

@kennytm
Copy link
Member

kennytm commented Aug 23, 2018

@eddyb works for me 😃

@wesleywiser
Copy link
Member

@eddyb We don't have per-query breakdown yet, just broad level categorization. Per-query is the next thing I plan on working on.

@nagisa
Copy link
Member

nagisa commented Aug 23, 2018

I want some way to output various "events" pertaining to the query system (query X start, query X end, query X executes query Y, etc) in whatever format, as they occur. Preferably the format would target some tooling that already exists, but I cannot really advise on anything in particular at the moment. See this comment for examples of what I’m talking about.

This could possibly be the solution for the granularity problem. I can say from experience, that tracing such events is definitely not as expensive as it may sound and usually introduces overhead in proximity of tens to hundreds of ns per event.

@eddyb
Copy link
Member Author

eddyb commented Aug 23, 2018

@nagisa I think we already have something similar because of dep graph tracking, but @michaelwoerister knows more than me here (I remember seeing event-collecting threads).

@nagisa
Copy link
Member

nagisa commented Aug 25, 2018

Looking into webrender I noticed it uses thread_profiler, which outputs traces in Trace Event Format. Links lead to catapult, which upon the first blush seems similar to the tools I referenced before.

@nikomatsakis
Copy link
Contributor

That is roughly what the old stuff did, iirc. I think we mostly ripped that out. I like the idea emitting events, at least in principle. I also like the idea of per-query profiling. I also like the idea of removing time-passes =)

It seems like we can fix things to emit a result in the case of error without great difficulty? Mostly a matter of using a Drop impl or something, no?

@nikomatsakis
Copy link
Contributor

Ah I see @eddyb already made a check-list, 👍 from me. I'd be ok with removing -Ztime-passes earlier too but I don't mind waiting.

@FrankSD
Copy link

FrankSD commented Apr 1, 2019

Anyone is working on this?

@wesleywiser wesleywiser added the A-self-profile Area: Self-profiling feature of the compiler label Apr 1, 2019
@wesleywiser
Copy link
Member

Hi @FrankSD! The self-profile feature has a fair bit of churn going on right now. Before you decide to work on this, I'd recommend stopping by the t-compiler/wg-self-profile channel on Zulip.

@jyn514
Copy link
Member

jyn514 commented Mar 8, 2021

I don't think we should remove time-passes until self-profile measures memory usage: #81348

@john-h-k
Copy link
Contributor

Is this still a desired thing?

@jyn514
Copy link
Member

jyn514 commented Apr 27, 2023

I would rather not make this change. -Z time-passes is nice because you can get feedback while compilation is still ongoing, without having to wait for it to finish. It's less accurate but it's also a lot nicer to use.

@fee1-dead
Copy link
Member

fee1-dead commented Jul 1, 2023

I'm going to close this issue based on the comments above since it appears that no further input is likely.

@fee1-dead fee1-dead closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler A-self-profile Area: Self-profiling feature of the compiler C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests