After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773054 - High CPU usage due to huge spinner
High CPU usage due to huge spinner
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: client
master
Other Linux
: Normal normal
: ---
Assigned To: Geary Maintainers
Geary Maintainers
Depends on: 765516
Blocks:
 
 
Reported: 2016-10-16 20:47 UTC by Gautier Pelloux-Prayer
Modified: 2016-10-24 23:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for huge spinner (2.29 KB, patch)
2016-10-21 22:25 UTC, Michael Gratton
committed Details | Review

Description Gautier Pelloux-Prayer 2016-10-16 20:47:46 UTC
Since the merge of https://bugzilla.gnome.org/show_bug.cgi?id=765516, it looks like Geary UI is getting greedy with the CPU. Following measures are made with top and last through time:

- on idle (eg. only Geary's service running, no UI), Geary consumes around 0% CPU
- when the huge conversation spinner is displayed, Geary consumes around 54% CPU
- when a conversation (7 mails) is selected and displayed, Geary consumes around 5% CPU
- when a folder with no message is displayed (so no spinner nor conversation displayed), Geary consumes around 1.5% CPU

I did not test before the merge happened, but if needed I can do so.
Comment 1 Michael Gratton 2016-10-17 11:40:02 UTC
Hmm, does the spinner still animate when not visible? I'm seeing Geary on 0% when actually idle — i.e. nothing being printed when launched with "-d". Of course, the only time the spinner should be visible is when the engine is busy in the background, and hence Geary will be be using a non-trivial amount of CPU.

If you can do some testing and/or some patching around the conversation viewer's two spinners, I'd be happy to look at it. Note also some of changes to how WebKit is embedded in Bug 765516 may have also contributed to this, and will get shook up further with the port to WK2, so it's probably not worth putting too much effort into issues specifically there for now.

I'll flag this for 0.12 so if nothing else it should be revisited after WK2 lands but before the release.
Comment 2 Michael Gratton 2016-10-17 13:52:09 UTC
Just pushed commit 8ea5e3c to master, which should make the spinner go away after the folder has finished loading when autoselect is off. Still not sure why your spinner is epic-sized though.
Comment 3 Gautier Pelloux-Prayer 2016-10-17 14:55:08 UTC
Michael, it seems you inverted if/else condition in the patch (oops!).
Comment 4 Gautier Pelloux-Prayer 2016-10-17 15:02:55 UTC
Hm, actually it is not that; yet the "autoselect=off" option is broken and Geary always selects the next conversation.
Comment 5 Michael Gratton 2016-10-18 00:39:18 UTC
Really? That's weird. This is what I see after 8ea5e3c:

Auto select pref is ticked: Click on a folder, spinnner appears when loading. When loaded first conversation in list is selected and displayed.

Auto select pref is unticked: Click folder, spinnner appears, no conversation is selected and "No conversation selected" UI is shown when loaded.

What are you getting?
Comment 6 Gautier Pelloux-Prayer 2016-10-18 07:40:01 UTC
Yes that is working great, but there are 2 other uses cases (at least):

1. Select a conversation, delete it. If autoselect is unticked, you should then have the "No conversation is selected" UI. Currently, it automatically selects the next one, as if autoselect is ticked (regression)
2. It looks like the spinner goes away too soon, eg when the first mail of the conversation is loaded or so. Here is a video sample: http://www.webm.land/w/xPGA/
I selected a conversation, then the spinner went back and forth until finally the conversation was shown.
Comment 7 Michael Gratton 2016-10-19 01:08:02 UTC
Oh I see - I can reproduce both of those, will look into it.
Comment 8 Michael Gratton 2016-10-20 05:30:37 UTC
Hey, I just pushed commit 310daa8 to fix (1) and commit 0e5df2c which should fix (2).

Do they work for you?
Comment 9 Gautier Pelloux-Prayer 2016-10-20 07:44:06 UTC
Yeah, thanks! All looks great now. There remain only 2 issues(?) in that bug:

3) Huge conversation spinner is expected to be smaller?
4) Higher CPU usage when a conversation selected (between 20 and 50%). I would expect it to end to 5% (as when nothing is selected), any idea what can cause extra CPU usage?
Comment 10 Michael Gratton 2016-10-21 22:25:10 UTC
Created attachment 338217 [details] [review]
Fix for huge spinner

Hey Gautier, this patch may fix the huge spinner, can you please test it out and let me know if it does?
Comment 11 Michael Gratton 2016-10-21 22:26:54 UTC
Not sure about the CPU, but as I mentioned it may well change after the WK2 port, so I'm not too inclined to look at the until after that lands and it remains an issue.
Comment 12 Gautier Pelloux-Prayer 2016-10-24 09:03:03 UTC
> Hey Gautier, this patch may fix the huge spinner, can you please test it out and let me know if it does?

Indeed it does! It is now a nice 32x32 icon.

> I'm not too inclined to look at the until after that lands and it remains an issue.

Seems fair; I think this bug can be close then (since spinner bug is fixed) and I'll open a new one after WK2 port lands in master, if needed.
Comment 13 Michael Gratton 2016-10-24 23:04:20 UTC
Review of attachment 338217 [details] [review]:

Pushed to master as commit 797bb44.
Comment 14 Michael Gratton 2016-10-24 23:05:36 UTC
Sounds like a good plan, thanks for testing it out.