qt5reactor increases idle CPU usage

[crwood]

I recently noticed that applications using qt5reactor will continuously consume additional CPU time when idle (a steady 2-3% on My Machine) -- a behavior that is not seen/observed in competing solutions (e.g., using quamash with asyncio or even running Twisted's reactor in a separate thread from the Qt event loop and communicating between the two using Qt signals/slots). Has anybody else noticed the same and/or done any prior investigating in relation to this? I'm not as familiar with Twisted's internals as I should be but I would like to try to fix/reduce this if possible (and figured I should at least open an issue here to invite further discussion/collaboration before delving into it further).

[exarkun]

https://github.com/sunu/qt5reactor/blob/4f6d5921b2155a9180d92ad607a08189ed07734a/qt5reactor.py#L289-L295 seems to imply that all qt5reactor-based applications will wake up at least 100 times per second.

First the scheduled-call-based timeout is computed. Then, if there are no scheduled calls, the timeout is made 0.01 seconds. If there are scheduled calls, the timeout is made either the time until the first such call or 0.01 seconds, whichever is smaller.

So the reactor basically iterates every 0.01 seconds (or more often if there are scheduled calls which require it).

It would be interesting to compare CPU usage in this mode with CPU usage when there is no mandatory 100-wakeups-per-second logic in place. If that yields a significant speedup then it would be worth looking in to why this logic is in place, whether it is needed at all anymore, and if so whether there is a less expensive way to achieve the same result.

For what it's worth, this kind of logic has been motivated in the past by incomplete or incorrect integration with the external event loop (qt5 in this case) or limitations in the external event loop (either intentional or due to bugs). Basically, if Qt5 can't otherwise wake up the Twisted reactor as often as Twisted believes it is necessary to wake up the reactor, this kind of logic papers over the disagreement (by just telling Qt5 to wake Twisted up a ton, just in case!).

[crwood]

Thanks for looking at this @exarkun! :)

Increasing the timeout value (to 1) indeed drastically reduces idle CPU usage (from ~2.7% to as low as ~0.3%) on my Ganoo plus Linux machine. I will need to do some more testing, however (particularly on Windows) to see whether and to what extent cranking this value higher results in negative side-effects (I remember Windows UI updates becoming choppy when I was playing around with this in the past but that may have been a result of my underpowered VM).

For what it's worth, qt4reactor uses a timeout of 0.1 but this was changed at some point along the way..

[nehbit]

Anecdata: I also remember the UI becoming choppy when I played around with this for about the same reason.

[crwood]

Do you recall whether the choppiness was only present on Windows, @nehbit? UI updates are happening smoothly here on *nix (even with a timeout value as high as 1000).

If the choppiness is indeed exclusive to Windows, I could perhaps increase the timeout value used by QtReactor to some Sensible Value but then override doIteration() in the QtEventReactor class (which inherits from QtReactor and is only used by win32) to keep the value low where it's needed..

[nehbit]

Honestly my memory of this is vague now because I stopped using it three years ago, gave up and ported my application to Go + Electron. But I seem to remember it was actually better on Windows and pretty bad on Mac. I also ran this on Linux (Ubuntu) but I don't remember anything specific about that one.

[exarkun]

On Ubuntu 17.10 I'm now running with this patch:

diff --git a/qt5reactor.py b/qt5reactor.py
index 9ce0b9a..5120195 100644
--- a/qt5reactor.py
+++ b/qt5reactor.py
@@ -286,13 +286,10 @@ class QtReactor(posixbase.PosixReactorBase):
         delay = max(delay, 1)
         if not fromqt:
             self.qApp.processEvents(QEventLoop.AllEvents, delay * 1000)
-        t = self.timeout()
-        if t is None:
-            timeout = 0.01
-        else:
-            timeout = min(t, 0.01)
-        self._timer.setInterval(timeout * 1000)
-        self._timer.start()
+        timeout = self.timeout()
+        if timeout is not None:
+            self._timer.setInterval(timeout * 1000)
+            self._timer.start()

     def runReturn(self, installSignalHandlers=True):
         self.startRunning(installSignalHandlers=installSignalHandlers)

CPU usage is reduced and responsiveness is still good.

No doubt Qt5 has different behavior on each platform, as it has different event loops to integrate with in each case. So considering the impacts of any behavior on all platforms is important - but it's also possible to put a platform check into the code to get improvements for some platforms (where there's no downside).

[crwood]

Okay, I just did some rather extensive testing with this patch on the three big desktop platforms (GNU/Linux, Windows, and macOS) across a number of PyQt5 versions (covering Qt 5.8.2 through 5.10.1) all using Twisted 17.9 and could not identify any negative impacts (at least in terms of animation/UI choppiness or added resource-consumption)..

@exarkun Would you like to submit a quick PR for your patch? I could merge it myself but figure you should get credit for the fix. :)