[PATCH] XKB: cache xkbcomp output for fast start-up v3 for 1.6.0
Alan Coopersmith
Alan.Coopersmith at Sun.COM
Fri Apr 3 07:55:34 PDT 2009
Is there a race condition if two servers start at once (two vt's or
multiseat, etc.) and the first server has started writing the .xkm
but not finished when the second one finds it and tries to read?
(Perhaps it should write to a temporary name and then rename() when
writing is complete.) We had problems with that with the old
code before we switched to exclusively using server-<display>.xkm
naming.
I'd also rather see the Sha1 helper function moved to a file under
the os dir and shared by both render/glyph.c & this code instead
of duplicating the code here and giving people two places to make
the same updates when adding new sha1 library implementations.
-alan-
Yan Li wrote:
> xkbcomp outputs will be cached in files with hashed keymap as
> names. This saves boot time for around 1s on commodity netbooks.
>
> Signed-off-by: Yan Li <yan.i.li at intel.com>
> ---
> configure.ac | 6 +-
> xkb/README.compiled | 8 ++--
> xkb/ddxLoad.c | 141 +++++++++++++++++++++++++++++++++++++++-----------
> 3 files changed, 117 insertions(+), 38 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index f2718b8..5d8a6e5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -476,9 +476,9 @@ AC_ARG_WITH(default-font-path, AS_HELP_STRING([--with-default-font-path=PATH], [
> AC_ARG_WITH(xkb-path, AS_HELP_STRING([--with-xkb-path=PATH], [Path to XKB base dir (default: ${datadir}/X11/xkb)]),
> [ XKBPATH="$withval" ],
> [ XKBPATH="${datadir}/X11/xkb" ])
> -AC_ARG_WITH(xkb-output, AS_HELP_STRING([--with-xkb-output=PATH], [Path to XKB output dir (default: ${datadir}/X11/xkb/compiled)]),
> +AC_ARG_WITH(xkb-output, AS_HELP_STRING([--with-xkb-output=PATH], [Path to XKB output dir (default: ${localstatedir}/cache/xkb)]),
> [ XKBOUTPUT="$withval" ],
> - [ XKBOUTPUT="compiled" ])
> + [ XKBOUTPUT="${localstatedir}/cache/xkb" ])
> AC_ARG_WITH(serverconfig-path, AS_HELP_STRING([--with-serverconfig-path=PATH],
> [Directory where ancillary server config files are installed (default: ${libdir}/xorg)]),
> [ SERVERCONFIG="$withval" ],
> @@ -1753,7 +1753,7 @@ AC_DEFINE_DIR(XKB_BIN_DIRECTORY, bindir, [Path to XKB bin dir])
> XKBOUTPUT_FIRSTCHAR=`echo $XKBOUTPUT | cut -b 1`
>
> if [[ x$XKBOUTPUT_FIRSTCHAR != x/ ]] ; then
> - XKBOUTPUT="$XKB_BASE_DIRECTORY/$XKBOUTPUT"
> + AC_MSG_ERROR([xkb-output must be an absolute path.])
> fi
>
> # XKM_OUTPUT_DIR (used in code) must end in / or file names get hosed
> diff --git a/xkb/README.compiled b/xkb/README.compiled
> index 71caa2f..a4a2ae0 100644
> --- a/xkb/README.compiled
> +++ b/xkb/README.compiled
> @@ -4,10 +4,10 @@ current keymap and/or any scratch keymaps used by clients. The X server
> or some other tool might destroy or replace the files in this directory,
> so it is not a safe place to store compiled keymaps for long periods of
> time. The default keymap for any server is usually stored in:
> - X<num>-default.xkm
> -where <num> is the display number of the server in question, which makes
> -it possible for several servers *on the same host* to share the same
> -directory.
> + server-<SHA1>.xkm
> +
> +where <SHA1> is the SHA1 hash of keymap source, so that compiled
> +keymap of different keymap sources are stored in different files.
>
> Unless the X server is modified, sharing this directory between servers on
> different hosts could cause problems.
> diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
> index 4d5dfb6..8819353 100644
> --- a/xkb/ddxLoad.c
> +++ b/xkb/ddxLoad.c
> @@ -32,6 +32,12 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
> #include <xkb-config.h>
> #endif
>
> +#ifdef HAVE_SHA1_IN_LIBMD /* Use libmd for SHA1 */
> +# include <sha1.h>
> +#else /* Use OpenSSL's libcrypto */
> +# include <stddef.h> /* buggy openssl/sha.h wants size_t */
> +# include <openssl/sha.h>
> +#endif
> #include <stdio.h>
> #include <ctype.h>
> #define NEED_EVENTS 1
> @@ -52,18 +58,6 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
> #include <paths.h>
> #endif
>
> - /*
> - * If XKM_OUTPUT_DIR specifies a path without a leading slash, it is
> - * relative to the top-level XKB configuration directory.
> - * Making the server write to a subdirectory of that directory
> - * requires some work in the general case (install procedure
> - * has to create links to /var or somesuch on many machines),
> - * so we just compile into /usr/tmp for now.
> - */
> -#ifndef XKM_OUTPUT_DIR
> -#define XKM_OUTPUT_DIR "compiled/"
> -#endif
> -
> #define PRE_ERROR_MSG "\"The XKEYBOARD keymap compiler (xkbcomp) reports:\""
> #define ERROR_PREFIX "\"> \""
> #define POST_ERROR_MSG1 "\"Errors from xkbcomp are not fatal to the X server\""
> @@ -179,6 +173,45 @@ OutputDirectory(
> }
>
> static Bool
> +Sha1Asc(char sha1Asc[SHA_DIGEST_LENGTH*2+1], const char * input)
> +{
> + int i;
> + unsigned char sha1[SHA_DIGEST_LENGTH];
> +
> +#ifdef HAVE_SHA1_IN_LIBMD /* Use libmd for SHA1 */
> + SHA1_CTX ctx;
> +
> + SHA1Init (&ctx);
> + SHA1Update (&ctx, input, strlen(input));
> + SHA1Final (sha1, &ctx);
> +#else /* Use OpenSSL's libcrypto */
> + SHA_CTX ctx;
> + int success;
> +
> + success = SHA1_Init (&ctx);
> + if (! success)
> + return BadAlloc;
> +
> + success = SHA1_Update (&ctx, input, strlen(input));
> + if (! success)
> + return BadAlloc;
> +
> + success = SHA1_Final (sha1, &ctx);
> + if (! success)
> + return BadAlloc;
> +#endif
> +
> + /* convert sha1 to sha1_asc */
> + for(i=0; i<SHA_DIGEST_LENGTH; ++i) {
> + sprintf(sha1Asc+i*2, "%02X", sha1[i]);
> + }
> +
> + return Success;
> +}
> +
> +/* call xkbcomp and compile XKB keymap, return xkm file name in
> + nameRtrn */
> +static Bool
> XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> XkbComponentNamesPtr names,
> unsigned want,
> @@ -187,7 +220,10 @@ XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> int nameRtrnLen)
> {
> FILE * out;
> - char *buf = NULL, keymap[PATH_MAX], xkm_output_dir[PATH_MAX];
> + char * buf = NULL, xkmfile[PATH_MAX], xkm_output_dir[PATH_MAX];
> + char * canonicalXkmfileName;
> + char sha1Asc[SHA_DIGEST_LENGTH*2+1], xkbKeyMapBuf[1024];
> + int ret;
>
> const char *emptystring = "";
> const char *xkbbasedirflag = emptystring;
> @@ -198,16 +234,65 @@ XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> /* WIN32 has no popen. The input must be stored in a file which is
> used as input for xkbcomp. xkbcomp does not read from stdin. */
> char tmpname[PATH_MAX];
> - const char *xkmfile = tmpname;
> + const char *xkbfile = tmpname;
> #else
> - const char *xkmfile = "-";
> + const char *xkbfile = "-";
> +#endif
> +
> + /* Write keymap source (xkbfile) to memory buffer `xkbKeyMapBuf',
> + of which SHA1 is generated and used as result xkm file name */
> + memset(xkbKeyMapBuf, 0, sizeof(xkbKeyMapBuf));
> + out = fmemopen(xkbKeyMapBuf, sizeof(xkbKeyMapBuf), "w");
> + if (NULL == out) {
> + ErrorF("[xkb] Open xkbKeyMapBuf for writing failed\n");
> + return False;
> + }
> + ret = XkbWriteXKBKeymapForNames(out, names, xkb, want, need);
> + fclose(out);
> +#ifdef DEBUG
> + if (xkbDebugFlags) {
> + ErrorF("[xkb] XkbDDXCompileKeymapByNames compiling keymap:\n");
> + fputs(xkbKeyMapBuf, stderr);
> + }
> #endif
> + if (!ret) {
> + ErrorF("[xkb] Generating XKB Keymap failed, giving up compiling keymap\n");
> + return False;
> + }
>
> - snprintf(keymap, sizeof(keymap), "server-%s", display);
> + DebugF("[xkb] computing SHA1 of keymap\n");
> + if (Success == Sha1Asc(sha1Asc, xkbKeyMapBuf)) {
> + snprintf(xkmfile, sizeof(xkmfile), "server-%s", sha1Asc);
> + }
> + else {
> + ErrorF("[xkb] Computing SHA1 of keymap failed, "
> + "using display name instead as xkm file name\n");
> + snprintf(xkmfile, sizeof(xkmfile), "server-%s", display);
> + }
>
> - XkbEnsureSafeMapName(keymap);
> + XkbEnsureSafeMapName(xkmfile);
> OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir));
>
> + /* set nameRtrn, fail if it's too small */
> + if ((strlen(xkmfile)+1 > nameRtrnLen) && nameRtrn) {
> + ErrorF("[xkb] nameRtrn too small to hold xkmfile name\n");
> + return False;
> + }
> + strncpy(nameRtrn, xkmfile, nameRtrnLen);
> +
> + /* if the xkm file already exists, reuse it */
> + canonicalXkmfileName = Xprintf("%s%s.xkm", xkm_output_dir, xkmfile);
> + if (access(canonicalXkmfileName, R_OK) == 0) {
> + /* yes, we can reuse the old xkm file */
> + LogMessage(X_INFO, "XKB: reuse xkmfile %s\n", canonicalXkmfileName);
> + xfree(canonicalXkmfileName);
> + return True;
> + }
> + LogMessage(X_INFO, "XKB: generating xkmfile %s\n", canonicalXkmfileName);
> + xfree(canonicalXkmfileName);
> +
> + /* continue to call xkbcomp to compile the keymap */
> +
> #ifdef WIN32
> strcpy(tmpname, Win32TempDir());
> strcat(tmpname, "\\xkb_XXXXXX");
> @@ -235,9 +320,9 @@ XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> xkbbindir, xkbbindirsep,
> ( (xkbDebugFlags < 2) ? 1 :
> ((xkbDebugFlags > 10) ? 10 : (int)xkbDebugFlags) ),
> - xkbbasedirflag, xkmfile,
> + xkbbasedirflag, xkbfile,
> PRE_ERROR_MSG, ERROR_PREFIX, POST_ERROR_MSG1,
> - xkm_output_dir, keymap);
> + xkm_output_dir, xkmfile);
>
> if (xkbbasedirflag != emptystring) {
> xfree(xkbbasedirflag);
> @@ -250,13 +335,12 @@ XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> #endif
>
> if (out!=NULL) {
> -#ifdef DEBUG
> - if (xkbDebugFlags) {
> - ErrorF("[xkb] XkbDDXCompileKeymapByNames compiling keymap:\n");
> - XkbWriteXKBKeymapForNames(stderr,names,xkb,want,need);
> + /* write XKBKeyMapBuf to xkbcomp */
> + if (EOF==fputs(xkbKeyMapBuf, out))
> + {
> + ErrorF("[xkb] Sending keymap to xkbcomp failed\n");
> + return False;
> }
> -#endif
> - XkbWriteXKBKeymapForNames(out,names,xkb,want,need);
> #ifndef WIN32
> if (Pclose(out)==0)
> #else
> @@ -265,16 +349,12 @@ XkbDDXCompileKeymapByNames( XkbDescPtr xkb,
> {
> if (xkbDebugFlags)
> DebugF("[xkb] xkb executes: %s\n",buf);
> - if (nameRtrn) {
> - strncpy(nameRtrn,keymap,nameRtrnLen);
> - nameRtrn[nameRtrnLen-1]= '\0';
> - }
> if (buf != NULL)
> xfree (buf);
> return True;
> }
> else
> - LogMessage(X_ERROR, "Error compiling keymap (%s)\n", keymap);
> + LogMessage(X_ERROR, "Error compiling keymap (%s)\n", xkbfile);
> #ifdef WIN32
> /* remove the temporary file */
> unlink(tmpname);
> @@ -375,7 +455,6 @@ unsigned missing;
> DebugF("Loaded XKB keymap %s, defined=0x%x\n",fileName,(*xkbRtrn)->defined);
> }
> fclose(file);
> - (void) unlink (fileName);
> return (need|want)&(~missing);
> }
>
--
-Alan Coopersmith- alan.coopersmith at sun.com
Sun Microsystems, Inc. - X Window System Engineering
More information about the xorg
mailing list