libgtk2.0-0 performance problem due to unused cruft from 10 years ago

Bug #689741 reported by eXtace
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
GTK+
Fix Released
Medium
gtk+2.0 (Ubuntu)
Fix Released
Low
Unassigned

Bug Description

Binary package hint: libgtk2.0-0

After discussions with Owen Taylor it was found that GTK+ has a longstanding performance problem in cases where a program has a large number of GTK+ widgets on a window.

https://bugzilla.gnome.org/show_bug.cgi?id=637155
https://bugzilla.gnome.org/show_bug.cgi?id=637156

Removing gtk_widget_reset_shapes() from the gtk_widget_set_style_internal() function in gtkwidget.c and rebuilding.reinstalling GTK+ results in a SIGNIFICANT performance improvement for applications which use a large number of widgets with GtkWindows.

Patch attached for gtk+2.0_2.20.0-0ubuntu4, for ubuntu 10.04, however this bug applies to all GTK+ versions.

Source: gtk+2.0_2.20.0-0ubuntu4

Distro tested on: Ubuntu 10.04, however this bug is present in ALL GTK+ versions

apt-cache policy libgtk2.0-0
libgtk2.0-0:
  Installed: 2.20.1-0ubuntu2
  Candidate: 2.20.1-0ubuntu2
  Version table:
 *** 2.20.1-0ubuntu2 0
        500 http://mirrors.ccs.neu.edu/ubuntu/ lucid-updates/main Packages
        100 /var/lib/dpkg/status
     2.20.0-0ubuntu4 0
        500 http://mirrors.ccs.neu.edu/ubuntu/ lucid/main Packages

Without this patch, GTK+ applications with a large number of widgets with GtkWindows will suffer higher CPU usage, and significantly slower performance.

Related branches

Revision history for this message
eXtace (djandruczyk) wrote :
tags: added: patch
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thank you for your bug report, that doesn't seem a trivial change though and not something we want to do before GNOME

Changed in gtk+2.0 (Ubuntu):
importance: Undecided → Low
status: New → Triaged
Revision history for this message
eXtace (djandruczyk) wrote :

Removing a call to a cpu eating yet UNNEEDED function isn't trivial? I don't understand your reasoning. Removing 10 year old UNUSED cruft that results in a significant speedup to Applications with large(r) numbers of widgets, seems like a win, esp since it appears to be unused and unneeded.

From My chat logs with Owen taylor on #gtk+ (irc.gnome.org)

(10:54:06 AM) owen: profiling it seems to indicate some possible major optimizations - weirdly most of the time seems to be spent on a piece of GTK+ that isn't really used at all - support for widgets shaped with the SHAPE extension
(11:01:45 AM) owen: dj1: I don't have a source build of GTK+ 2.0 around, but if you have one or want to create one ,and you remove the call to gtk-widget_reset_shapes() from gtk_widget_set_style_internal() I think that will help a lot
(11:02:40 AM) owen: (it was added for some experiemental stuff we did when we were first adding theming to GTK+ in 1998 and has absolutely no point now)
(11:17:41 AM) dave: owen: huge improvement for your test program with that fix. i can get up to 32x32 easily, and even up to 48x48 without pegging the cpu, though it renders at a bit over half speed at that size
(11:18:16 AM) dave: I'm going to see if ubuntu will accept that as a patch as its bound to improve other areas I suspect.
(11:21:08 AM) owen: dj1: filed https://bugzilla.gnome.org/show_bug.cgi?id=637155 for and https://bugzilla.gnome.org/show_bug.cgi?id=637156 for the two issues that combine together here to create the problem
(11:21:45 AM) dave: owen: the patched GTK+ solves the render issue in my app, thanks for the help!
(11:24:18 AM) owen: dj1: thanks for mentioning the problem, and sorry for blaming your app. :-) I knew there was something going weird that wasn't just GTK+ recomputing sizes, but wasn't expecting it to be some left-over code in GTK+ from a decade ago.

Even owen admits it was forgotten leftover code.

Revision history for this message
Sebastien Bacher (seb128) wrote :

there is not a lot to argue if the change is trivial upstream has no reason to commit it to git and then we will backport it there...

Revision history for this message
Sebastien Bacher (seb128) wrote :

"to not commit" to git

tags: added: patch-forwarded-upstream
removed: patch
Revision history for this message
eXtace (djandruczyk) wrote :

The reason why I posted to ubuntu/launchpad, is that I don't expect the upstream to really care about this enough to release a new version in a timely manner, as its not something that directly causes crashes or a security leak, it just causes a big performance drop for some applications. I would have thought the distro maintainers may want it out faster as it in theory will improve the overall performance of the distribution in all of its releases, which is a huge benefit to the millions of end users.

Revision history for this message
Sebastien Bacher (seb128) wrote :

we care about stability before performance though and since we don't know the upstream code as well as people maintaining it we tend to wait for them to review and ack a patch before using it. we don't need a release, it can just get commited in git or reviewed on the upstream bug

Changed in gtk+2.0 (Ubuntu):
status: Triaged → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gtk+2.0 - 2.23.2-0ubuntu3

---------------
gtk+2.0 (2.23.2-0ubuntu3) natty; urgency=low

  * debian/patches/093_git_no_start_delay.patch:
    - git patch to no do sync io calls when opening an url, should fix issues
      where the caller hangs while the url is opening.
  * debian/patches/094_git_optimization.patch:
    - optimization from git to avoid recomputing when no needed (lp: #689741)
 -- Sebastien Bacher <email address hidden> Wed, 15 Dec 2010 11:46:19 +0100

Changed in gtk+2.0 (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Sebastien Bacher (seb128) wrote :

Upstream commit that fix in their stable serie:

http://git.gnome.org/browse/gtk+/log/?h=gtk-2-24
http://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=c282958fb4bb25c736c682735002d15c50cb44d0

I've backported that to natty now, could you test the update and let we know how much of the issue it solves or if the other gtkwidget change is also required?

Upstream did that extra change in trunk: http://git.gnome.org/browse/gtk+/commit/?id=b7fd6f1e8826589646e2b0e2d7a848fc2fab3ee3

Which will land in the next gtk3 update, that one is not in gtk2 yet though.

If the fix works correctly we will consider stable updates for other ubuntu series but it needs testing in natty before

Revision history for this message
eXtace (djandruczyk) wrote : Re: [Bug 689741] Re: libgtk2.0-0 performance problem due to unused cruft from 10 years ago
  • test.py Edit (776 bytes, application/octet-stream; name="test.py")

I'm sorry, I don't have a natty system to test against, however you can do some
testing with owen's python testcase app, which I've attached.

Just change the SIZE = XX line and test, i.e. setting it to 12, creates a
12x12 grid, changing that to 48, creates a 48x48 grid and so on. You should
be able to get to a significantly larger grid and have it update at the correct
rate (1/sec) with the patched GTK+, especially on slower hardware, with less CPU
usage, than with the unpatched GK+ version, the effect may be harder to see on
the latest and greatest machines.

-- David J. Andruczyk

----- Original Message ----
From: Sebastien Bacher <email address hidden>
To: <email address hidden>
Sent: Wed, December 15, 2010 6:09:33 AM
Subject: [Bug 689741] Re: libgtk2.0-0 performance problem due to unused cruft
from 10 years ago

Upstream commit that fix in their stable serie:

http://git.gnome.org/browse/gtk+/log/?h=gtk-2-24
http://git.gnome.org/browse/gtk+/commit/?h=gtk-2-24&id=c282958fb4bb25c736c682735002d15c50cb44d0

I've backported that to natty now, could you test the update and let we
know how much of the issue it solves or if the other gtkwidget change is
also required?

Upstream did that extra change in trunk:
http://git.gnome.org/browse/gtk+/commit/?id=b7fd6f1e8826589646e2b0e2d7a848fc2fab3ee3

Which will land in the next gtk3 update, that one is not in gtk2 yet
though.

If the fix works correctly we will consider stable updates for other
ubuntu series but it needs testing in natty before

--
You received this bug notification because you are a direct subscriber
of the bug.
https://bugs.launchpad.net/bugs/689741

Title:
  libgtk2.0-0 performance problem due to unused cruft from 10 years ago

Changed in gtk:
importance: Unknown → Medium
status: Unknown → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Patches

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.