Skip to content

Conversation

@nnethercote
Copy link
Contributor

It produces output for summarize, flamegraph, and crox.

@nnethercote
Copy link
Contributor Author

@Mark-Simulacrum: I admit I don't quite understand how the existing -Zself-profile support works for the benchmark commands. Anyway, this adds support for the profile command.

It produces output for `summarize`, `flamegraph`, and `crox`.
@Mark-Simulacrum
Copy link
Member

How would you feel about not running the tools within perf, and instead expecting that folks will cd Zsp && crox or so?

I'm not opposed, just not sure if they're slow or not and if there's reasons why people wouldn't want to run all of them...

Other than that, I think it's likely fine to merge this. I don't think we need to match the pre-existing support for self-profile summaries coupled to perf stat; and the new code modulo my comment seems fine as well.

(Feel free to merge with my comment resolved).

@nnethercote
Copy link
Contributor Author

For the other profilers I've taken the approach of doing as much work for the user as possible, so they don't have to run any additional tools. (And "they" is usually "me"! :)

I haven't used the measureme tools much but they seem pretty speedy -- much faster than all the Valgrind tools, which easily incur slowdowns of 5-10x.

@nnethercote nnethercote merged commit 6c3af99 into rust-lang:master Feb 28, 2020
@nnethercote nnethercote deleted the support-self-profile branch February 28, 2020 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants