launchpad-dev team mailing list archive

[Jonathan Lange <jml@xxxxxxxxxxxxx>]

On Mon, Mar 28, 2011 at 3:41 PM, John Arbash Meinel
<john@xxxxxxxxxxxxxxxxx> wrote:
...
> I've been digging through some bzr-on-launchpad performance problems,
> and I've come across something interesting. I was wondering if anyone
> hand feedback that could help me.
>
> Here is the bug I was tracking into:
> https://bugs.launchpad.net/launchpad/+bug/740759
>
> Basically what I've found is that twisted.python.failure.Failure is
> instantiated when a Deferred raises an Exception. And Failure scales
> O(state).
>
> Specifically, it walks all the stack frames and traceback frames, and
> copies all of the global and locals. When the deferred thinks it is done
> with the traceback, it then walks all of those frames again, and calls
> 'safe_repr' on everything. So that you still have hints at all the
> state, but you don't actually reference the original objects.
>
> For a hint as to how expensive this is. When creating a new branch, we
> call 'initialize_...'. Doing this in various permutations:
>
>  30ms   Directly on my VM
>  450ms  Using codehosting on my VM
>  250ms  Codehosting with Failure.cleanFailure hacked to not call
>        safe_repr, but keep references to the original objects.
>  183ms  Codehosting with Failure.__init__ hacked to always set
>        'tb = None'
>
> And of the 183ms, 147ms was spent in XMLRPC calls. So at least "CPU
> time" is back down to the 30ms in bzrlib.
>
> Now, I can understand that Failure is meant to provide useful feedback
> about the original state when an exception doesn't get handled.
>
> But in bzrlib, and certainly in Launchpad exceptions are often used for
> signaling. They certainly don't signal fatal conditions on every exception.
>
> I don't think it is very tasteful to hack Twisted to supress the 'tb'
> parameter. Certainly the code isn't written to be modular in that
> respect. All the walking is done directly in the __init__ function, so
> it can't be simply overridden.
>
> Any hints are welcome.
>

Many Twisted apps would be affected by this problem, and it's
relatively easy to address in Twisted itself. We'd just need to change
Failure to not care about traceback information if a certain flag is
set. We could do this work ourselves fairly easily, and preliminary
discussions with #twisted indicate that it's likely to be accepted
upstream.

This would be much easier than changing the bzrlib.transport contract
to rely on a rich set of return codes rather than exceptions.

Also, if we dropped the SFTP service, we wouldn't need to use Twisted
in our smart-server support. I think we could probably get away with
this.

jml