[PATCH xserver 3/4] dix: Convert ResourceClientBits to a variable

[Adam Jackson]

This turns out to be a remarkably hot function in XID lookup.  Consider
the expansion of CLIENT_ID. We start with:

 #define RESOURCE_CLIENT_BITS ResourceClientBits()
 #define RESOURCE_AND_CLIENT_COUNT 29
 #define CLIENTOFFSET (RESOURCE_AND_CLIENT_COUNT - RESOURCE_CLIENT_BITS)

So that's effectively:

 #define CLIENTOFFSET (29 - RCB())

Then:

 #define RESOURCE_CLIENT_MASK \
    ((1 << RESOURCE_CLIENT_BITS) - 1) << CLIENTOFFSET

Is effectively:

 #define RESOURCE_CLIENT_MASK (((1 << RCB()) - 1) << (29 - RCB())

And:

 #define CLIENT_BITS(id) ((id) & RESOURCE_CLIENT_MASK)
 #define CLIENT_ID(id) ((int)(CLIENT_BITS(id) >> CLIENTOFFSET))

Is:

 #define CLIENT_ID(id) \
    ((id) & (((1 << RCB()) - 1) << (29 - RCB()))) >> (29 - RCB())

Since ResourceClientBits isn't marked __attribute__((pure)), the
optimizer isn't allowed to CSE those function calls, since it doesn't
know they don't have side effects. And, worse, ResourceClientBits
doesn't just return an integer, it computes the ilog2 afresh every time.

Instead we can just compute the value we need at argv parse.

v2: Ensure ResourceClientBits is initialized even when we don't pass
-maxclients (Alan Coopersmith)

Signed-off-by: Adam Jackson <ajax at redhat.com>
---
 dix/resource.c     | 23 -----------------------
 include/resource.h |  4 ++--
 os/osinit.c        |  1 +
 os/utils.c         | 13 +++++++++++++
 4 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/dix/resource.c b/dix/resource.c
index ad71b24..f06bf58 100644
--- a/dix/resource.c
+++ b/dix/resource.c
@@ -600,29 +600,6 @@ CreateNewResourceClass(void)
 
 static ClientResourceRec clientTable[MAXCLIENTS];
 
-static unsigned int
-ilog2(int val)
-{
-    int bits;
-
-    if (val <= 0)
-	return 0;
-    for (bits = 0; val != 0; bits++)
-	val >>= 1;
-    return bits - 1;
-}
-
-/*****************
- * ResourceClientBits
- *    Returns the client bit offset in the client + resources ID field
- *****************/
-
-unsigned int
-ResourceClientBits(void)
-{
-    return (ilog2(LimitClients));
-}
-
 /*****************
  * InitClientResources
  *    When a new client is created, call this to allocate space
diff --git a/include/resource.h b/include/resource.h
index 5871a4c..3cce6c6 100644
--- a/include/resource.h
+++ b/include/resource.h
@@ -85,10 +85,10 @@ typedef uint32_t RESTYPE;
 #define RT_LASTPREDEF	((RESTYPE)9)
 #define RT_NONE		((RESTYPE)0)
 
-extern _X_EXPORT unsigned int ResourceClientBits(void);
+extern _X_EXPORT unsigned int ResourceClientBits;
 /* bits and fields within a resource id */
 #define RESOURCE_AND_CLIENT_COUNT   29  /* 29 bits for XIDs */
-#define RESOURCE_CLIENT_BITS        ResourceClientBits() /* client field offset */
+#define RESOURCE_CLIENT_BITS        ResourceClientBits /* client field offset */
 #define CLIENTOFFSET	    (RESOURCE_AND_CLIENT_COUNT - RESOURCE_CLIENT_BITS)
 /* resource field */
 #define RESOURCE_ID_MASK	((1 << CLIENTOFFSET) - 1)
diff --git a/os/osinit.c b/os/osinit.c
index 54b39a0..92aced3 100644
--- a/os/osinit.c
+++ b/os/osinit.c
@@ -88,6 +88,7 @@ int limitNoFile = -1;
 
 /* The actual user defined max number of clients */
 int LimitClients = LIMITCLIENTS;
+unsigned int ResourceClientBits;
 
 static OsSigWrapperPtr OsSigWrapper = NULL;
 
diff --git a/os/utils.c b/os/utils.c
index e48d9f8..8b81d39 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -513,6 +513,18 @@ AdjustWaitForDelay(void *waitTime, unsigned long newdelay)
     }
 }
 
+static unsigned int
+ilog2(int val)
+{
+    int bits;
+
+    if (val <= 0)
+	return 0;
+    for (bits = 0; val != 0; bits++)
+	val >>= 1;
+    return bits - 1;
+}
+
 void
 UseMsg(void)
 {
@@ -1061,6 +1073,7 @@ ProcessCommandLine(int argc, char *argv[])
             FatalError("Unrecognized option: %s\n", argv[i]);
         }
     }
+    ResourceClientBits = ilog2(LimitClients);
 }
 
 /* Implement a simple-minded font authorization scheme.  The authorization
-- 
2.7.4