Patchwork [2/4] dix: Tune dtrace hooks around Dispatch

login
register
mail settings
Submitter Adam Jackson
Date Dec. 13, 2011, 10:31 p.m.
Message ID <1323815513-6624-3-git-send-email-ajax@redhat.com>
Download mbox | patch
Permalink /patch/8296/
State Accepted
Commit 83a98543b58c661a22574a6f8d8f9d777c0955b8
Headers show

Comments

Adam Jackson - Dec. 13, 2011, 10:31 p.m.
Don't call LookupMajorName if the hooks aren't active, it's quite expensive.

Before:
40000000 trep @   0.0009 msec (1087458.5/sec): PutImage 10x10 square
60000000 trep @   0.0005 msec (2012238.6/sec): ShmPutImage 10x10 square

After:
40000000 trep @   0.0009 msec (1109091.3/sec): PutImage 10x10 square
60000000 trep @   0.0005 msec (2072652.2/sec): ShmPutImage 10x10 square

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 dix/dispatch.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
Alan Coopersmith - Dec. 13, 2011, 10:54 p.m.
On 12/13/11 14:31, Adam Jackson wrote:
> Don't call LookupMajorName if the hooks aren't active, it's quite expensive.

It was a simple table lookup when I added it, before the conversion to the
registry functions in commit 996b621bec.

> @@ -425,9 +425,11 @@ Dispatch(void)
>   			client->minorOp = ext->MinorOpcode(client);
>   		}
>   #ifdef XSERVER_DTRACE
> -		XSERVER_REQUEST_START(LookupMajorName(client->majorOp), client->majorOp,
> -			      ((xReq *)client->requestBuffer)->length,
> -			      client->index, client->requestBuffer);
> +		if (XSERVER_REQUEST_START_ENABLED())
> +		    XSERVER_REQUEST_START(LookupMajorName(client->majorOp),
> +					  client->majorOp,
> +					  ((xReq *)client->requestBuffer)->length,
> +					  client->index, client->requestBuffer);
>   #endif
>   		if (result>  (maxBigRequestSize<<  2))
>   		    result = BadLength;
> @@ -438,8 +440,10 @@ Dispatch(void)
>   		    XaceHookAuditEnd(client, result);
>   		}
>   #ifdef XSERVER_DTRACE
> -		XSERVER_REQUEST_DONE(LookupMajorName(client->majorOp), client->majorOp,
> -			      client->sequence, client->index, result);
> +		if (XSERVER_REQUEST_DONE_ENABLED())
> +		    XSERVER_REQUEST_DONE(LookupMajorName(client->majorOp),
> +					 client->majorOp, client->sequence,
> +					 client->index, result);
>   #endif
>
>   		if (client->noClientException != Success)

Seems like there's room for further optimization in the enabled case by caching 
the LookupMajorName result, but that's far less important than the disabled 
case, so just doing this for now is great.

Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Patch

diff --git a/dix/dispatch.c b/dix/dispatch.c
index b39271f..bb95368 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -425,9 +425,11 @@  Dispatch(void)
 			client->minorOp = ext->MinorOpcode(client);
 		}
 #ifdef XSERVER_DTRACE
-		XSERVER_REQUEST_START(LookupMajorName(client->majorOp), client->majorOp,
-			      ((xReq *)client->requestBuffer)->length,
-			      client->index, client->requestBuffer);
+		if (XSERVER_REQUEST_START_ENABLED())
+		    XSERVER_REQUEST_START(LookupMajorName(client->majorOp),
+					  client->majorOp,
+					  ((xReq *)client->requestBuffer)->length,
+					  client->index, client->requestBuffer);
 #endif
 		if (result > (maxBigRequestSize << 2))
 		    result = BadLength;
@@ -438,8 +440,10 @@  Dispatch(void)
 		    XaceHookAuditEnd(client, result);
 		}
 #ifdef XSERVER_DTRACE
-		XSERVER_REQUEST_DONE(LookupMajorName(client->majorOp), client->majorOp,
-			      client->sequence, client->index, result);
+		if (XSERVER_REQUEST_DONE_ENABLED())
+		    XSERVER_REQUEST_DONE(LookupMajorName(client->majorOp),
+					 client->majorOp, client->sequence,
+					 client->index, result);
 #endif
 
 		if (client->noClientException != Success)