[MERGE] Minor profiling fixes
ian.clatworthy at internode.on.net
Mon Sep 10 07:31:39 BST 2007
Robert Collins wrote:
> On Mon, 2007-09-10 at 13:06 +1000, Ian Clatworthy wrote:
>> === modified file 'bzrlib/lsprof.py'
>> ret = f(*args, **kwds)
>> + except (KeyboardInterrupt, Exception), e:
>> + import bzrlib.trace
>> + bzrlib.trace.report_exception(sys.exc_info(), sys.stderr)
>> + ret = 3
> Why not just profile run_bzr_catch_errors instead of duplicating code?
> That would be much simpler.
Given run_bzr_catch_errors is several levels higher in the call stack
and command line parameters have been handled in between, it isn't
simpler at all. It is far cleaner IMO to make the change I've proposed.
The dependency is well commented in both places so I don't believe the
redundancy (3 lines) is a risk.
The try:except does need to be nested in the try/finally though for 2.4
so I've fixed that in the new patch.
> Also, KeyboardInterrupt is an Exception, so that code is clearly wrong
That's true. I went digging to understand why the original code I copied
was suboptimal in this regard. Perhaps the author was getting ready for
Python 3 where KeyboardInterrupt will not derived from Exception? See
I've removed the trapping of KeyboardInterrupt in my patch because you
asked for it. I left it unchanged in commands.py because it's not wrong,
Do you want this fully "sorted" now or can we revisit this when we add
support for Python 3.0?
>> @@ -118,7 +126,8 @@
>> otherwise the format is given by the filename extension.
>> if format is None:
>> - if filename.startswith('callgrind.out'):
>> + basename = os.path.basename(filename)
>> + if basename.startswith('callgrind.out'):
>> format = "callgrind"
>> ext = os.path.splitext(filename)
> This is unfriendly to windows - did you see Alexanders mail about not
> using os.path functions ?
Good point. Fixed.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 8692 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070910/d60d0949/attachment.bin
More information about the bazaar