[PATCH] xkb: Use cached XKB keymap when rules haven't changed

Dan Nicholson dbn.lists at gmail.com
Tue Jan 6 07:06:01 PST 2009


Rather than compiling a new keymap every time XkbInitKeyboardDeviceStruct is
called, cache the previous keymap and reuse it if the rules have not been
changed. When XkbSetRulesDflts is called, the cached map is invalidated if it
differs from the previous default rules.

The map is also invalidated if named components are supplied. This could be
fixed by caching Ktcsg from the previous run and comparing.

The meaning of xkb_cached_map has been changed in the code to reporesent the
keymap reused over multiple calls. The freshly compiled keymap used by
XkbInitDevice has been renamed to xkb_initial_map.

Signed-off-by: Dan Nicholson <dbn.lists at gmail.com>
---
 This has been rebased to master over Peter's recent XkbSetRulesDflts
 changes. I was able to do a bit of timing using some truly impressive
 profiling code[1]. Here are some results I gather on my Thinkpad X60 for
 cold and warm starts. 5 keyboard devices are initialized. The time below
 effectively measures the amount of time for xkbcomp to run and then to
 parse and apply the .xkm file.
 
 master, cold start:
 (II) XKB: 0.5534 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0700 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0510 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0360 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0366 seconds in XkbInitKeyboardDeviceStruct
 Total: 0.7470 seconds
 
 master, warm start:
 (II) XKB: 0.0358 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0370 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0413 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0356 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0359 seconds in XkbInitKeyboardDeviceStruct
 Total: 0.1856 seconds
 
 patched, cold start:
 (II) XKB: 0.5387 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0898 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0003 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0362 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0003 seconds in XkbInitKeyboardDeviceStruct
 Total: 0.6653 seconds
 
 patched, warm start:
 (II) XKB: 0.0364 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0359 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0003 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0468 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0002 seconds in XkbInitKeyboardDeviceStruct
 Total: 0.1196 seconds
 
 So, using the cached map saves ~35-45 ms per device when applicable. The
 initial keymap still needs to be created through xkbcomp, and this is
 really slow on a cold start. Just for fun, here's a run where I got both
 the core keyboard and all the HAL devices to use the same RMLVO so the
 single cached map was most effective.
 
 (II) XKB: 0.0359 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0002 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0004 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0004 seconds in XkbInitKeyboardDeviceStruct
 (II) XKB: 0.0004 seconds in XkbInitKeyboardDeviceStruct
 Total: 0.0373 seconds
 
 Alright to push?
 
 xkb/xkbInit.c |  185 ++++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 124 insertions(+), 61 deletions(-)

 [1] http://people.freedesktop.org/~dbn/patches/xserver-lame-xkb-prof.patch

diff --git a/xkb/xkbInit.c b/xkb/xkbInit.c
index 3f07af5..83a3831 100644
--- a/xkb/xkbInit.c
+++ b/xkb/xkbInit.c
@@ -129,6 +129,7 @@ static char *		XkbLayoutUsed=	NULL;
 static char *		XkbVariantUsed=	NULL;
 static char *		XkbOptionsUsed=	NULL;
 
+static XkbDescPtr       xkb_initial_map = NULL;
 static XkbDescPtr       xkb_cached_map = NULL;
 
 Bool			noXkbExtension=		XKB_DFLT_DISABLED;
@@ -251,6 +252,8 @@ void
 XkbSetRulesDflts(char *rulesFile,char *model,char *layout,
 					char *variant,char *options)
 {
+    Bool changed = False;
+
     if (!rulesFile && !XkbRulesFile)
     {
 	LogMessage(X_WARNING, "[xkb] No rule given, and no previous rule "
@@ -260,33 +263,56 @@ XkbSetRulesDflts(char *rulesFile,char *model,char *layout,
     }
 
     if (rulesFile) {
-	if (XkbRulesFile)
-	    _XkbFree(XkbRulesFile);
-	XkbRulesFile= _XkbDupString(rulesFile);
-	rulesDefined= True;
+        if (!XkbRulesFile || strcmp(rulesFile, XkbRulesFile) != 0) {
+            changed = True;
+            if (XkbRulesFile)
+                _XkbFree(XkbRulesFile);
+            XkbRulesFile = _XkbDupString(rulesFile);
+        }
+        rulesDefined = True;
     }
 
     if (model) {
-	if (XkbModelDflt)
-	    _XkbFree(XkbModelDflt);
-	XkbModelDflt= _XkbDupString(model);
+        if (!XkbModelDflt || strcmp(model, XkbModelDflt) != 0) {
+            changed = True;
+            if (XkbModelDflt)
+                _XkbFree(XkbModelDflt);
+            XkbModelDflt = _XkbDupString(model);
+        }
     }
+
     if (layout) {
-	if (XkbLayoutDflt)
-	    _XkbFree(XkbLayoutDflt);
-	XkbLayoutDflt= _XkbDupString(layout);
+        if (!XkbLayoutDflt || strcmp(layout, XkbLayoutDflt) != 0) {
+            changed = True;
+            if (XkbLayoutDflt)
+                _XkbFree(XkbLayoutDflt);
+            XkbLayoutDflt = _XkbDupString(layout);
+        }
     }
+
     if (variant) {
-	if (XkbVariantDflt)
-	    _XkbFree(XkbVariantDflt);
-	XkbVariantDflt= _XkbDupString(variant);
+        if (!XkbVariantDflt || strcmp(variant, XkbVariantDflt) != 0) {
+            changed = True;
+            if (XkbVariantDflt)
+                _XkbFree(XkbVariantDflt);
+            XkbVariantDflt = _XkbDupString(variant);
+        }
     }
+
     if (options) {
-	if (XkbOptionsDflt)
-	    _XkbFree(XkbOptionsDflt);
-	XkbOptionsDflt= _XkbDupString(options);
+        if (!XkbOptionsDflt || strcmp(options, XkbOptionsDflt) != 0) {
+            changed = True;
+            if (XkbOptionsDflt)
+                _XkbFree(XkbOptionsDflt);
+            XkbOptionsDflt = _XkbDupString(options);
+        }
+    }
+
+    /* If the rules have changed, remove the cached map. */
+    if (changed && xkb_cached_map) {
+        XkbFreeKeyboard(xkb_cached_map, XkbAllComponentsMask, True);
+        xkb_cached_map = NULL;
     }
-    return;
 }
 
 void
@@ -303,6 +329,8 @@ XkbDeleteRulesDflts()
     _XkbFree(XkbOptionsDflt);
     XkbOptionsDflt = NULL;
 
+    XkbFreeKeyboard(xkb_initial_map, XkbAllComponentsMask, True);
+    xkb_initial_map = NULL;
     XkbFreeKeyboard(xkb_cached_map, XkbAllComponentsMask, True);
     xkb_cached_map = NULL;
 }
@@ -506,14 +534,17 @@ XkbEventCauseRec	cause;
     if ( xkbi ) {
 	XkbDescPtr	xkb;
 
-        if (xkb_cached_map) {
-            xkbi->desc = xkb_cached_map;
-            xkb_cached_map = NULL;
+        xkbi->desc = XkbAllocKeyboard();
+        if (!xkbi->desc)
+            FatalError("Couldn't allocate keyboard description\n");
+        if (xkb_initial_map) {
+            if(!XkbCopyKeymap(xkb_initial_map, xkbi->desc, False))
+                FatalError("XKB: Couldn't copy initial keymap\n");
+            xkbi->desc->defined = xkb_initial_map->defined;
+            xkbi->desc->flags = xkb_initial_map->flags;
+            xkbi->desc->device_spec = xkb_initial_map->device_spec;
         }
         else {
-            xkbi->desc= XkbAllocKeyboard();
-            if (!xkbi->desc)
-                FatalError("Couldn't allocate keyboard description\n");
             xkbi->desc->min_key_code = pXDev->key->curKeySyms.minKeyCode;
             xkbi->desc->max_key_code = pXDev->key->curKeySyms.maxKeyCode;
         }
@@ -610,8 +641,6 @@ XkbDescPtr              xkb;
 	return False;
     pSyms= pSymsIn;
     pMods= pModsIn;
-    bzero(&defs,sizeof(XkbRF_VarDefsRec));
-    rules= XkbGetRulesDflts(&defs);
 
     /*
      * The strings are duplicated because it is not guaranteed that
@@ -626,42 +655,76 @@ XkbDescPtr              xkb;
     if (names->geometry) names->geometry = _XkbDupString(names->geometry);
     if (names->symbols) names->symbols = _XkbDupString(names->symbols);
 
-    if (defs.model && defs.layout && rules) {
-	XkbComponentNamesRec	rNames;
-	bzero(&rNames,sizeof(XkbComponentNamesRec));
-	if (XkbDDXNamesFromRules(dev,rules,&defs,&rNames)) {
-	    if (rNames.keycodes) {
-		if (!names->keycodes)
-		    names->keycodes =  rNames.keycodes;
-		else
-		    _XkbFree(rNames.keycodes);
-	    }
-	    if (rNames.types) {
-		if (!names->types)
-		    names->types = rNames.types;
-		else  _XkbFree(rNames.types);
-	    }
-	    if (rNames.compat) {
-		if (!names->compat) 
-		    names->compat =  rNames.compat;
-		else  _XkbFree(rNames.compat);
-	    }
-	    if (rNames.symbols) {
-		if (!names->symbols)
-		    names->symbols =  rNames.symbols;
-		else _XkbFree(rNames.symbols);
-	    }
-	    if (rNames.geometry) {
-		if (!names->geometry)
-		    names->geometry = rNames.geometry;
-		else _XkbFree(rNames.geometry);
-	    }
-	    XkbSetRulesUsed(&defs);
-	}
+    /*
+     * If named components were passed, we can't guarantee the validity
+     * of the cached keymap.
+     */
+    if (xkb_cached_map &&
+        (names->keymap || names->keycodes || names->types ||
+         names->compat || names->geometry || names->symbols)) {
+        XkbFreeKeyboard(xkb_cached_map, XkbAllComponentsMask, True);
+        xkb_cached_map = NULL;
+    }
+
+    /*
+     * If we have a cached compiled map, then the rules have not changed
+     * and we can avoid recompiling.
+     */
+    if (xkb_cached_map) {
+        LogMessageVerb(X_INFO, 4, "XKB: Reusing cached keymap\n");
+        xkb = xkb_cached_map;
+        ok = True;
     }
+    else {
+        bzero(&defs, sizeof(XkbRF_VarDefsRec));
+        rules = XkbGetRulesDflts(&defs);
+
+        if (defs.model && defs.layout && rules) {
+            XkbComponentNamesRec rNames;
+            bzero(&rNames, sizeof(XkbComponentNamesRec));
+
+            if (XkbDDXNamesFromRules(dev, rules, &defs, &rNames)) {
+                if (rNames.keycodes) {
+                    if (!names->keycodes)
+                        names->keycodes = rNames.keycodes;
+                    else
+                        _XkbFree(rNames.keycodes);
+                }
+                if (rNames.types) {
+                    if (!names->types)
+                        names->types = rNames.types;
+                    else
+                        _XkbFree(rNames.types);
+                }
+                if (rNames.compat) {
+                    if (!names->compat)
+                        names->compat = rNames.compat;
+                    else
+                        _XkbFree(rNames.compat);
+                }
+                if (rNames.symbols) {
+                    if (!names->symbols)
+                        names->symbols = rNames.symbols;
+                    else
+                        _XkbFree(rNames.symbols);
+                }
+                if (rNames.geometry) {
+                    if (!names->geometry)
+                        names->geometry = rNames.geometry;
+                    else
+                        _XkbFree(rNames.geometry);
+                }
+                XkbSetRulesUsed(&defs);
+            }
+        }
 
-    ok = (Bool) XkbDDXLoadKeymapByNames(dev,names,XkmAllIndicesMask,0,
-                                        &xkb,name,PATH_MAX);
+        ok = (Bool) XkbDDXLoadKeymapByNames(dev, names, XkmAllIndicesMask,
+                                            0, &xkb, name, PATH_MAX);
+
+        /* Cache the map so it can be reused the next time. */
+        if (ok && xkb)
+            xkb_cached_map = xkb;
+    }
 
     if (ok && (xkb!=NULL)) {
 	KeyCode		minKC,maxKC;
@@ -697,13 +760,13 @@ XkbDescPtr              xkb;
 	}
         /* Store the map here so we can pick it back up in XkbInitDevice.
          * Sigh. */
-        xkb_cached_map = xkb;
+        xkb_initial_map = xkb;
     }
     else {
 	LogMessage(X_WARNING, "Couldn't load XKB keymap, falling back to pre-XKB keymap\n");
     }
     ok= InitKeyboardDeviceStruct((DevicePtr)dev,pSyms,pMods,bellProc,ctrlProc);
-    xkb_cached_map = NULL;
+    xkb_initial_map = NULL;
     if ((pSyms==&tmpSyms)&&(pSyms->map!=NULL)) {
 	_XkbFree(pSyms->map);
 	pSyms->map= NULL;
-- 
1.5.6.6




More information about the xorg mailing list