GNOME Bugzilla – Bug 588050
Keep overview around rather than recreating it every time
Last modified: 2011-02-08 18:57:03 UTC
This may be something that will fix itself automatically (come time and new code etc) but I want to mention it never the less because it could be critical to the success of the shell: right now, it takes about 2 seconds for the overlay to activate after the button is pressed. This does not sound like a long time but when trying to work with this all the time it is like an eternity. Problems: - it interrupts the users work flow - I often find myself clicking on the button twice because I get the feeling that my first click did not work. If timed just "right", nothing will happen (overlay is closed even before the zoom-out ever happened!) - having to go to overlay (a new "location") to launch apps "feels" more complicated as before at first (It is not - compared to the SLAB menu it's actually quite the same, but the impression is different I think). The delayed opening of overlay adds to this bad impression - My PC is not high-end but still quite good (C2D 6600 @ 2.4Ghz, 4GB RAM, GeForce 7950). I fear the delay could be even more drastic on weaker hardware - This may even be related to bug 587820 in some way?
We've been talking about this with Walters and a couple other persons on IRC. So far we've found out that this only happens when there are lots of items in ~/.recently-used.xbel, and I've found the following workarounds: - Changing createIcon in js/misc/docInfo.js to just "return new Clutter.Texture();". - In js/ui/genericDisplay.js, changing the following loop so that it only processes a small amount of files: for (let i = 0; i < this._matchedItems.length; i++) { this._addDisplayItem(this._matchedItems[i]); } So I guess there's the icons or some stuff for *all* items in the GtkRecentlyUsed history being generated when we enter the overlay, but I couldn't find where that comes from.
(In reply to comment #1) > We've been talking about this with Walters and a couple other persons on IRC. Nice... I had to leave IRC early today but I'm glad someone else is also looking for a solution to this :) > So far we've found out that this only happens when there are lots of items in > ~/.recently-used.xbel Wow... I can confirm this 100%. After deleting the file, overlay comes up instantly. I also noticed that mutter's memory usage (e.g. bug 587820) increases much more slowly now... not even 1mb per overlay trigger (it still keeps on growing, but a *lot* slower)
I'm working on fixing it. DocDisplay.js and ShellTextureCache will cooperate and make sure we're only parsing/loading these things once.
Created attachment 138090 [details] [review] Better caching for document textures and information Move thumbnail creation into ShellTextureCache. It's now asynchronous, and we cache the result. Create a DocManager class which keeps around the DocInfo objects between invocations. This is also where we ensure we remove thumbnails for recent items not known anymore.
I'm curious to what extent this patch helps people; I tried adding 100 nontrivial SVGs to my recent items, the overlay took about 2 seconds. With this patch it's about 1. I haven't profiled, so what we were doing before with the document textures may not have even been the most expensive thing. But I/O in the mainloop and reloading the pixbufs each time was definitely bad.
Just tried out your patch, and with it I get white squared boxes everywhere a thumbnail/icon should be, except for images (ie., no icon for text documents, videos nor anything else).
Oops, yeah I'd replaced my whole recent list with svgs (which work), but non-images fail. I see the bug - a bit tricky to fix, working on it.
Created attachment 138164 [details] [review] Better caching for document textures and information Move thumbnail creation into ShellTextureCache. It's now asynchronous, and we cache the result. Create a DocManager class which keeps around the DocInfo objects between invocations. This is also where we ensure we remove thumbnails for recent items not known anymore.
Can someone else try out the patch? I'm sorry to say this but here the problem persist after applying it.
To clarify, the persisting problem is the slowness, the icons are correct now.
I don't have the problem without the patch, so I can't test that the patch fixes it. The code looks ok, although I found a bug in GnomeThumbnailFactory while reviewing it. :) [bug 588244]
Hm, disappointing. It does seem to improve performance for me, but it's only part of the story.
danw: Ok to commit then?
Merged, but leaving this open for further improvements.
Created attachment 138562 [details] [review] Bug 588050 - Cache information icon forever Extend ShellTextureCache by adding the concept of a policy, which we expose to the public API for loading URIs. This lets us have the shell tell the cache to keep the information icon texture around forever.
Created attachment 138567 [details] [review] Bug 588050 - Cache information icon forever, only look up recent once Extend ShellTextureCache by adding the concept of a policy, which we expose to the public API for loading URIs. This lets us have the shell tell the cache to keep the information icon texture around forever. Secondly, fix the caching of recent info; we shouldn't always be loading the backup pixbuf. Move recent info loading entirely into ShellTextureCache.
I'm going to have to do a lame review since it's getting a bit late, and I want to get going. - In general, the patch looks very reasonable - I haven't reviewed things at a level of detail to catch memory management errors, I didn't see anything looking through things at a higher level. - Using unions and enums rather than structures were fields are magically mutually exclusive, would probably make the code quite a bit clearer, but I don't consider this a blocking issue - shell_texture_cache_unref_recent_thumbnail() needs a better name. ref() and unref() should be reserved for memory management. I think evict_recent_thumbnail() would be appropriate. - The comment in the cache key equal function /* don't compare policy here...yet */ isn't very useful. It doesn't say anything about why we might we don't compare policy now, or might want to later, as implied by "yet"
Fixed up for those comments and committed.
From my profiling work, most of our remaining time is spent in JavaScript. To improve things further, we need to re-architect the code so that the overlay is persistent, rather than recreating everything each time.
You seem to be getting a hand on the issue. I can reproduce the issue on a tricore 2.6GHz AMD with a Nvidia 8600GT. The whole shell is very very slow, almost painfully. I have done no further testing but I can confirm that the recent history is so full that the numbers for pages goes offscreen.
(In reply to comment #19) > From my profiling work, most of our remaining time is spent in JavaScript. To > improve things further, we need to re-architect the code so that the overlay is > persistent, rather than recreating everything each time. updating summary
we do this