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 500279 - Theme is parsed/evaluated on every window focus event
Theme is parsed/evaluated on every window focus event
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: themes
trunk
Other All
: Normal critical
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2007-11-28 19:23 UTC by iain
Modified: 2007-12-31 19:48 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
Patch to reduce theme parsing functions (73.05 KB, patch)
2007-11-28 19:24 UTC, iain
none Details | Review
Theme parser fixes (56.15 KB, patch)
2007-12-05 20:13 UTC, iain
none Details | Review
bugtrace (5.19 KB, text/plain)
2007-12-07 12:18 UTC, Andrea Cimitan
  Details

Description iain 2007-11-28 19:23:39 UTC
Please describe the problem:
From a profile of metacity
samples  %        symbol name
13908    21.8501  pos_eval_helper
10471    16.4504  pos_tokenize
7621     11.9729  meta_color_spec_render
5408      8.4962  compositor_idle_cb
3785      5.9464  fix_region
1559      2.4493  meta_draw_op_draw_with_env
1511      2.3738  free_tokens
1480      2.3251  meta_parse_position_expression

Metacity appears to spend over 50% of its execution time parsing theme code.
Attached is a patch that reduces the timings for pos_tokenize to 0.04% and meta_color_spec_render to 1.08%

Instead of the theme information being stored as strings that need to be tokenized every time Metacity draws a window, the patch tokenizes the string at theme load time.

For metacity_color_spec_render the GdkColors for blend and shade are calculated once at the start.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 iain 2007-11-28 19:24:52 UTC
Created attachment 99793 [details] [review]
Patch to reduce theme parsing functions
Comment 2 Elijah Newren 2007-11-29 04:26:23 UTC
Sweet!  You rock!

(Leaving official patch approval for Thomas since theme code is his territory...)
Comment 3 iain 2007-12-05 20:13:36 UTC
Created attachment 100346 [details] [review]
Theme parser fixes

Makes the expression into a structure containing cache and tokens. Makes everything cleaner and removes some of the need to call pos_eval.
Comment 4 Andrea Cimitan 2007-12-07 11:55:40 UTC
Here "Appearence" Applet and metacity-theme-viewer are crashing... is this related to this patch you merged in your branch?
Comment 5 Thomas Thurman 2007-12-07 12:05:18 UTC
Andrea: You're running bucket-o-bling? Do you have a stack trace handy?
Comment 6 Andrea Cimitan 2007-12-07 12:18:48 UTC
Created attachment 100516 [details]
bugtrace

Yes I'm running the bling branch with ./autogen.sh --prefix=/usr --enable-compositor

The crash happenes with and without the shadows.

Another [offtopic] bug is that the shadow of the gnome-panel appears over the titleabr of the application if you login and start gnome... then for draw it below you must restart metacity or enable/disable the shadows in the gconf-editor to restart them
Comment 7 Andrea Cimitan 2007-12-07 12:20:05 UTC
I've forgot to say that everyone who used the bling branch of my friends have this crash. (5/5 guys)
Comment 8 Thomas Thurman 2007-12-09 22:09:51 UTC
Andrea: You should raise that as a separate bug against Iain's compositor; this is a bug about themes.
Comment 9 Andrea Cimitan 2007-12-09 22:13:31 UTC
Thomas: why?
I got this bug just after the update
Comment 10 Thomas Thurman 2007-12-09 22:19:35 UTC
Andrea: I think possibly you're looking at the wrong bug. This bug is an enhancement request: the theme is reparsed on every window focus event. Iain has supplied a patch to fix that, which is attached to this bug. But this patch has not been committed. If it WAS committed it would go on trunk: the bucket-o-bling branch is going to die as soon as we can merge it.
Comment 11 Andrea Cimitan 2007-12-09 22:41:30 UTC
I know, in fact I've updated the iain branch to try it... And the app crashed :)
So I'm saying we should fix those apps before considering this patch acceptable for the merge
Comment 12 Thomas Thurman 2007-12-09 22:44:52 UTC
Okay, let's put this off until after we merge b-o-b, then.
Comment 13 iain 2007-12-10 16:51:54 UTC
No, I have committed this to the bucket o bling branch, just so I could get people testing it. I'll take a look over it and see whats wrong once I'm back at my main machine next week possibly.
Comment 14 Thomas Thurman 2007-12-10 17:12:04 UTC
Okay, great. Let me know when you're ready to merge to trunk; a lot of people are very excited about it. And apparently they think your branch name is unpronouncable in Italy. :)
Comment 15 iain 2007-12-11 14:39:49 UTC
Ok, I've worked out what the crasher is, I should have it fixed fairly soon. When using the library to draw as the theme-viewer and appearance capplet do, the default theme isn't set, and I was dereferencing the default theme to get some speed optimisations. I'll just make it fall back to the slow strcmps whenever the default theme isn't set...
Comment 16 Thomas Thurman 2007-12-30 14:22:36 UTC
This was merged when BOB was merged, right?
Comment 17 iain 2007-12-31 19:48:01 UTC
yes, it was...closing :)