GNOME Bugzilla – Bug 141813
Speed up metacity
Last modified: 2005-07-24 20:05:36 UTC
See http://mail.gnome.org/archives/desktop-devel-list/2004-May/msg00028.html Basically the idea is to "compile" as much as possible of a metacity theme into a pixmaps and/or pixbufs.
Created attachment 29273 [details] [review] Patch to cache frames after they have been exposed This patch caches the frames for a second after they have been exposed. The result is a decrease in lagging when you drag windows around, especially with complex themes like Bluecurve or Industrial. I consider this a band-aid, because it doesn't do anything to speed up drawing of frame resizing and because it adds too much code. The real fix is still to 'compile' a theme into a better representation, in my opinion.
Comment on attachment 29273 [details] [review] Patch to cache frames after they have been exposed Some comments on patch, though it sounds like we shouldn't apply it anyway? I'd tend to agree... caching all over the place seems to hurt overall performance by increasing memory usage/bandwidth, an approach that makes us faster dynamically would be better and have higher impact. The static variables for the cache should really be in some object, perhaps the MetaFrames. This patch probably conflicts with keithp's patch on d-d-l about separate GtkStyle per colormap. Is gdk_window_get_position() the one that does a round trip? If so it might be nice to avoid. I think there's some no-round-trip position query for GdkWindow. Kind of messy to have invalidate_cache() all over, maybe at least factor out the invalidate_rect()/invalidate_cache() pair into a single invalidate_whole_window() call. The cut and paste from GDK sort of sucks...
"It sucks." Yeah, I agree, though the patch doesn't actually increase overall memory use because the cache is deleted after 1 second without action. I can certainly fix the the things you pointed out if you think it's worth applying it. That depends on whether somebody is likely to actually fix it properly, which is a big job.
What's your view on whether this makes things noticeably better for the user?
With complex themes like Industrial and Bluecurve, the improvement is very noticable, especially in combination with the RENDER speedups and the patch in bug 143914. It is a bit difficult to actually measure the lag, but I have seen metacity drop from ~35% to ~5% on profiles where I have dragged a window around.
Seems like it might be worth it, if we try to keep the patch relatively clean and easy to revert later...
Created attachment 29949 [details] [review] New patch Here is a new attempt that should take care of your comments. gdk_window_get_position() doesn't do a round trip.
Any news?
Havoc? Have you had a chance to look at this?
Comment on attachment 29949 [details] [review] New patch It's OK to commit this. I didn't review it in detail but since it doesn't change any high-level structure or semantics of the code, at worst there are a couple of minor bugs we can fix when they arise.
Soeren: Did you want to commit this or wait and make some changes? Also, should we branch before committing this, or is it okay to stick it on 2.10?
Could someone commit this for me? My metacity tree is full of GL stuff at the moment.
Created attachment 47012 [details] [review] Patch updated to CVS HEAD The patch had conflicts due to unrelated changes, so here's an updated version that applies to HEAD. If you'll provide a ChangeLog, Soeren, I'll go ahead and commit it.
I have committed it, but I consider this more of a band-aid than a real fix. Sun Jun 26 11:19:18 2005 Soeren Sandmann <sandmann@redhat.com> * src/frames.c: Add a cache of pixmaps for recently exposed frame areas. Makes metacity a bit faster when dragging windows around. See bug 141813.
Are we leaving this bug open for a reason or was it accidental?
The fix that went in is just a band-aid, not a real fix, IMO. See the linked mail for the suggestions for a better fix. But I doubt anybody will ever do that, so we might as well close this bug.