Created attachment 6905 Screenshot of top and panels After upgrading to 1.1.0 with GTK+3 the battery applet started consuming CPU a lot. I have two indicators: the compact purple one in the top panel and the more verbose one in the sidebar if this matters.
After a few days of running it has also shown a memory leak: http://pic4a.ru/611/pW.png
Unfortunately I can't reproduce this problem. Does it always happen? As soon as Xfce starts? Did this problem affect the GTK2-based plugin?
(In reply to André Miranda from comment #2) > Does it always happen? Yes, I kill and restart the panel about once a day to free up consumed resources and each time it gradually leaks again. > Did this problem affect the GTK2-based plugin? No, it started just after upgrading to GTK+3 version.
Sorry for the long delay, I have been monitoring the plugin and as you suggested now I have one in a horizontal panel and another in a deskbar panel. However, only after an entire day (8~10 hours) the second plugin memory consumption is around 55mb and the first is ~27mb. The CPU consumption is mostly 0% for the first plugin and about 2% for the second one after a long time running. Does it take all that time for you to notice the leak? As in my case, does this only affect the deskbar plugin?
Reviewing the commits during the GTK 3 port I've found potential leaks, here is the updated source (preview, not pushed to Xfce's repository): https://github.com/andreldm/xfce4-battery-plugin If possible, build and check if it solves your problem.
I have built the plugin with those fixes, replaced libbattery.so and readded it to panels. After a few hours of running it still leaks, right now it has 70MB RAM/16% CPU/32 min. of CPU time. Just like you mentioned it affects only one of the processes, the other one keeps in 15 MB RAM and ~0% CPU.
This problem is caused by gtk_css_provider_new and gtk_style_context_add_provider functions being called with each invocation of update_apm_status. More css providers => more memory occupied and more work to do during style-rendering. Over time this effect accumulates enormously. What's the solution? Previously added css provider needs to be either removed and g_object_unref'ed or reused.
Possible solution (against git master): http://pastebin.com/zAC3zWCS
Thanks revel for your suggestion. I've pushed to my GitHub repository changes to reuse the CssProvider. bodqhrohro, please check if this solves the leak for you (it takes a very long time leak here, so far everything is ok).
André, thanks for maintaining this project. Merry Christmas & Happy New Year.
(In reply to revel from comment #7) > This problem is caused by gtk_css_provider_new and > gtk_style_context_add_provider functions being called with each invocation > of update_apm_status. More css providers => more memory occupied and more > work to do during style-rendering. Over time this effect accumulates > enormously. What's the solution? Previously added css provider needs to be > either removed and g_object_unref'ed or reused. Are you sure of this ? If so, i'll have to apply this 'fix' to other plugins... but i'd like to have this assertion backed by doc or numbers first..
It *seems* you can g_object_unref (provider) after assigning it to a GtkStyleContext which might be another way to 'fix' the leak, if there's one here. Maybe no need to 'remove' the previous provider ?
oh and fwiw i've commited this two days ago: https://git.xfce.org/panel-plugins/xfce4-battery-plugin/commit/?id=2ab3a4ba61665083059af54271d8ccc30926776f so whatever the fix is, it has to go on top of this....
Unfortunately Gnome/GTK docs are not too good. I'm using XFCE on Arch Linux and just like the original bug reporter, some time ago, I noticed this problem with high resource usage by battery plugin. And just like OP I've been killing the plugin process once a day or so to keep it under control. Here are the numbers: - before patching: http://pastebin.com/dhgDbkrR - after patching: http://pastebin.com/5sGEsxjJ (memory usage jump around 1h50m was caused by me opening configuration dialog for plugin) The 'after patching' numbers are for the patch I posted before, although André's solution seems valid as well.
Code-wise i dont like readding/removing the provider or using a static ref or using a 'permanent' struct member if only using g_object_unref on it is enough to fix the leak, can you experiment with this possibility ? And yeah i agree the docs arent clear about that.
Of course, my solution was a quick fix to "make it work" for me with least effort :) Ok, I can do some more testing, but I'd say unref alone won't be enough.
tested latest release (1.1.0) tested patch: http://pastebin.com/RQ28Zgyn results: http://pastebin.com/hffqCdXw So, no luck.
I have applied the patch from http://pastebin.com/zAC3zWCS and do not notice a leak. For about 8 hours of running it stably consumes 22 MB RAM.
According to https://developer.gnome.org/gtk3/stable/gtk-migrating-GtkStyleContext-parsing.html _unref() *should* do what it's supposed to. sigh. And it seems to be a widely used construct, cf https://mail.gnome.org/archives/commits-list/2014-November/msg04700.html for example. gtk_style_context_add_provider() calls this code: https://git.gnome.org/browse/gtk+/tree/gtk/gtkstylecascade.c?h=gtk-3-22#n352 - which calls g_object_ref() on the provider, so g_object_unref() makes sense to me here. Granted, you have patches that fix the issue for you, but that's not the correct way to fix the leak...... it's just a bandaid.
note that the same problem exists in wavelan plugin since it recreates a cssprovider every second.
Aren't you missing the point here? Calling _unref() is ok and justified, but another ref is still held by gtkstylecascade, which is ok as well. The point is, that gtk_style_context_add_provider does not remove any previously added providers. The call at https://git.gnome.org/browse/gtk+/tree/gtk/gtkstylecascade.c#n370 is clearly to prevent adding the same provider more than once, nothing more. So the internal array cascade->providers keeps growing since more and more providers are added with each periodic invocation of update_apm_status in the plugin.
i agree with you, but keeping a reference to the 'previous' provider and removing it before adding the new one feels wrong in the code flow. There *should* be an elegant way to solve that. Maybe using https://developer.gnome.org/gtk3/stable/GtkCssProvider.html#gtk-css-provider-get-default and loading the css in this one, instead of creating a new one ?
Just take a look at Andree's patch: https://github.com/andreldm/xfce4-battery-plugin/commit/664856361da66754465d609223eeb7ff61e6ec14 He's reusing the provider, no creating/adding in the periodically invoked function.
I of course saw it, it *is* nicer, but i still hope there's a better way :)
Just please don't spend weeks looking for "the perfect one" while users are hurting and their cpus burning :)
using a singleton pattern like this for the provider might be the 'best' solution for now, i just dont really like 'reapplying' a style at every update even if it didnt change, ie if the color didnt change because the battery level is still the same, it will still reload the css.... might need some refactoring to keep track of color changes ? oh well, perfection doesnt exist. i dont see the relation with cpu usage though, is it a different problem from the leak, or the fix corrects leaks *and* cpu usage ?
That's the same issue. There is no considerable memory leak in the exact sense, just very wasteful use of css providers. Lots of providers added => lots of memory taken and lots of time used to process them.
I've pushed andre's commits to master branch (cf https://git.xfce.org/panel-plugins/xfce4-battery-plugin/log/), keeping attribution but just shuffling things around and adding #if to ensure it was only used with Gtk >= 3.16. Fixed another bug while here: change-mode callback was force-triggering update_apm_status every second, instead of the default 60s... made no sense to me. I dont know what happens on linux, but polling every 2s looks insane. Here on OpenBSD it only updates once in a minute. Please test it and report back if it works for you (i dont have a laptop around to properly test this...), then i'll do a bugfix release.
Memory-wise: works fine. CPU-wise: works even better, CPU usage is basically flat zero. However, now plugin reaction time is sluggish, it takes some time for the plugin to notice plugging/unplugging of power supply and change bar color.
Well, that's the timeout thing. I dont have a linux here and the code is a maze between the acpi/apm methods, i dont understand why there are so many cases. Of course it takes some time to update, if update_apm_status() isnt called every second, but on the other hand if you call it every second you waste cpu cycles and consume power.
Yeah I saw the mess in the code. I'm testing xfce4-power-manager at the moment. It uses almost no CPU but still manages to react swiftly. Plus apparently it leaks only a little memory. No bar indicator, but I might be switching in favour of it anyway.
I've run into this bug, and can confirm that the code introduced in the following commit is the problem: https://git.xfce.org/panel-plugins/xfce4-battery-plugin/commit/?id=b2985f36af2d8f62f0d6585c8917f8e920ccddc9 It's very, very bad. Memory usage *and* CPU usage increases linearly as the process continues running! When I delete the block of code beginning with #if GTK_CHECK_VERSION (3, 16, 0) (including the #else clause, which doesn't compile correctly anymore), things are much saner. (and the code doesn't seem terribly necessary in the first place-- only relevant when battery is low?) I'm quite sure that the css provider thing mentioned above is the culprit (I discovered this on my own before seeing it had been mentioned here as well). As mentioned above, it's not a memory leak in the strict sense, but is still an incredible leak in a practical sense. Speaking of memory leaks, valgrind says that gdk_rgba_to_string (in that same block of code) causes a memory leak. Presumably you need to free the string that it returns. To make things worse, this version made it into the new Debian stable (stretch). Get ready for lots of mad folks... :/
Whoops, should have mentioned that my comments were about version 1.1.0 (the version that made it into stretch). I see that the latest git has fixed code.
Yes, both issues have been fixed but not released. Did you build from git and check if the issues are solved for you?
I just released 1.1.1, can you guys tell me if the problem is solved?
Closing since no one complained, please reopen if this is still an issue.