timbunce / devel-nytprof

Devel::NYTProf is a powerful feature-rich source code profiler for Perl. (Mostly in maintenance mode, so PRs are much more likely to be acted upon than Issues.)
http://blog.timbunce.org/tag/nytprof/
67 stars 51 forks source link

For version 6.07 -- assuming we're including bin/flamegraph.pl changes #150

Closed jkeenan closed 3 years ago

jkeenan commented 3 years ago

@timbunce, this p.r. is based on the assumption that we're releasing the Mar 18 2019 changes to bin/flamegraph.pl as part of CPAN 6.07.

Please let me know whether this is okay, as I cannot myself vouch for them. If not, I will revert that commit prior to CPAN release.

Thank you very much. Jim Keenan

jkeenan commented 3 years ago

Per correspondence with @timbunce, am closing this p.r. We'll set aside the changes to bin/flamegraph.pl until they're better vetted. The fix to https://github.com/timbunce/devel-nytprof/issues/143 is more urgent.

Thank you very much. JIm Keenan

versable commented 3 years ago

@jkeenan Changes are correct.

Relevant upstream MR: https://github.com/brendangregg/FlameGraph/pull/189

Most relevant change is in commit https://github.com/brendangregg/FlameGraph/commit/d4d085cf46b8e9eb89aab36419424270dfa787d6 which allows to ctrl+click to zoom in the Flamegraph.

jkeenan commented 3 years ago

On 4/6/21 9:56 AM, Michael Langfermann wrote:

@jkeenan https://github.com/jkeenan Changes are correct.

Relevant upstream MR: brendangregg/FlameGraph#189 https://github.com/brendangregg/FlameGraph/pull/189

Most relevant change is in commit @.*** https://github.com/brendangregg/FlameGraph/commit/d4d085cf46b8e9eb89aab36419424270dfa787d6 which allows to ctrl+click to zoom in the Flamegraph.

Thank you very much for re-inspecting this. However, since I needed to get 6.07 out the door (and following consultation with Tim), I reverted the commit and will reinsert it once I get a chance to study flamegraph more. I will keep you updated on this.

Thanks again. Jim Keenan

versable commented 3 years ago

On 4/6/21 9:56 AM, Michael Langfermann wrote: @jkeenan https://github.com/jkeenan Changes are correct. Relevant upstream MR: brendangregg/FlameGraph#189 <brendangregg/FlameGraph#189> Most relevant change is in commit @.*** [brendangregg/FlameGraph@d4d085c](https://github.com/brendangregg/FlameGraph/commit/d4d085cf46b8e9eb89aab36419424270dfa787d6) which allows to ctrl+click to zoom in the Flamegraph. Thank you very much for re-inspecting this. However, since I needed to get 6.07 out the door (and following consultation with Tim), I reverted the commit and will reinsert it once I get a chance to study flamegraph more. I will keep you updated on this. Thanks again. Jim Keenan

Thanks for the heads-up, I'd suggest taking a look at http://www.brendangregg.com/flamegraphs.html.

Thanks,

Mike