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

Render icicle graph in "Flame Graph" view #367

Merged
merged 3 commits into from
Apr 29, 2018
Merged

Conversation

narqo
Copy link
Contributor

@narqo narqo commented Apr 27, 2018

This PR changes Flame Graph to be rendered as "icicle graph" (inverted flame graph).

The inverted option was introduced in d3_flame_graph.js 2.x, so the component was updated to current latest 2.0.0-alpha4.

In addition to that, some visual changes were done:

  • Graph details were moved above the graph.
  • Nodes are labelled with package name instead of full package path, that is <package>.<func>. Graph details bar still shows full package path <github.com/owner/package>.<func>.
  • d3-tooltip has been disabled since the data it showed is always visible above the graph.

screen shot 2018-04-28 at 01 53 11

Fixes #359

@narqo narqo force-pushed the icicle-graph branch 3 times, most recently from b47cfba to 1ffe40c Compare April 28, 2018 17:21
</div>
{{template "script" .}}
<script>viewer(new URL(window.location.href), {{.Nodes}});</script>
<script>{{template "d3script" .}}</script>
<script>{{template "d3tipscript" .}}</script>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this isn't used now, remove d3tip from third_party?

@@ -0,0 +1,44 @@
#!/usr/bin/env bash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to only update the flamegraph directory, so maybe name the file as flamegraph_update.sh or something like that?

@aalexand
Copy link
Collaborator

@narqo Left a couple comments, other than that looks good - thanks a lot for doing this!

Add third_party_update.sh as a helper script for updating
d3flamegraph third_party library.
Update d3_flame_graph to current latest 2.x.
@narqo
Copy link
Contributor Author

narqo commented Apr 29, 2018

Addressed the comments and updated the PR. Please take another look.

@codecov-io
Copy link

Codecov Report

Merging #367 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
+ Coverage   66.64%   66.68%   +0.04%     
==========================================
  Files          36       36              
  Lines        7448     7457       +9     
==========================================
+ Hits         4964     4973       +9     
  Misses       2082     2082              
  Partials      402      402
Impacted Files Coverage Δ
internal/driver/webhtml.go 100% <100%> (ø) ⬆️
internal/driver/flamegraph.go 89.09% <100%> (+3.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1a3fd8...78f6879. Read the comment docs.

@aalexand aalexand merged commit be265fa into google:master Apr 29, 2018
KISSMonX pushed a commit to KISSMonX/pprof that referenced this pull request Jun 14, 2018
* 'master' of github.com:google/pprof: (32 commits)
  record diff base profile label key/value constant (google#384)
  apply additional command overrides based on report format (google#381)
  profile: fix legacy format Go heap profile parsing (google#382)
  add -diff flag for better profile comparision (google#369)
  Use -u flag in pprof installation command so that it updates if needed. (google#376)
  Add GetBase support for ASLR kernel mappings (google#371)
  Add show_from profile filter. (google#372)
  Update README.md due to 8dff45d (google#375)
  Remove stale docs, move useful ones. (google#374)
  internal/driver: skip tests requiring tcp on js (google#373)
  Add "trim path" option which can be used to relocate sources. (google#366)
  Move update_d3flamegraph.sh script from the root directory. (google#370)
  Skip unsymbolizable mapping during symbolz pass. (google#368)
  remove -positive_percentages flag (google#365)
  Render icicle graph in "Flame Graph" view (google#367)
  Add command-line editing support for interactive pprof (google#362)
  Improve profile filter unit tests, fix show filtering of mappings (google#354)
  Fake mappings should be merged into a single one during merging (google#357)
  Hack the code so that both existing Go versions and current Go master format it the same (google#358)
  moved filter tests into their own test file which matches the implementation file, filter.go. (google#356)
  ...
gmarin13 pushed a commit to gmarin13/pprof that referenced this pull request Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants