Merge lp:~vanvugt/compiz/fix-1095001 into lp:compiz/0.9.9

Proposed by Daniel van Vugt
Status: Merged
Approved by: Sam Spilsbury
Approved revision: 3534
Merged at revision: 3533
Proposed branch: lp:~vanvugt/compiz/fix-1095001
Merge into: lp:compiz/0.9.9
Diff against target: 85 lines (+28/-5)
3 files modified
plugins/opengl/src/paint.cpp (+12/-3)
plugins/opengl/src/privates.h (+3/-0)
plugins/opengl/src/screen.cpp (+13/-2)
To merge this branch: bzr merge lp:~vanvugt/compiz/fix-1095001
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+141551@code.launchpad.net

Commit message

Avoid calling code that results in calls to regexec as much as possible.
Regular expression matching is too expensive to do very often and compiz
was spending 31% of its CPU time in regexec().
(LP: #1095001)

Description of the change

Before: 31%
After: 0.01%

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

2 if (unredirectFS &&
23 !blacklisted &&
24 - unredirectable.evaluate (w) &&
25 !(mask & PAINT_SCREEN_TRANSFORMED_MASK) &&
26 !(mask & PAINT_SCREEN_WITH_TRANSFORMED_WINDOWS_MASK) &&
27 - fs.isCoveredBy (w->region (), flags))
28 + fs.isCoveredBy (w->region (), flags) &&
29 + (!cw->redirected () || unredirectable.evaluate (w)))

It might be worth trying to fix that wall of conditionals to be more readable in future too.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/opengl/src/paint.cpp'
2--- plugins/opengl/src/paint.cpp 2012-12-13 10:12:23 +0000
3+++ plugins/opengl/src/paint.cpp 2013-01-01 10:11:23 +0000
4@@ -354,22 +354,31 @@
5 if (w->alpha ())
6 flags |= FullscreenRegion::Alpha;
7
8+ CompositeWindow *cw = CompositeWindow::get (w);
9+
10 /*
11 * Windows with alpha channels can partially occlude windows
12 * beneath them and so neither should be unredirected in that case.
13+ *
14+ * Performance note: unredirectable.evaluate is SLOW because it
15+ * involves regex matching. Too slow to do on every window for
16+ * every frame. So we only call it if a window is redirected AND
17+ * potentially needs unredirecting. This means changes to
18+ * unredirect_match while a window is unredirected already may not
19+ * take effect until it is un-fullscreened again. But that's better
20+ * than the high price of regex matching on every frame.
21 */
22 if (unredirectFS &&
23 !blacklisted &&
24- unredirectable.evaluate (w) &&
25 !(mask & PAINT_SCREEN_TRANSFORMED_MASK) &&
26 !(mask & PAINT_SCREEN_WITH_TRANSFORMED_WINDOWS_MASK) &&
27- fs.isCoveredBy (w->region (), flags))
28+ fs.isCoveredBy (w->region (), flags) &&
29+ (!cw->redirected () || unredirectable.evaluate (w)))
30 {
31 unredirected.insert (w);
32 }
33 else
34 {
35- CompositeWindow *cw = CompositeWindow::get (w);
36 if (!cw->redirected ())
37 {
38 if (fs.allowRedirection (w->region ()))
39
40=== modified file 'plugins/opengl/src/privates.h'
41--- plugins/opengl/src/privates.h 2012-12-13 10:12:23 +0000
42+++ plugins/opengl/src/privates.h 2013-01-01 10:11:23 +0000
43@@ -222,6 +222,9 @@
44 CompSize rootPixmapSize;
45
46 const char *glVendor, *glRenderer, *glVersion;
47+
48+ mutable CompString prevRegex;
49+ mutable bool prevBlacklisted;
50 };
51
52 class PrivateGLWindow :
53
54=== modified file 'plugins/opengl/src/screen.cpp'
55--- plugins/opengl/src/screen.cpp 2012-12-12 07:33:12 +0000
56+++ plugins/opengl/src/screen.cpp 2013-01-01 10:11:23 +0000
57@@ -1248,7 +1248,9 @@
58 rootPixmapSize (),
59 glVendor (NULL),
60 glRenderer (NULL),
61- glVersion (NULL)
62+ glVersion (NULL),
63+ prevRegex (),
64+ prevBlacklisted (false)
65 {
66 ScreenInterface::setHandler (screen);
67 }
68@@ -2155,7 +2157,16 @@
69 bool
70 PrivateGLScreen::driverIsBlacklisted (const char *regex) const
71 {
72- return blacklisted (regex, glVendor, glRenderer, glVersion);
73+ /*
74+ * regex matching is VERY expensive, so only do it when the result might
75+ * be different to last time. The gl* variables never change value...
76+ */
77+ if (prevRegex != regex)
78+ {
79+ prevBlacklisted = blacklisted (regex, glVendor, glRenderer, glVersion);
80+ prevRegex = regex;
81+ }
82+ return prevBlacklisted;
83 }
84
85 GLTexture::BindPixmapHandle

Subscribers

People subscribed via source and target branches