[PATCH xserver 1/4] dix: Push UpdateCurrentTimeIf down out of the main loop

[Peter Hutterer]

On Fri, Apr 29, 2016 at 02:22:51PM -0400, Adam Jackson wrote:
> This was added in:
> 
>     commit 312910b4e34215aaa50fc0c6092684d5878dc32f
>     Author: Chase Douglas <chase.douglas at canonical.com>
>     Date:   Wed Apr 18 11:15:40 2012 -0700
> 
>         Update currentTime in dispatch loop
> 
> Unfortunately this is equivalent to calling GetTimeInMillis() once per
> request. In the absolute best case (as on Linux) you're only hitting the
> vDSO; on other platforms that's a syscall. Either way it puts a pretty
> hard ceiling on request throughput.
> 
> Instead, push the call down to the requests that need it; basically,
> grab processing and event generation.
> 
> Cc: Chase Douglas <chase.douglas at canonical.com>
> Cc: Peter Hutterer <peter.hutterer at who-t.net>
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  Xext/shape.c     |  1 +
>  Xi/extinit.c     |  2 ++
>  Xi/xiproperty.c  |  6 +++++-
>  dix/devices.c    | 16 ++++++++++------
>  dix/dispatch.c   |  5 +----
>  dix/enterleave.c |  1 +
>  dix/events.c     |  6 ++++++
>  dix/property.c   | 12 ++++++------
>  dix/selection.c  |  3 +++
>  randr/randr.c    |  2 ++
>  xfixes/cursor.c  |  1 +
>  xfixes/select.c  |  1 +
>  12 files changed, 39 insertions(+), 17 deletions(-)

[...]

> diff --git a/dix/events.c b/dix/events.c
> index efaf91d..0404eba 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -1822,6 +1822,7 @@ ProcAllowEvents(ClientPtr client)
>      REQUEST(xAllowEventsReq);
>  
>      REQUEST_SIZE_MATCH(xAllowEventsReq);
> +    UpdateCurrentTime();
>      time = ClientTimeToServerTime(stuff->time);
>  
>      mouse = PickPointer(client);
> @@ -2241,6 +2242,7 @@ DeliverEventsToWindow(DeviceIntPtr pDev, WindowPtr pWin, xEvent
>                                     this mask is the mask of the grab. */
>      int type = pEvents->u.u.type;
>  
> +    UpdateCurrentTimeIf();
>      /* Deliver to window owner */
>      if ((filter == CantBeFiltered) || core_get_type(pEvents) != 0) {
>          enum EventDeliveryState rc;
> @@ -4952,6 +4954,7 @@ ProcChangeActivePointerGrab(ClientPtr client)
>          return Success;
>      if (!SameClient(grab, client))
>          return Success;
> +    UpdateCurrentTime();
>      time = ClientTimeToServerTime(stuff->time);
>      if ((CompareTimeStamps(time, currentTime) == LATER) ||
>          (CompareTimeStamps(time, device->deviceGrab.grabTime) == EARLIER))
> @@ -5132,6 +5135,7 @@ ProcGrabKeyboard(ClientPtr client)
>      GrabMask mask;
>  
>      REQUEST_SIZE_MATCH(xGrabKeyboardReq);
> +    UpdateCurrentTime();
>  
>      mask.core = KeyPressMask | KeyReleaseMask;
>  

this one isn't needed, afaict. unless something else fails, GrabDevice()
calls UpdateCurrentTime() anyway before any time comparisons are done.
which also means we can drop the existing one from ProcGrabPointer, I don't
see how that one matters.

> @@ -5544,6 +5548,7 @@ ProcGrabButton(ClientPtr client)
>      int rc;
>  
>      REQUEST_SIZE_MATCH(xGrabButtonReq);
> +    UpdateCurrentTime();
>      if ((stuff->pointerMode != GrabModeSync) &&
>          (stuff->pointerMode != GrabModeAsync)) {
>          client->errorValue = stuff->pointerMode;
> @@ -5632,6 +5637,7 @@ ProcUngrabButton(ClientPtr client)
>      DeviceIntPtr ptr;
>  
>      REQUEST_SIZE_MATCH(xUngrabButtonReq);
> +    UpdateCurrentTime();
>      if ((stuff->modifiers != AnyModifier) &&
>          (stuff->modifiers & ~AllModifiersMask)) {
>          client->errorValue = stuff->modifiers;

these two aren't needed, passive grabs aren't time-sensitive

rest looks good though. feel free to use a
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> for the patch as-is,
trying to be perfect here is not going to make much of a difference.

Cheers,
   Peter