[PATCH 2/7] Add 'likely' and 'unlikely' macros

[Alan Coopersmith]

On 05/12/15 11:02 PM, Keith Packard wrote:
> walter harms <wharms at bfs.de> writes:
>
>> Just for my curiosity .. is there any benchmark that shows that this
>> actually improves something
>
> Yeah, I was measuring the new text code during development and saw some
> (tiny) improvements -- it's got a ton of branches in an inner loop
> loading glyphs pointers to hardware.

Speaking of which, something I've wondered about for a while, but never
had time to test, is if telling the server to optimize for valid requests
would make a difference, such as:

--- a/include/dix.h
+++ b/include/dix.h
@@ -67,25 +67,25 @@ SOFTWARE.
  #define ARRAY_SIZE(a)  (sizeof((a)) / sizeof((a)[0]))

  #define REQUEST_SIZE_MATCH(req)\
-    if ((sizeof(req) >> 2) != client->req_len)\
+    if (_X_UNLIKELY((sizeof(req) >> 2) != client->req_len)) \
           return(BadLength)

  #define REQUEST_AT_LEAST_SIZE(req) \
-    if ((sizeof(req) >> 2) > client->req_len )\
+    if (_X_UNLIKELY((sizeof(req) >> 2) > client->req_len )) \
           return(BadLength)

  #define REQUEST_AT_LEAST_EXTRA_SIZE(req, extra)  \
-    if (((sizeof(req) + ((uint64_t) extra)) >> 2) > client->req_len ) \
+    if (_X_UNLIKELY(((sizeof(req) + ((uint64_t) extra)) >> 2) > client->req_len
           return(BadLength)

  #define REQUEST_FIXED_SIZE(req, n)\
-    if (((sizeof(req) >> 2) > client->req_len) || \
-        (((n) >> 2) >= client->req_len) ||                              \
-        ((((uint64_t) sizeof(req) + (n) + 3) >> 2) != (uint64_t) client->req_le
+    if (_X_UNLIKELY(((sizeof(req) >> 2) > client->req_len) || \
+                    (((n) >> 2) >= client->req_len) ||                  \
+                    ((((uint64_t) sizeof(req) + (n) + 3) >> 2) != (uint64_t) cl
           return(BadLength)

  #define LEGAL_NEW_RESOURCE(id,client)\
-    if (!LegalNewID(id,client)) \
+    if (_X_UNLIKELY(!LegalNewID(id,client)))    \
      {\
         client->errorValue = id;\
          return BadIDChoice;\
@@ -94,12 +94,13 @@ SOFTWARE.
  #define VALIDATE_DRAWABLE_AND_GC(drawID, pDraw, mode)\
      {\
         int tmprc = dixLookupDrawable(&(pDraw), drawID, client, M_ANY, mode);\
-       if (tmprc != Success)\
+       if (_X_UNLIKELY(tmprc != Success)) \
             return tmprc;\
         tmprc = dixLookupGC(&(pGC), stuff->gc, client, DixUseAccess);\
-       if (tmprc != Success)\
+       if (_X_UNLIKELY(tmprc != Success))\
             return tmprc;\
-       if ((pGC->depth != pDraw->depth) || (pGC->pScreen != pDraw->pScreen))\
+       if (_X_UNLIKELY((pGC->depth != pDraw->depth) || \
+                        (pGC->pScreen != pDraw->pScreen))) \
             return BadMatch;\
      }\
      if (pGC->serialNumber != pDraw->serialNumber)\

Or optimizing for same-endian requests:

  #define WriteReplyToClient(pClient, size, pReply) { \
-   if ((pClient)->swapped) \
+   if (_X_UNLIKELY((pClient)->swapped)) \
        (*ReplySwapVector[((xReq *)(pClient)->requestBuffer)->reqType]) \
             (pClient, (int)(size), pReply); \
     else WriteToClient(pClient, (int)(size), (pReply)); }

  #define WriteSwappedDataToClient(pClient, size, pbuf) \
-   if ((pClient)->swapped) \
+   if (_X_UNLIKELY((pClient)->swapped)) \
        (*(pClient)->pSwapReplyFunc)(pClient, (int)(size), pbuf); \
     else WriteToClient(pClient, (int)(size), (pbuf));

If anyone wants to try it out and report back, please do so!

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc