Ticket #4680 (new defect)

Opened 7 years ago

Last modified 6 years ago

Sugar apps' pygtk main loop polls 10 times a second, always

Reported by: gnu Owned by: marco
Priority: normal Milestone: Future Release
Component: sugar Version: 1.0-software-build-623
Keywords: power, performance Cc: jg, krstic, tomeu, cjb, bemasc, cscott, dsd
Action Needed: never set Verified: no
Deployments affected: Blocked By:
Blocking:

Description

The interface between Python and GTK+ limits the GTK+ main loop's ability to sleep when idle, forcing it to wake up every 100 ms. This appears to be true for every Python program that calls GTK+.

I noticed that the SimCity Python helper process was doing tons of system calls -- lots of poll() calls with timeouts of 100ms. You can see this by running the strace command on any Python sugar application, such as the Journal's sugar-activity-factory,

So I debugged the SimCity one. The timeout is used in the poll() call in g_main_loop_run() in glib2, and is set by this code in pygtk_main_watch_prepare in pygtk2's gtk.override:

1058 /* This code (pygtk main watch) was copied with minor changes from 1059 * pygobject/gobject/pygmainloop.c */ 1060 static gboolean 1061 pygtk_main_watch_prepare(GSource *source, 1062 int *timeout) 1063 { 1064 /* Python only invokes signal handlers from the main thread, 1065 * so if a thread other than the main thread receives the signal 1066 * from the kernel, PyErr_CheckSignals() from that thread will 1067 * do nothing. So, we need to time out and check for signals 1068 * regularily too. 1069 * Also, on Windows g_poll() won't be interrupted by a signal 1070 * (AFAIK), so we need the timeout there too. 1071 */ 1072 #ifndef PLATFORM_WIN32 1073 if (pyg_threads_enabled) 1074 #endif 1075 *timeout = 100;

Clearly, this code should not be waking up every 10th of a second to "check for signals regularily" -- neither on the OLPC, nor on other machines running Python GTK applications. But I don't know enough about this code to suggest a fix.

Bug #4677 is a tracker for similar bugs.

Change History

Changed 7 years ago by marco

Upstream bug:

http://bugzilla.gnome.org/show_bug.cgi?id=481569

Note that in joyride we are not initializing threads in the base Activity class anymore, so this should only happen for activities which actually use threads.

Changed 7 years ago by marco

Places that still initialize threads in sugar:

services/shell/sugar-shell-service:gobject.threads_init() services/shell/sugar-shell-service:dbus.glib.threads_init() shell/intro/glive.py:gobject.threads_init()

And in the datastore:

bin/datastore-service:gobject.threads_init() bin/datastore-service:dbus.glib.threads_init()

Changed 7 years ago by marco

  • cc jg, krstic added

Thread on the python mailing list about this. Patches/proposal by the pygtk people was refused.

http://mail.python.org/pipermail/python-dev/2006-September/068569.html

Changed 7 years ago by tomeu

  • cc tomeu added

I think we only really use threads in the DS, is that right?

Changed 7 years ago by marco

See comment 2 about where we are initializing them. glive.py is not even used (but it might get called by import). Shell service is initializing but not using threads, hopefully we can just get rid of the initialization (assuming the dbus/dbus-python race is gone, otherwise I guess we will have to fix it). There are likely activities which are using threads though.

Changed 7 years ago by jg

  • milestone changed from Never Assigned to Future Release

Changed 7 years ago by cjb

  • cc cjb added
  • keywords power, performance added; power removed

Changed 7 years ago by bemasc

The upstream thread is broken apart in the archives, starting at: http://mail.python.org/pipermail/python-dev/2006-September/068569.html http://mail.python.org/pipermail/python-dev/2006-September/068585.html http://mail.python.org/pipermail/python-dev/2006-September/068610.html http://mail.python.org/pipermail/python-dev/2006-September/068646.html http://mail.python.org/pipermail/python-dev/2006-September/068714.html

There is a python bug at: http://bugs.python.org/issue1564547

Having read through it, the bottom line is that Python upstream may make the changes that would allow PyGTK to remove this polling loop, but not before 2.6. However, the polling loop serves only to catch UNIX signals, which are not normally used by PyGTK programs. Except for catching these sorts of signals, it seems that multithreaded PyGTK programs will work perfectly without this polling loop.

I am not at all knowledgeable about this topic, but it seems to me that we could probably patch PyGTK to remove this loop, or increase its timeout to 60 seconds, and nobody would notice. The only change would be that Ctrl-C and 'kill -SIGTERM' might not work to kill PyGTK programs. Once we have suspend-on-cpuidle, it would be a valuable power savings.

Changed 7 years ago by bemasc

  • cc bemasc added

Changed 7 years ago by jg

I have brought this bug to the attention of to Guido VR and he will be talking to the PyGTK folks next week.

Changed 7 years ago by jafo

I've started up discussion on this again at:

http://mail.python.org/pipermail/python-dev/2007-December/075589.html

My understanding is that the current patch on bugs.python.org doesn't fix the issue. This is based on reading the threads referenced in the python.org bug.

If anyone here has some input on this, you should get involved in the bug or the discussion on python-dev.

Sean

Changed 7 years ago by gnu

Hooray! Python 2.6 now includes an interface that causes low-level signal handlers to write a single zero byte to a user-set file descriptor. This allows a library like PyGTK to use a pipe to break out of its select() or poll() loop and process the signal in the main thread. Which means the main thread doesn't have to wake up all the time to check for a pending signal.

This is described in Johan Dahlin's blog entry, Python Issue 1583 (which includes this final patch from Father Guido), and in the PyGObject 2.14.1 announcement and the PyGTK 2.12.1 announcement, which use the interface (if available) to eliminate polling.

Johan recommends that people who care about eliminating these wakeups should patch their current Python interpreter to add this interface, rather than wait for 2.6.

Changed 6 years ago by gnu

  • next_action set to never set

In joyride-2263, this is only slightly improved. Here's why:

Python-2.5.1 was patched in F9 to add the patch from Python Issue 1583. But the F9 maintainer installed Guido's last posted patch, rather than his final SVN changes. Those final changes added a C api that, it turns out, is the interface that pygobject and PyGTK use. So gobject and gtk don't know the feature is there yet.

PyGobject is probably OK once it gets recompiled in an environment with the better-patched Python. Meanwhile, it now polls once a second rather than 10x a second, until it sees the new Python interface, so at least we have that improvement.

PyGTK, however, still apparently has some issues; its signal handling has to deal with both the case of a Python interpreter with the feature, and one without it, and there have been some bugs in their first try. See:

http://bugzilla.gnome.org/show_bug.cgi?id=481569

So we can't close this OLPC bug yet. If F10 straightens out Python, and Gnome straightens out PyGTK, then we should pull in those changes for the following release.

Changed 6 years ago by mstone

  • cc cscott added

Changed 6 years ago by cscott

I removed the only use of signals from sugar-shell in #8532, so we should be okay just hacking out the signal polling, too, if we need a quick fix.

Changed 6 years ago by gnu

Python handles signals all the time, anyway, so it can handle SIGINT (C). The real fix is coming along, so I suggest just working on it rather than on short-term hacks. If you want to spend an hour or two on it, look at the GNOME bug above and volunteer to test their changes by building them in the OLPC environment and testing 'em. They lack a realistic test case.

Changed 6 years ago by dsd

  • cc dsd added

working on this today

To start with, here is a Python scratchbuild which includes the right PySignal_SetWakeupFd() patch

pygobject needs fixing as described in the GNOME bug, but this is no biggie

Changed 6 years ago by dsd

Changed 6 years ago by dsd

OK, pushed into joyride for testing:

Python 2.5.2 with the correct patch backported from Python 2.6 (final scratchbuild: http://koji.fedoraproject.org/koji/taskinfo?taskID=983981)

pygobject (built locally with rpmbuild) including a patch I just wrote to fix the usage of the new Python feature, posted at http://bugzilla.gnome.org/show_bug.cgi?id=481569

Let's see what breaks...

Changed 6 years ago by dsd

bug report to get the python changes included in Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=475005

Changed 6 years ago by dsd

also pushed a new PyGTK (built with rpmbuild) with the equivalent patch through public_rpms, and posted the patch on the same GNOME bug

Note: See TracTickets for help on using tickets.