code review 13261046: misc/pprof: web command does not open browser/svg-viewe...

misc/pprof: web command does not open browser/svg-viewer in windows

Incase of windows, open svg file using default viewer as associated in the OS.
Let OS open the file or bring up file association dialog as the case maybe.

Better error handling/message when "dot"(graphviz) is not present.

[shiv, 2013-08-28 02:23:05]

Hello golang-dev@googlegroups.com (cc: bradfitz@golang.org,
golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[brainman, 2013-08-28 07:33:54]

This is what I get

(pprof) web
Total: 24 samples
'dot' is not recognized as an internal or external command,
operable program or batch file.
Error while executing dot(is graphviz installed?): Invalid argument

What am I supposed to do?

Alew

[slimsag, 2013-08-28 13:07:09]

AFAIK pprof requires graphviz to generate SVG graphs for displaying in 
browsers.

Do you have graphviz installed and in your path?

Stephen

On Wednesday, August 28, 2013 12:33:54 AM UTC-7, brainman wrote:
>
> This is what I get 
>
> (pprof) web 
> Total: 24 samples 
> 'dot' is not recognized as an internal or external command, 
> operable program or batch file. 
> Error while executing dot(is graphviz installed?): Invalid argument 
>
> What am I supposed to do? 
>
> Alew 
>
> https://codereview.appspot.com/13261046/ 
>

[shiv, 2013-08-28 18:06:10]

On 2013/08/28 07:33:54, brainman wrote:
> 
> (pprof) web
> Total: 24 samples
> 'dot' is not recognized as an internal or external command,
> operable program or batch file.
> Error while executing dot(is graphviz installed?): Invalid argument
> 

As stephen suggested, install graphviz and ensure "dot" command is in path.
Previously empty svg file was getting generated and opened. With this CL it
fails with above error message.
The text "is graphviz installed?" in the error message is to indicate what is
required.

[bradfitz, 2013-08-28 18:12:46]

https://codereview.appspot.com/13261046/diff/6001/misc/pprof
File misc/pprof (right):

https://codereview.appspot.com/13261046/diff/6001/misc/pprof#newcode2036
misc/pprof:2036: close(DOT) || error("Error while executing dot(is graphviz
installed?): $!\n");
This doesn't feel like the right place.

Just check for dot somewhere else (earlier) and just say "The 'dot' binary (from
the graphviz package) is not available."

[brainman, 2013-08-29 01:28:39]

Please add "Fixes Issue 6263" if it does. Thank you.

Alex

[brainman, 2013-08-29 01:52:30]

On 2013/08/28 18:06:10, shiv wrote:
> 
> ..., install graphviz and ensure "dot" command is in path. ...

I did that and now it works.

There is still some error

C:\go\root\src\pkg\archive\zip>go tool pprof zip.test.exe cpu.out
Welcome to pprof!  For help, type 'help'.
(pprof) web
Total: 24 samples
Loading web page file:///C:\DOCUME~1\brainman\LOCALS~1\Temp\241Vx9OlDn.0.svg
'uname' is not recognized as an internal or external command, operable program
or batch file.
(pprof)

If you know what it is, perhaps you should fix it.

Leaving it for you and Brad to settle on final version. Thank you very much.

Alex

[shiv, 2013-09-05 00:54:26]

PTAL

On 2013/08/29 01:28:39, brainman wrote:
> Please add "Fixes Issue 6263" if it does. Thank you.
> 

It does not. Issue 6263 needs its own CL. It has additional 3rd party
dependencies(ghostscript) for ps/pdf handling and seems to require further
investigation after installing the dependencies.

On 2013/08/29 01:52:30, brainman wrote:
> There is still some error
> 
> C:\go\root\src\pkg\archive\zip>go tool pprof zip.test.exe cpu.out
> Welcome to pprof!  For help, type 'help'.
> (pprof) web
> Total: 24 samples
> Loading web page file:///C:\DOCUME~1\brainman\LOCALS~1\Temp\241Vx9OlDn.0.svg
> 'uname' is not recognized as an internal or external command, operable program
> or batch file.
> (pprof)
> 

Fixed

https://codereview.appspot.com/13261046/diff/6001/misc/pprof
File misc/pprof (right):

https://codereview.appspot.com/13261046/diff/6001/misc/pprof#newcode2036
misc/pprof:2036: close(DOT) || error("Error while executing dot(is graphviz
installed?): $!\n");
On 2013/08/28 18:12:46, bradfitz wrote:
> This doesn't feel like the right place.
> 
> Just check for dot somewhere else (earlier) and just say "The 'dot' binary
(from
> the graphviz package) is not available."

OK

[shiv, 2013-09-18 08:45:20]

PTAL.

Can this go into Go1.2. Thanks?

[bradfitz, 2013-09-23 16:33:08]

Too late for Go 1.2.

Maybe https://code.google.com/p/go/issues/detail?id=6281 will happen before Go
1.3.

https://codereview.appspot.com/13261046/diff/17001/misc/pprof
File misc/pprof (right):

https://codereview.appspot.com/13261046/diff/17001/misc/pprof#newcode83
misc/pprof:83: use File::Which;
This isn't in the Perl standard library.

https://codereview.appspot.com/13261046/diff/17001/misc/pprof#newcode1850
misc/pprof:1850: which("$prog") || error("Error - $prog (graphviz) does not seem
to be installed\n");
$prog is not guaranteed to be "(graphviz)".

[gobot, 2013-12-20 05:19:54]

R=bradfitz@golang.org (assigned by alex.brainman@gmail.com)

[gobot, 2013-12-20 16:26:00]

Replacing golang-dev with golang-codereviews.

[bradfitz, 2014-01-14 23:27:49]

R=close

I don't think we're doing any more development on this Perl version of pprof.

Let's wait to tinker with this once it's in Go.