[PATCH] Improve profiling doc and make the data easier to visualise

[Ian Clatworthy]

John Arbash Meinel wrote:

> First, I think Robert was saying that there should be a unit-testable
> helper function for determining the output format. And that it should
> have unit tests. Not that it was something external that you need to run
> after the profile has been run.
> 
> Which I fully support.

Ah - that makes sense now. Done complete with tests.

> Further, a few comments on the code:
> 
> -        stats.freeze()
> -        cPickle.dump(stats, open(filename, 'w'), 2)
> +        outfile = open(filename, 'w')
> +        if filename.endswith(".callgrind"):
> +            stats.calltree(outfile)
> +        elif filename.endswith(".txt"):
> +            stats.pprint(file=outfile)
> +        else:
> +            stats.freeze()
> +            cPickle.dump(stats, outfile, 2)
> +        outfile.close()
> 
> I always wonder when I see a file opened in 'w', versus 'wb'. I don't
> know how python pickles interact with Windows, but I generally prefer to
> see 'wb'.
> 
> Second, we have taken the stance of always using try/finally whenever we
> open a file. It probably doesn't hurt anything here, but it is good to
> stay in the habit. So you don't forget somewhere that it is important.
> So it should be:
> 
> outfile = open(filename, 'wb)
> try:
>   if filename.endswith('.callgrind'):
>     stats.calltree(outfile)
> ...
> finally:
>   outfile.close()
> 

Sure. Done.

> As an aside, I believe the reason for 'isinstance()' is because you
> can't serialize code objects. So when you load things from a pickle,
> they don't come back quite the same as when you put them in.

FWIW, that part of the patch isn't my code - it's a bugfix from
http://ddaa.net/blog/python/lsprof-calltree. I've tried to make that
clear in both the code and NEWS. Having said that, I'd like the comment
to be correct given Robert felt it was necessary to have one. I can't
see anywhere in the code where the pickling is done except just before
the data is dumped to pickle format. Looking through the .callgrind
file, the only place I see the output implied by the isinstance calls is
when built-ins are reported on. If I'm missing something here, let me know.

> Other than that, +1 from me.

New bundle attached now. It probably doesn't need a full review again
but I'd appreciate it if you could confirm that the saving helper method
is as expected and unit tested sufficiently.

Thanks,
Ian C.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: profiling-easier3.patch
Type: text/x-patch
Size: 34692 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070606/12250938/attachment-0001.bin