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

Submitted by Adam Jackson on Dec. 13, 2011, 10:31 p.m.

Details

Message ID 1323815513-6624-3-git-send-email-ajax@redhat.com
State Accepted
Commit 83a98543b58c661a22574a6f8d8f9d777c0955b8
Headers show

Commit Message

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(-)

Patch hide | download patch | download mbox

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)

Comments

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>