The patch looks pretty good. It definitely fixes a big part of the issue. I think there's a bit more room for improvement though. ;)

simulate still imposes a 0.1 second maximum on the timeout it uses to reschedule itself. Can we get away with removing this as well? If it stays, the reactor will still wake up 10 times a second as long as there are any outstanding delayed calls.

The 0.1 in callLater looks like a slightly cheesy hack to make ensure that an earlier delayed call set up after a later one will actually get run on time. It'd be nice to do this a bit more cleanly (perhaps looking at _pendingTimedCalls[0] and cancelling and rescheduling _simtag if it's earlier than the previous one).

The multiplications by 1010 still in gtk2reactor.py are a sort of irksome. I dunno if that really falls under this ticket or not; it might be worth thinking about a bit though.

I talked to thomasvs offline about this a bit. He said there was potentially a performance issue with the extra calls to reactor.timeout() necessary to address point 1 above, but we both agreed this bears further investigation (timeout should really be a cheap operation, and something else in glib2reactor should be fixed if it's not there; another possibility is that the performance problem was coming from somewhere we still don't entirely understand).

I talked to thomasvs offline about this a bit. He said there was potentially a performance issue with the extra calls to reactor.timeout() necessary to address point 1 above, but we both agreed this bears further investigation (timeout should really be a cheap operation, and something else in glib2reactor should be fixed if it's not there; another possibility is that the performance problem was coming from somewhere we still don't entirely understand).

Trying to address 2 rescheduling the next timeout in callLater(), we saw a degradation increasing with the number of concurrent connections. Whith few concurrent connections we were able the get almost twice iterations per seconds more, but this performance decreased linearly with the number of concurrent connections.
My guess is the that in the case of X concurrent connections, timeout() is called only once in simulate(), whilst it would be called X times in callLater(). I think profiling the benchmark test would let us understand much better what is really going on :)

Regarding 1, the idea was to create a new timeout as it would be done in simulate(), which is actually also using min(timeout, 0.1) and this would wake up the reactor 10 times per seconds as long as there still are delayed call.

Probably looking at _pendingTimedCalls[0] would be a good approach. I'll try to profile the benchmark with gprof2dot[1], since a graphic representation of the time spent on each call is probably the best way to figure out where this improvement should be done.

glib handles timeouts much less efficiently than any of the existing Twisted reactors. Letting glib take care of these would probably make the glib-based reactors unusable for applications with many timeouts.

Some playing around with gtk3 reactor suggests we can just get rid of the 0.1 minimum timeout and be done with it... possibly with an addition of self.wakeUp() call to reactor.stop(). It doesn't seem necessary at all - unless someone can tell me why it's there in the first place?

I deleted the timeouts... and all the tests still pass. And I see no reason why they shouldn't. (There's one test that was failing with dirty reactor, but that seems to be because it's not a great test, so I fixed it.)

(I was wrong about trial, iterate is only used in test cleanup, not test running. I was also partially wrong about callFromThread, see below for new solution.)

I added a couple more tests, rewrote lots of the reactor, and removed the changes to test_ssl.

Previously, simulate was called every 100ms, and on every I/O event. This ensured callFromThread scheduled events happened, at the expense of extra processing and power usage. The new code only calls _simulate when the reactor is woken up via the waker (so that callFromThread works), or when the earliest scheduled callLater is pending. Since reactor.stop only sets a flag which is relevant to Twisted code, we use wakeUp to ensure glib will call back into Twisted immediately after current event handler is done, otherwise it wouldn't necessarily notice immediately.

Thanks. manhole now works on my Ubuntu 10.04 desktop. strace confirms that it is not waking up when there is nothing to do (although blinking the input cursor while the window is visible counts as something to do).

Can you stick the notion of idleness into the news fragment? The reactors may well wake up 10 times per second, if there is work to do. :)

I might have made the waker customizable with a factory instead of an argument, like:

if not self.waker:
self.waker = self._wakerFactory()
...

It's a minor point, but it keeps the public interface as small as it was.

I think that I had build up some kind of understanding of how gtk2reactor worked before (over years of poking it). I don't think I understand how it works now. If you feel like it, it would probably be a pretty big maintenance win if you added a few maintainer notes about how the pieces fit together.

The way ReactorBuilder is used by GlibTimeTestsBuilder isn't really what I had in mind for ReactorBuilder. I think we should eventually figure out a better way to test particular reactors.