Use Graphviz_jll #71
Use Graphviz_jll #71
Conversation
This removes the need to manually install Graphviz
| proc[] = pprof_jll.pprof() do pprof_path | ||
| open(pipeline(`$pprof_path -http=$webhost:$webport $relative_percentages_flag $file`)) | ||
| Graphviz_jll.dot() do dot_path | ||
| open(pipeline(`$pprof_path -http=$webhost:$webport $relative_percentages_flag $file`)) | ||
| end | ||
| end |
There was a problem hiding this comment.
Since this package requires Julia v1.6, you could use the recommended thread-safe syntax
run(`$(mungify_exe()) $arguments`)instead of
mungify_exe() do exe
run(`$exe $arguments`)
endbut I don't understand what this is doing: dot_path isn't used. Is it used only to merge the environments?
There was a problem hiding this comment.
Yeah we just need to instead dot into the path, since it is going to be called by pprof
There was a problem hiding this comment.
Bit convoluted, but can do something like
pprof = addenv(pprof_jll.pprof(), Graphviz_jll.JLLWrappers.LIBPATH_env=>Graphviz_jll.LIBPATH[], "PATH"=>Graphviz_jll.PATH[])
open(pipeline(`$pprof -http=$webhost:$webport $relative_percentages_flag $file`))There was a problem hiding this comment.
It is not enough to just modify PATH, because shared libs used by dot must also be found.
I did the change the way I did it because it was the easiest and quickest way for me to achieve what I needed. I was not aware of the new syntax. I simply extended what was there (in particular it seems to me that any thread safety issues are already present and not just introduced by this PR?)
the patch by @giordano seems interesting, but I don't understand it right now -- e.g. shouldn't the "LIBPATH" and PATH content be merged with what is already there instead of overriding it? Or perhaps this is somehow done automatically? This may be obvious to you, but sadly not me :-/.
As it is, I don't have the time and energy to understand and figure all this out, then test it again etc. So if you want it to use that method I think it makes more sense if someone else (@giordano?) opens a PR with it. Feel free to close this PR then!
In the meantime, this patch allowed me to use PProf.jl and that in turn allowed me to tune some code substantially using the Memory Allocation Profiler. Thanks !!!
|
Does JuliaPackaging/Yggdrasil#5652 supersede this PR? Does PProf now work even if you don't have graphviz installed? 🎉 thanks Valentin, btw! |
This removes the need to manually install Graphviz
Resolves #16
Seems to work fine for me on macOS. Did not test on Linux or Windows