GNOME Bugzilla – Bug 683867
Schedule actions with higher idle priority
Last modified: 2013-02-12 08:50:16 UTC
Due to constant CPU usage I queried the dri-devel and Michael Dänzer commented on how Evolution schedules actions. Am Dienstag, den 11.09.2012, 15:24 +0200 schrieb Michel Dänzer: > On Die, 2012-09-11 at 15:07 +0200, Paul Menzel wrote: […] > > Now that you are mentioning it, in the bottom there is the message > > »Checking for New Messages« and next to it there is an animation where > > something goes around a circle. Canceling that removes X’s CPU usage. > > That's a GTK+ spinner widget, which uses RENDER trapezoids, which is a > software rendering fallback with EXA. > > > Should I recommend something to the Evolution folks on how to due such > > animations? Or is the only way to avoid animations? > > I don't think there's anything wrong with the animation per se. However, > one issue I've found is that Evolution schedules many actions as glib > idle callbacks with priority lower than G_PRIORITY_HIGH_IDLE + 20, which > is the priority used by GTK+ for drawing animations. This can result in > the animations delaying the completion of the actual work they're > representing. [1] http://lists.freedesktop.org/archives/dri-devel/2012-September/027623.html
I would rather this be fixed in GTK+ than have to hack around it all throughout Evolution and any other application that uses a spinner. The priority value is not part of GTK's stable API, so for all I know the value could change from one release to another. I'd rather not chase it.
Matthias, could I ask you for your opinion, please?
Animations will be done differently in 3.6. I can't say right now if it will be better for this problem or not, but please check it out when Benjamin merges the new spinner animation.
Created attachment 224328 [details] [review] evolution-data-server: Tune idle callback priorities.
Created attachment 224329 [details] [review] evolution: Tune priorities for ilde callbacks.
Created attachment 224331 [details] [review] evolution: mail: Prevent message formatting from getting delayed indefinitely.
(In reply to comment #1) > I would rather this be fixed in GTK+ than have to hack around it all throughout > Evolution [...] How can GTK+ know if a callback needs to be called before drawing or can be delayed until after it? Here's some patches I've been using on top of the gnome-3-4 branches. They significantly improve mailer responsiveness on a single core machine. I think at least callbacks used for signalling from helper threads to the main thread need to be scheduled with much higher priority than they are now. Note: I'm not suggesting these are perfect, just illustrating the problem and suggesting a possible approach to fix it. Note 2: I'm not sure this caused or even directly contributed to Paul's problem.
(In reply to comment #7) > How can GTK+ know if a callback needs to be called before drawing or can be > delayed until after it? It shouldn't matter. If animated images are getting redrawn on _every_ _single_ main loop iteration, preventing the loop from ever becoming idle, I would point to that as the main culprit. I expect some rate limiting on the animation. Adding hacks to libraries that don't even link to GTK+ to work around GTK+ drawing priorities is definitely not the right approach. Let's wait and reevaluate with GTK+ 3.6.
Unfortunately, GTK+ 3.6 seems to make this even worse for me. Even with the attached patches (or with unpatched Evolution 3.6), it again makes me constantly run into situations where work items keep piling up in the Evolution status bar, without any visible progress on the actual work they represent. This quickly results in the Evolution and X processes hogging the CPU and Evolution becoming unusable. I had to kill and restart Evolution about every half hour at least with GTK+ 3.6, so in order to get anything done, I had to downgrade to GTK+ 3.4 again. I hope you guys can find a solution for this problem before GTK+ 3.6 enters Debian sid, or I may have to look for alternatives for handling my e-mail.
Best workaround is to either just remove the spinners from the task bar or keep them static (not animated) until this gets sorted out in GTK+.
(In reply to comment #10) > Best workaround is to either just remove the spinners from the task bar or keep > them static (not animated) until this gets sorted out in GTK+. Indeed, the latter seems to help, thanks for the suggestion. Note that this is just the worst problem, the patches still make a big difference for general mailer responsiveness.
(In reply to comment #11) > (In reply to comment #10) > > Best workaround is to either just remove the spinners from the task bar or keep > > them static (not animated) until this gets sorted out in GTK+. > > Indeed, the latter seems to help, thanks for the suggestion. Is there a simple configuration option or is it required to rebuild GTK+(?)? […]
Created attachment 226746 [details] [review] Don't start spinners (In reply to comment #12) > Is there a simple configuration option or is it required to rebuild GTK+(?)? You need to rebuild Evolution with something like this patch to keep the spinners from spinning.
The gtk3-3.6.1 has the spinner (animation) issue improved, and it was not about evolution itself, as I was able to reproduce this with gtk3-demo too. The spinner is not eating whole CPU when animated, "only" slightly more than 8% of CPU for two spinner in the Spinner demo. As soon as I "Stop" the demo it stops to eat CPU.
spinner bug is bug #684639
(In reply to comment #14) GTK+ 3.6.1 might make the problem described in comment #9 occur slightly less often, but it's still happening.
This issue might be the cause for bug 679377 comment 18 [1]. [1] https://bugzilla.gnome.org/show_bug.cgi?id=679377#c18
I tried, some time ago, to reproduce this issue in a test application, but no way, though I see one difference, mine spinners were all visible and were not part of a status bar, but of a container, which was always completely visible. I'll try to simulate the status bar more closely, whether it'll help to reproduce it. My intention is to check where the issue is, either in evolution or in gtk/gdk.
Created attachment 231169 [details] test.c test program; This opens a dialog, where are buttons (make it larger to see them all): Add spinner - adds a new spinner Start/Stop Spinners - obvious :) Schedule idle - uses g_idle_add() Schedule idle timeout - adds a timeout for 1 second and then g_idle_add() Schedule async - schedules async operation, which is finished on idle after 2 seconds of "activity" You can add as many spinners as you wish, the box will behave in a similar way like the status bar in evolution, the text is ellipsized and the spinners are spinning unless stopped. The window is never made larger than it is. The "Schedule..." buttons write into a label whether they scheduled callback and only if it is received they change the label accordingly. It's good to click only one button at a time, because they reuse the same label. The first line in the file contains a comment, where is a command line to compile the code. The thing is that I cannot reproduce the issue in this test program, though it seems to me that it is quite close to the Evolution ode. My CPU has 4 cores, which may be related, though I'd expect it isn't. I see higher CPU usage when adding more and more spinners, even when they are not visible in the window, but it doesn't block the idle delivery. Could anyone of you try with it, please?
Ouch, I forgot to add, this is with gtk3-3.6.1-1 and glib2-2.34.1-1.
Created attachment 231181 [details] updated test.c Little update for the previous test program, it shows how many spinners were already added. To reproduce, I added 110 spinners, then the "Schedule IDLE" is not received, while the async idle is received. The g_simple_async_result_complete_in_idle() uses g_idle_source_new() with G_PRIORITY_DEFAULT. I guess we can use this priority as well?
And I just got a confirmation, that a one-core machine with Hyper Threading suffers of this with only 20 spinners.
(In reply to comment #22) > And I just got a confirmation, that a one-core machine with Hyper Threading > suffers of this with only 20 spinners. Hah! On this good old PowerBook, I'm getting noticeable delays as of the first few spinners, up to on the order of a minute with just 5 spinners. (In reply to comment #21) > The g_simple_async_result_complete_in_idle() uses g_idle_source_new() with > G_PRIORITY_DEFAULT. I guess we can use this priority as well? Actually, when I tried that, some things seemed to get delayed more again, maybe because some idle handlers themselves used up too many cycles... Right now I'm mostly using G_PRIORITY_HIGH_IDLE, which seems to work pretty well. In your test program, at that priority, I can't see any delay even with 40 spinners. For Evolution, I'm not sure there's a single golden value that works best everywhere...
Created attachment 231266 [details] [review] evolution-data-server: Tune idle callback priorities.
Created attachment 231267 [details] [review] evolution: Tune priorities for ilde callbacks.
Created attachment 231270 [details] [review] gtkhtml: Tune idle callback priorities.
Updated patches rebased for Evolution 3.6.
I would go with your patches for priority level changes, it is documented what each of them does after all, and if we want to make sure that our requests are received properly, without fighting with glib/gtk, then this is the right thing to do, in my opinion. From the documentation: G_PRIORITY_HIGH - Use this for high priority event sources. It is not used within GLib or GTK+. G_PRIORITY_DEFAULT - Use this for default priority event sources. In GLib this priority is used when adding timeout functions with g_timeout_add(). In GDK this priority is used for events from the X server. G_PRIORITY_HIGH_IDLE - Use this for high priority idle functions. GTK+ uses G_PRIORITY_HIGH_IDLE + 10 for resizing operations, and G_PRIORITY_HIGH_IDLE + 20 for redrawing operations. (This is done to ensure that any pending resizes are processed before any pending redraws, so that widgets are not redrawn twice unnecessarily.) G_PRIORITY_DEFAULT_IDLE - Use this for default priority idle functions. In GLib this priority is used when adding idle functions with g_idle_add(). G_PRIORITY_LOW - Use this for very low priority background tasks. It is not used within GLib or GTK+. I would go even with the G_PRIORITY_HIGH, though the G_PRIORITY_DEFAULT should be enough. You changed some places with it too, but I believe only those g_idle_add() replaced with g_idle_add_full() makes the difference. I verified that in the test.c above, if I make g_idle_add() to g_idle_add_full (G_PRIORITY_DEFAULT,), then it's received as expected, while the simple g_idle_add() is not. Nonetheless, Matthew disagreed.
Created attachment 235495 [details] [review] eds patch for evolution-data-server; As long as we mimic what GLib/Gtk+ people do, and as long as GAsyncResult uses G_PRIORITY_DEFAULT, I'd say our own operations, which do not use GAsyncResult directly, should also use G_PRIORITY_DEFAULT for its idle callbacks. And looking around the code, we already tweak the priorities, thus no difference at all so far. This is an extended version of your patch. One difference is to use G_PRIORITY_DEFAULT, and the other, from my point of view the significant difference, is changing JOB_PRIORITY in camel-session.c from LOW to DEFAULT.
Created attachment 235498 [details] [review] evo patch for evolution; Again, there is used G_PRIORITY_DEFAULT. I didn't change that many calls, as those other seem to me unrelated. I'm still committing all the patches with your name, as I didn't add anything significant into the change. Thanks for your patience on this.
Created attachment 235499 [details] [review] eds patch ][ for evolution-data-server; The same like before, only the change in camel-operation.c is removed.
Created commit 224e8be in eds master (3.7.90+) Created commit b6f0db1 in eds gnome-3-6 (3.6.4+) Created commit 2b50771 in evo master (3.7.90+) Created commit 6be763e in evo gnome-3-6 (3.6.4+) Created commit cb712c5 in gtkhtml master (4.7.90+) Created commit 6746969 in gtkhtml gnome-3-6 (4.6.4+)
Milan, your changes seem to work quite well for me, thanks!
Thanks Michel for testing. Please note, not sure if you noticed already, that my changes didn't survive for long, sadly.
At least it's an improvement already, but the main point to take away is that there is no single magic priority value that works best everywhere (if nothing else, it means all idle callbacks are serialized wrt each other). The Evolution developers should carefully audit all code using idle callbacks and tune the priorities as appropriate. In some cases it really doesn't matter if the callback is delayed indefinitely, so G_PRIORITY_DEFAULT_IDLE is fine. But in some cases callbacks are used for things which directly affect responsiveness to user actions, in which case G_PRIORITY_DEFAULT or higher might be good.