[Twisted-Python] Some things I've learned: safer callbacks, better t.p.context

Tue Oct 18 20:09:25 MDT 2016

> making such aggressive use of private APIs that it could win a contest
about how to ensure that you break on every new release of Twisted :)

We're very aware of that! It's one of the reasons we have the test matrix
set up to run multiple versions of Python and Twisted. I have not started
on 16.X compatibility yet.

 >  I imagine that it's a non-trivial impact to performance, so it would be
worthwhile to track that.

We put this this through some extensive benchmarks and testing to measure
the performance impact. For example, the details are logged in a commit
message but, we initially implemented the @inlineCallbacks extension as a
coroutine wrapper. However, we found that the way t.p.Failure tries to
serialize itself, and its local+global scopes, to a dictionary caused
enormous memory and CPU consumption when triggered because of the added
objects in those spaces. The negative impact grew exponentially with levels
of nested coroutines. Very bad day.

Once we pivoted to a small fork of @inlineCallbacks, we measured the
overall performance hit to be negligible in our services. I'll dig around
to see if I can find where we documented the actual numbers we saw. At a
macro level, our service wide stats showed no meaningful growth of runtime
or memory consumption.

> digression on "Don't Switch In The Core"

I was surprised at how much switching this context implementation was when
we put it in the lower level read/write callbacks. Each of our services
process a large amount of continually streaming data and our profiles show,
IIRC, that one of the top 5 consumers of CPU time was calling the
read/write callbacks. When we added this to those paths it increased
overall CPU usage by double digit percentage points. If this feature were
available as an opt-in reactor extension then providers could capacity plan
around the performance hit. We found it more valuable to move the switching
closer to application protocol code where switches happen less frequently.

  > I also take it from the performance notes that you're not using PyPy?

We're still on cPython. PyPy is something we've talked about before but
haven't invested much time into yet. I don't know to what extent PyPy might
change the performance characteristics of the project.

On Tue, Oct 18, 2016 at 7:50 PM Glyph Lefkowitz <glyph at twistedmatrix.com>
wrote:

On Oct 18, 2016, at 4:05 PM, Kevin Conway <kevinjacobconway at gmail.com>
wrote:

> This is why twisted.python.context came to exist in the first place; I
always wanted to attach it to Deferred somehow

Well, it's not something we've announced yet through any official channel,
but we had to solve the context propagation problem at Atlassian to
instrument our services with traceable logging. We open sourced our
solution at https://bitbucket.org/hipchat/txlocal. The answer for us was an
extension for the reactor, thread pool, and inline callbacks that maintain
the needed state. There's a readme with some insight into how we tool our
services.

This is (A) very cool, and (B) making such aggressive use of private APIs
that it could win a contest about how to ensure that you break on every new
release of Twisted :).  I'm super impressed that you tracked the
introduction of twisted._threads and support both old- and new-style thread
pools!

We've had it on our backlog to address the mailing list and, possibly even,
discuss what it would take to put this into Twisted. I guess now is as good
if a time as any. Feel free to spin off a another thread or reach out to me
off list with any questions or feedback

This seems like as good a time to talk about it as any!  Integrating this
into the core in some fashion would be good, but I imagine that it's a
non-trivial impact to performance, so it would be worthwhile to track that.

Speaking of performance - I found the long digression on "Don't Switch In
The Core" interesting, since we actually _do_ tracking this kind of context
already, for logging.  I was a little surprised you didn't integrate with
this at all.

To be fair, this is something that our friends over at PyPy have been
bugging us about since forever; when you're benchmarking raw wire speed it
does tend to show up in profiling.

I also take it from the performance notes that you're not using PyPy?
 __slots__ shouldn't make much of a difference there.  (In fact I'm given
to believe it's a slight _decrease_ in performance on pypy...)

-glyph

On Tue, Oct 18, 2016, 14:47 Glyph Lefkowitz <glyph at twistedmatrix.com> wrote:

On Oct 18, 2016, at 5:50 AM, Itamar Turner-Trauring <itamar at itamarst.org>
wrote:

Not been doing much Twisted lately, but have been doing async stuff
elsewhere, and I've learned some useful things.

Thanks for writing these up, Itamar!  This sort of reflection is rare and
it's always helpful :).

1. Callbacks should be sync or async, but never
sometimes-one-sometimes-the-other. For details go read
http://blog.ometer.com/2011/07/24/callbacks-synchronous-and-asynchronous/.
For example, Deferred.addCallback(f) really should never run f()
immediately.

This has come up a lot in a compare-and-contrast of Twisted vs. asyncio.

I agree that the problems with synchronous callbacks are not insignificant
(reentrancy is a degenerate form of preemption, and as we all know
preemption is the corrupt wellspring of all bugs).  However, the benefit,
i.e. consistency of behavior with respect to reentrancy, comes with a cost:
tight coupling to an event loop.  In asyncio, Future's tight coupling to
call_soon is a source of problems; it makes it hard to write a test without
setting up an elaborate scheduling trampoline, whereas
successResultOf/failureResultOf are quite simple to work with.

I think Deferred as it is today is a pretty good compromise between the two
positions.  On the one hand it is decoupled from the event loop.  On the
other - and this is important - *no Deferred-returning API will ever call
your callbacks synchronously*.  Deferred.addCallback will, of course, but
savvy Twisted programmers can (and should) do this, if they have dependent
state changes:

self.manipulateSomeStateForSetup()

d = doSomethingPotentiallySynchronous()
*self.manipulateSomeStateForProcessing()*
d.addCallback(completeOperation)

As a caller, you can always decide whether you can safely be re-entered or
not.  In most cases, simply moving the 'addCallback' to the end of the
function (a-la Go's "defer", oddly enough) is fine.  In more complex cases
where you really need to unwind reentrancy completely, you can do your own
callLater(0) or callFromThread() from an object with a reference to a
reactor.

3.

What happened to '2'? :)

By instrumenting all callbacks it manages, which may or may not
require item #1, Twisted can have a context that automatically follows
callbacks. Node has this and it is extremely useful.
http://fredkschott.com/post/2014/02/conquering-asynchronous-context-with-cls/
is best summary I've found with a bit of searching.

This was _always_ supposed to be the way that Twisted worked, but frankly I
just wasn't smart enough to figure it out.  This is why
twisted.python.context came to exist in the first place; I always wanted to
attach it to Deferred somehow.  I will watch this talk intently; if #1
really is required to address this, my opinion might change.  A PR would be
intensely appreciated.

-glyph

_______________________________________________
Twisted-Python mailing list
Twisted-Python at twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

_______________________________________________
Twisted-Python mailing list
Twisted-Python at twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

_______________________________________________
Twisted-Python mailing list
Twisted-Python at twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20161019/1fb34863/attachment-0002.html>