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

Add TimerOutputs.to_flamegraph(to) to export a FlameGraph Profile #92

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NHDaly
Copy link

@NHDaly NHDaly commented Sep 21, 2020

Since TimerOutputs are already nested timings, they map 1:1 to a FlameGraph profile. This simply adds the option to export a FlameGraphs.jl FlameGraph profile object, which can then be displayed via many of the flame-graph compatible display packages: ProfileView, ProfileSVG, etc.

Since TimerOutputs are already nested timings, they map 1:1 to a
FlameGraph profile. This simply adds the option to export a
FlameGraphs.jl FlameGraph profile object, which can then be displayed
via many of the flame-graph compatible display packages: ProfileView,
ProfileSVG, etc.
@NHDaly
Copy link
Author

NHDaly commented Sep 21, 2020

This PR is a Draft, because I don't know how you feel (@KristofferC) about adding a dependency on FlameGraphs.jl and LeftChildRightSiblingTrees.jl. They seem to have pulled-in quite a lot of dependencies:

(TimerOutputs) pkg> add FlameGraphs LeftChildRightSiblingTrees
  Resolving package versions...
   Updating `~/.julia/dev/TimerOutputs/Project.toml`
  [08572546] + FlameGraphs v0.2.4
  [1d6d02ad] + LeftChildRightSiblingTrees v0.1.2
   Updating `~/.julia/dev/TimerOutputs/Manifest.toml`
  [1520ce14] + AbstractTrees v0.3.3
  [3da002f7] + ColorTypes v0.10.9
  [5ae59095] + Colors v0.12.4
  [5789e2e9] + FileIO v1.4.3
  [53c48c17] + FixedPointNumbers v0.8.4
  [08572546] + FlameGraphs v0.2.4
  [9b13fd28] + IndirectArrays v0.5.1
  [1d6d02ad] + LeftChildRightSiblingTrees v0.1.2
  [189a3867] + Reexport v0.2.0
  [2a0f44e3] + Base64
  [ade2ca70] + Dates
  [b77e0a4c] + InteractiveUtils
  [76f85450] + LibGit2
  [8f399da3] + Libdl
  [37e2e46d] + LinearAlgebra
  [56ddb016] + Logging
  [d6f4376e] + Markdown
  [44cfe95a] + Pkg
  [9abbd945] + Profile
  [3fa0cd96] + REPL
  [9a3f8284] + Random
  [ea8e919c] + SHA
  [9e88b42a] + Serialization
  [6462fe0b] + Sockets
  [2f01184e] + SparseArrays
  [10745b16] + Statistics
  [cf7118a7] + UUIDs

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2020

Codecov Report

Merging #92 into master will decrease coverage by 5.21%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   92.91%   87.70%   -5.22%     
==========================================
  Files           4        5       +1     
  Lines         353      374      +21     
==========================================
  Hits          328      328              
- Misses         25       46      +21     
Impacted Files Coverage Δ
src/TimerOutputs.jl 100.00% <ø> (ø)
src/profile.jl 0.00% <0.00%> (ø)

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 77d7316...72971a8. Read the comment docs.

@KristofferC
Copy link
Owner

This PR is a Draft, because I don't know how you feel (@KristofferC) about adding a dependency on FlameGraphs.jl and LeftChildRightSiblingTrees.jl. They seem to have pulled-in quite a lot of dependencies:

I think it is important that this package is very quick to load, for example, right now:

julia> @time using TimerOutputs
  0.003354 seconds (8.45 k allocations: 769.656 KiB)

but with this PR

julia> @time using TimerOutputs
  0.621417 seconds (1.43 M allocations: 96.632 MiB)

(which amazingly is a 200x increase).

So I think we'll need to find another way. If only we had JuliaLang/Pkg.jl#2005 :P

@IanButterworth
Copy link
Contributor

If only we had JuliaLang/Pkg.jl#2005 :P

If only..

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.

4 participants