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

Dan Nicholson dbn.lists at gmail.com
Sun Nov 23 18:06:54 PST 2008


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.

Signed-off-by: Dan Nicholson <dbn.lists at gmail.com>
---
 This takes Peter's suggestion and just copies the internal keymap structs
 when there aren't any changes. It seems to work in my testing and for all
 the cases I can think of.

 I plan to do some actual profiling to see what the effects are, but it
 seems like a win to not just blindly fork xkbcomp unless necessary.

 xkb/xkbInit.c |  182 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 118 insertions(+), 64 deletions(-)

diff --git a/xkb/xkbInit.c b/xkb/xkbInit.c
index 0b93e87..652cb6c 100644
--- a/xkb/xkbInit.c
+++ b/xkb/xkbInit.c
@@ -244,31 +244,57 @@ _X_EXPORT void
 XkbSetRulesDflts(char *rulesFile,char *model,char *layout,
 					char *variant,char *options)
 {
-    if (XkbRulesFile)
-	_XkbFree(XkbRulesFile);
-    XkbRulesFile= _XkbDupString(rulesFile);
-    rulesDefined= True;
+    Bool changed = False;
+
+    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
@@ -488,14 +514,12 @@ 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_cached_map)
+            XkbCopyKeymap(xkb_cached_map, xkbi->desc, False);
         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;
         }
@@ -592,8 +616,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
@@ -608,42 +630,78 @@ 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);
+        /* Store the map here so we can reuse the next time we're called.
+         * This is also picked back up in XkbInitDevice.
+         * Sigh. */
+        if (ok && xkb)
+            xkb_cached_map = xkb;
+    }
 
     if (ok && (xkb!=NULL)) {
 	KeyCode		minKC,maxKC;
@@ -677,15 +735,11 @@ XkbDescPtr              xkb;
 		pMods= tmpMods;
 	    }
 	}
-        /* Store the map here so we can pick it back up in XkbInitDevice.
-         * Sigh. */
-        xkb_cached_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;
     if ((pSyms==&tmpSyms)&&(pSyms->map!=NULL)) {
 	_XkbFree(pSyms->map);
 	pSyms->map= NULL;
-- 
1.5.6.5




More information about the xorg mailing list