Merge lp:~vanvugt/compiz-core/lastMotionTime into lp:compiz-core

[Daniel van Vugt]

Faster window movement, by faster MotionNotify handling.

PrivateScreen::queueEvents has always been designed to try and deduplicate
MotionNotify events so as to reduce CPU load. However the old algorithm
fails to deduplicate events most of the time as the event loop is fast
enough that only one such event happened per iteration. Hence there was
nothing to deduplicate against.

This new algorithm deduplicates motion events across event loop iterations,
at least halving, usually quartering, the number of motion events that
need to be processed. This translates directly to lower CPU use.

[Alan Griffiths (alan-griffiths)]

AFAICS There would be no cost to reducing the scope of peekEvent to its use.

review: Approve

[Daniel van Vugt (vanvugt)]

Sam had reasonable concerns about this branch. So consider it on hold for now.

[Sam Spilsbury (smspillaz)]

I don't really like the idea the idea of what this branch is doing for two reasons

1) It shouldn't be a user option to control the internals of core, sampling of motion events is definitely one of those
2) When we are processing events we need to have a correct idea of server side state at the time the event was emitted. For example, it is theoretically possible that we need to know the cursor position at the time a window geometry changes or at the time a window is mapped (this is relevant for the focus code). Its also relevant to things like, eg, edge detection.

Throttling processing of motion events is a good idea, however it shouldn't be done globally and really should be done on a plugin-by-plugin basis. Releasable queues are one way to do it. Throttling events like this takes away that information where it might otherwise be necessary and could result in very difficult to debug issues.

review: Disapprove

[Daniel van Vugt (vanvugt)]

All good points.

This branch really was just a proof of concept. And it's now redundant as Sam's shadow geometry optimizations have fixed window movement performance:
https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_969101/+merge/100270

Unmerged revisions

3085. By Daniel van Vugt on 2012-04-04

Faster window movement, by faster MotionNotify handling.

PrivateScreen::queueEvents has always been designed to try and deduplicate
MotionNotify events so as to reduce CPU load. However the old algorithm
fails to deduplicate events most of the time as the event loop is fast
enough that only one such event happened per iteration. Hence there was
nothing to deduplicate against.

This new algorithm deduplicates motion events across event loop iterations,
at least halving, usually quartering, the number of motion events that
need to be processed. This translates directly to lower CPU use.

=== modified file 'metadata/core.xml.in'
--- metadata/core.xml.in	2012-03-09 16:13:05 +0000
+++ metadata/core.xml.in	2012-04-04 08:03:19 +0000
@@ -49,6 +49,13 @@
 		       they will resume their past internal state when reloaded</_long>
 		<default>false</default>
 	    </option>
+	    <option name="max_motion_rate" type="int">
+		<_short>Maximum motion frame rate</_short>
+		<_long>Maximum frame rate at which to handle motion events. Events arriving faster than this will be skipped. Zero = infinite (old behaviour).</_long>
+		<default>60</default>
+		<min>0</min>
+		<max>1000</max>
+	    </option>
 	    <group>
 		<_short>Display Settings</_short>
 			<option name="overlapping_outputs" type="int">
 
=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-03-29 16:54:46 +0000
+++ src/privatescreen.h	2012-04-04 08:03:19 +0000
@@ -995,6 +995,7 @@
 	CompTimer               edgeDelayTimer;
 	CompDelayedEdgeSettings edgeDelaySettings;
 	Window	xdndWindow;
+	Time    lastMotionTime;
 };
 
 class CompManager
 
=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-03-30 06:26:33 +0000
+++ src/screen.cpp	2012-04-04 08:03:19 +0000
@@ -747,24 +747,19 @@
 {
     std::list <XEvent> events;
 
+    int maxMotionRate = optionGetMaxMotionRate ();
+    int minMotionTime = (maxMotionRate <= 0) ? 0 : 1000 / maxMotionRate;
+
     while (XPending (dpy))
     {
-	XEvent event, peekEvent;
+	XEvent event;
 	XNextEvent (dpy, &event);
 
-	/* Skip to the last MotionNotify
-	 * event in this sequence */
 	if (event.type == MotionNotify)
 	{
-	    while (XPending (dpy))
-	    {
-		XPeekEvent (dpy, &peekEvent);
-
-		if (peekEvent.type != MotionNotify)
-		    break;
-
-		XNextEvent (dpy, &event);
-	    }
+	    if ((event.xmotion.time - lastMotionTime) < minMotionTime)
+		continue;
+	    lastMotionTime = event.xmotion.time;
 	}
 
 	events.push_back (event);
@@ -5127,7 +5122,8 @@
     initialized (false),
     edgeWindow (None),
     edgeDelayTimer (),
-    xdndWindow (None)
+    xdndWindow (None),
+    lastMotionTime (0)
 {
     pingTimer.setCallback (
 	boost::bind (&PrivateScreen::handlePingTimeout, this));