Merge lp:~sil2100/compiz-core/cherry_3236_3237 into lp:compiz-core

Proposed by Łukasz Zemczak
Status: Merged
Merged at revision: 3106
Proposed branch: lp:~sil2100/compiz-core/cherry_3236_3237
Merge into: lp:compiz-core
Diff against target: 101 lines (+29/-14)
3 files modified
include/core/option.h (+1/-0)
src/event.cpp (+20/-14)
src/option.cpp (+8/-0)
To merge this branch: bzr merge lp:~sil2100/compiz-core/cherry_3236_3237
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Review via email: mp+111213@code.launchpad.net

Commit message

Cherry-picked from compiz trunk (revs 3236, 3237):
Avoid constructing and destructing lots of strings on every single event, which was wasting lots of CPU (LP: #1005569)
Improve the fix for LP: #1005569:
Calling CompOption::setName should not implicitly construct a new string object every time, when the name is not changing.

This is also an alternative fix to the previous commit, but both together don't hurt either.

Description of the change

A cherry-pick of:
https://code.launchpad.net/~vanvugt/compiz/fix-1005569.1/+merge/107942 and
https://code.launchpad.net/~vanvugt/compiz/fix-1005569.2/+merge/107941

Original descriptions:

Calling CompOption::setName should not implicitly construct a new string
object every time, when the name is not changing. That wastes lots of CPU.
(LP: #1005569)

Avoid constructing and destructing lots of strings on every single event,
which was wasting lots of CPU (LP: #1005569)

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The ugly refactoring in src/event.cpp of mine is not required and only adds risk if you have the setName change already.

However, this fix is approved upstream, it's still smaller than many other changes being backported, and I'm running it right now and don't see any problems (other than bug 1014957).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/core/option.h'
--- include/core/option.h 2012-02-16 08:30:33 +0000
+++ include/core/option.h 2012-06-20 12:47:23 +0000
@@ -238,6 +238,7 @@
238 ~CompOption ();238 ~CompOption ();
239239
240 void setName (CompString name, Type type);240 void setName (CompString name, Type type);
241 void setName (const char *name, Type type);
241242
242 void reset ();243 void reset ();
243244
244245
=== modified file 'src/event.cpp'
--- src/event.cpp 2012-04-04 03:11:55 +0000
+++ src/event.cpp 2012-06-20 12:47:23 +0000
@@ -649,15 +649,19 @@
649bool649bool
650PrivateScreen::handleActionEvent (XEvent *event)650PrivateScreen::handleActionEvent (XEvent *event)
651{651{
652 static CompOption::Vector o (8);652 static CompOption::Vector o;
653 Window xid;653 Window xid;
654654
655 o[0].setName ("event_window", CompOption::TypeInt);655 if (o.empty ())
656 o[1].setName ("window", CompOption::TypeInt);656 {
657 o[2].setName ("modifiers", CompOption::TypeInt);657 o.resize (8);
658 o[3].setName ("x", CompOption::TypeInt);658 o[0].setName ("event_window", CompOption::TypeInt);
659 o[4].setName ("y", CompOption::TypeInt);659 o[1].setName ("window", CompOption::TypeInt);
660 o[5].setName ("root", CompOption::TypeInt);660 o[2].setName ("modifiers", CompOption::TypeInt);
661 o[3].setName ("x", CompOption::TypeInt);
662 o[4].setName ("y", CompOption::TypeInt);
663 o[5].setName ("root", CompOption::TypeInt);
664 }
661 o[6].reset ();665 o[6].reset ();
662 o[7].reset ();666 o[7].reset ();
663667
@@ -937,6 +941,14 @@
937 if (event->type == xkbEvent)941 if (event->type == xkbEvent)
938 {942 {
939 XkbAnyEvent *xkbEvent = (XkbAnyEvent *) event;943 XkbAnyEvent *xkbEvent = (XkbAnyEvent *) event;
944 static CompOption::Vector o;
945
946 if (o.empty ())
947 {
948 o.resize (8);
949 o[0].setName ("event_window", CompOption::TypeInt);
950 o[1].setName ("window", CompOption::TypeInt);
951 }
940952
941 if (xkbEvent->xkb_type == XkbStateNotify)953 if (xkbEvent->xkb_type == XkbStateNotify)
942 {954 {
@@ -944,13 +956,10 @@
944956
945 o[0].value ().set ((int) activeWindow);957 o[0].value ().set ((int) activeWindow);
946 o[1].value ().set ((int) activeWindow);958 o[1].value ().set ((int) activeWindow);
959 o[2].setName ("modifiers", CompOption::TypeInt);
947 o[2].value ().set ((int) stateEvent->mods);960 o[2].value ().set ((int) stateEvent->mods);
948
949 o[3].setName ("time", CompOption::TypeInt);961 o[3].setName ("time", CompOption::TypeInt);
950 o[3].value ().set ((int) xkbEvent->time);962 o[3].value ().set ((int) xkbEvent->time);
951 o[4].reset ();
952 o[5].reset ();
953 o[6].reset ();
954 o[7].value ().set ((int) xkbEvent->time);963 o[7].value ().set ((int) xkbEvent->time);
955964
956 if (stateEvent->event_type == KeyPress)965 if (stateEvent->event_type == KeyPress)
@@ -971,9 +980,6 @@
971 o[2].setName ("time", CompOption::TypeInt);980 o[2].setName ("time", CompOption::TypeInt);
972 o[2].value ().set ((int) xkbEvent->time);981 o[2].value ().set ((int) xkbEvent->time);
973 o[3].reset ();982 o[3].reset ();
974 o[4].reset ();
975 o[5].reset ();
976
977983
978 foreach (CompPlugin *p, CompPlugin::getPlugins ())984 foreach (CompPlugin *p, CompPlugin::getPlugins ())
979 {985 {
980986
=== modified file 'src/option.cpp'
--- src/option.cpp 2012-02-20 08:09:35 +0000
+++ src/option.cpp 2012-06-20 12:47:23 +0000
@@ -458,6 +458,14 @@
458 priv->type = type;458 priv->type = type;
459}459}
460460
461void
462CompOption::setName (const char *name, CompOption::Type type)
463{
464 if (priv->name != name)
465 priv->name = name;
466 priv->type = type;
467}
468
461CompString469CompString
462CompOption::name ()470CompOption::name ()
463{471{

Subscribers

People subscribed via source and target branches