[MERGE] Minor profiling fixes

[Ian Clatworthy]

Robert Collins wrote:
> On Mon, 2007-09-10 at 13:06 +1000, Ian Clatworthy wrote:
> bb:resubmit
> 
>> === modified file 'bzrlib/lsprof.py'

>>      try:
>>          ret = f(*args, **kwds)
>> +    except (KeyboardInterrupt, Exception), e:
>> +        import bzrlib.trace
>> +        bzrlib.trace.report_exception(sys.exc_info(), sys.stderr)
>> +        ret = 3
>>      finally:
> 
> 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
> regardless.

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
http://www.python.org/dev/peps/pep-0352/.

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,
just sub-optimal.

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"
>>              else:
>>                  ext = os.path.splitext(filename)[1] 
> 
> This is unfriendly to windows - did you see Alexanders mail about not
> using os.path functions ?

Good point. Fixed.

Ian C.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: profiling-fixes-2.patch
Type: text/x-patch
Size: 8692 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070910/d60d0949/attachment.bin