Bug 304381 - X-resource leak when using Eclipse and oxygen-gtk
Summary: X-resource leak when using Eclipse and oxygen-gtk
Alias: None
Product: Oxygen
Classification: Plasma
Component: gtk2-engine (show other bugs)
Version: unspecified
Platform: openSUSE Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
Depends on:
Reported: 2012-08-01 14:39 UTC by Jaroslav Kameník
Modified: 2013-03-05 21:05 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:

Proposed patch (479 bytes, patch)
2013-03-02 21:43 UTC, Ruslan Kabatsayev

Note You need to log in before you can comment on or make changes to this bug.
Description Jaroslav Kameník 2012-08-01 14:39:23 UTC
I have problem with Eclipse - it slows down after some time, it take seconds to refresh window/open menu/... and later stops to respond and must be killed. I found that xrestop shows still growing values in Pxms and Misc columns and top shows Xorg eating lots of cpu time. When I use another default style then Oxygen or gtk style other then oxygen-gtk, it is ok.

Reproducible: Always

Steps to Reproduce:
1. Use Oxygen/oxygen-gtk style
2. Have Eclipse running long time or do lots of GUI operation.

Current config:

openSUSE 12.1
KDE 4.8.97
nvidia drivers 302.17
gtk2 oxygen engine 1.3
eclipse juno

but i have this problem longer time so it is not caused by latest versions of SW...
Comment 1 Hugo Pereira Da Costa 2012-08-01 15:58:10 UTC
I"ll have a look.
In priciple, pixmaps allocation via xrestop should significantly increase when you start the application, and ultimately saturate to some maximum, because we are using caches (to optimize rendering).
Now, maybe there is a leak somewhere.

Question: do you experience similar kind of issues with other gtk applications (e.g. gimp), notably concerning the xrestop behavior ? 

Comment 2 Jaroslav Kameník 2012-08-01 19:05:12 UTC
I am trying it now on another pc (OSS 12.1, kde 4.8.97, nvidia 302.17, oxygen-gtk2 1.2.50git.1342458469-41.1). 

I have tried pidgin - values grow while maximizing/minimizing window, opening/closing menus etc. In a few minutes it grows to thousands, more than any other app.

GIMP and opening menus still around - the same, just faster (i suppose it has bigger menus:).

GIMP with industrial gtk style after same procedure use just 19 pixmaps and 69 misc.
Comment 3 Hugo Pereira Da Costa 2012-08-03 08:46:20 UTC
My experience with gimp:
growing number of allocated pixmaps at startup and during first minutes of operations, then saturating to something like 1200 pixmaps, and 1.4M pixmap memory.

-> these are small pixmaps (one must compare the *memory* -not number of pixmaps- with other apps)

After that, number of pixmaps and memory might still grow by one or two every now and then but definitly not at the same pace as at the beginning.

this is the *expected* behavior, and corresponds to caching.
It is *normal* that you don't see it with other themes since they are not caching their (usually simpler) rendering.
=> comparison with "industrial" is irrelevant. 

Now: do you see the same behavior (that is: the number of allocated pixmaps reaching a maximum after some point), or do you see an ever-increasing amount of pixmaps being allocated, at constant pace ?
Comment 4 Hugo Pereira Da Costa 2012-08-03 08:47:21 UTC
Also, please provide some actual numbers rather than "grows to thousands". What I see is 1 thousand pixmaps allocated. Which is expected. Not thousand*s*
Comment 5 Gael Beaudoin 2013-02-28 15:48:34 UTC
I can confirm I have this problem for a very long time. Various KDE versions and using openSUSE and Kubuntu.

Currently, Kubuntu 12.10, KDE 4.10.

Hugo, how could I help fixing this ?
Comment 6 Gael Beaudoin 2013-02-28 15:53:28 UTC
For example, current xrestop for eclipse, launched for a few hours, very slow now :

res-base Wins  GCs Fnts Pxms Misc   Pxm mem  Other   Total   PID Identifier
1000000    48   88    1 20859 21830    77901K    515K  78417K  6697 Java EE - Eclipse
Comment 7 Gael Beaudoin 2013-02-28 15:58:28 UTC
And a freshly launched eclipse :

3e00000    16   47    1  464  569     2725K     15K   2741K 24176 Java EE - Eclipse

And after a window and a context menu opened, the total is more than 32k.
Comment 8 Hugo Pereira Da Costa 2013-03-01 09:04:23 UTC
thanks for the numbers.
20 thousands pixmaps is definitely too much.
Now I'm not sure they are allocated by us. Will check the caches again ... 
Also: which version of oxygen-gtk do you have (it is largely uncorrelated with the version of KDE and is not shipped with KDE). 

Concerning your question "how could I help to fix this".
Ideally: use valgrind with eclipse. But 
1/ I have not succeeded to do so myself
2/ I'm not sure it is even possible due to its plugin nature and the use of Java ... 
I've googled a bit already but with no success ...

(my concern, to be honest, is that as far as I know the same behavior is _not_ achieved with other gtk applications, running with oxygen-gtk. A lot of pixmaps are created, but the number "saturates" to around 1000, which corresponds to the caches being full. So that there must be something upstream (eclipse) that triggers this behavior together with oxygen-gtk ...
Comment 9 Ruslan Kabatsayev 2013-03-02 21:04:47 UTC
OK, I can reproduce.
@Hugo, if you need a way to reproduce, here it is:
1. Make some C++ project
2. Open some tree items in Project Explorer so that they give a screenful of rows (this will make it easier to reproduce)
3. Now start moving mouse up-down over that treeview, so that hover rect moves many times over the list. At the same time watch xrestop, Eclipse will linearly increase its memory consumption.

If I change "SimpleCache( size_t size=100..." to size of 1 (and the same for Cache()), nothing changes. Likewise, if I disable inner shadows hack, nothing is changed either.

Now if I add a return; at the beginning of "if( d.isCell() )", I don't reproduce the problem.
Comment 10 Ruslan Kabatsayev 2013-03-02 21:27:29 UTC
OK, traced it down to StyleHelper::selection(). Looks like _selectionCache.insert() is called for each prelight, with different surface, even if it has the same key as just before.
Comment 11 Ruslan Kabatsayev 2013-03-02 21:43:09 UTC
Created attachment 77692 [details]
Proposed patch

Apparently, TileSet's h1 and h3 appear both 0: h1 is passed as 0 constant, h3 looks like gets subtracted from surface height which appears equal it. And TileSet::isValid() checks for height()>0, where height = h1+h3.
So, should this check be changed somehow? Maybe "(width()>0||height()>0)&&_surfaces.size() == 9"? instead of both width()>0 && height()>0?

The change is attached as a proposed patch. I'm not sure it's the way to go, but at least I no longer reproduce this bug with the patch.
Comment 12 Hugo Pereira Da Costa 2013-03-02 22:15:56 UTC
thank's for tracking this down (and for the aurorae patch).
Will investigate your patch tomorrow or monday.
Then there is the ecclipse "expander" bug. 
Then I guess we'll make a release :)
Comment 13 Ruslan Kabatsayev 2013-03-02 22:53:52 UTC
Now great news: this bug is that same bug for pidgin becoming unresponsive with time. I can reproduce it with pidgin's buddy list just the same way I did for Eclipse. In fact, you can reproduce it even with twf!
So, I'm now happy we know what's going on. Thanks to reporters :)
Comment 14 Ruslan Kabatsayev 2013-03-03 10:13:48 UTC
Git commit 118aba00061ee239b07cff00093a6a7de1293a60 by Ruslan Kabatsayev.
Committed on 03/03/2013 at 11:12.
Pushed by kabatsayev into branch '1.3'.

Add some debug warnings

M  +6    -0    src/oxygentileset.cpp

Comment 15 Gael Beaudoin 2013-03-03 16:13:00 UTC
Ah well, thank you for looking into this! This was really annoying. Thank you thank you thank you! :)
Comment 16 Hugo Pereira Da Costa 2013-03-05 13:20:43 UTC
I don't think the patch is correct, sadly (might create other issues)
A tileSet is valid only if *both* (not either) its dimensions are right.
I believe there must be a way to fix this "upstream" ... investigating.
Comment 17 Hugo Pereira Da Costa 2013-03-05 13:51:16 UTC
... interestingly, the same issue might be present in oxygen@kde ...
Comment 18 Hugo Pereira Da Costa 2013-03-05 14:07:39 UTC
ok. I 'think' that the only test that is needed for TileSet validity is in fact _surfaces.size()==9
Which means that proper constructor was called.
Now, I also need to add some checks in the rendering method that the surfaces being rendered are valid.

... and then that leaves the fact that deleted tilesets are not properly clearing the allocated resources
Comment 19 Hugo Pereira Da Costa 2013-03-05 15:20:29 UTC
Git commit 37d090ece6b81aa4c30a66ac637c571c210af84b by Hugo Pereira Da Costa.
Committed on 05/03/2013 at 16:18.
Pushed by hpereiradacosta into branch '1.3'.

only check pixmap list size in ::isValid

M  +1    -1    src/oxygentileset.h

Comment 20 Hugo Pereira Da Costa 2013-03-05 15:22:40 UTC
Commit above prevents the bug to occur, but does not fix the deep-down issue that resources are apparently leaked at tileset deletion. (it just prevents the cache to be filled too quickly).

... still investigating the upstream issue.
Comment 21 Hugo Pereira Da Costa 2013-03-05 16:03:36 UTC
Git commit 5149fe325591aa5da370aa374cd2c1ec680a400e by Hugo Pereira Da Costa.
Committed on 05/03/2013 at 16:18.
Pushed by hpereiradacosta into branch 'gtk3'.

only check pixmap list size in ::isValid

M  +1    -1    src/oxygentileset.h

Comment 22 Hugo Pereira Da Costa 2013-03-05 21:05:43 UTC
closing down this one since both the not using cached value bug, and the underlying resource leak are fixed.