[Xorg] [PATCH] Cached XIM data

Alan Coopersmith Alan.Coopersmith at Sun.COM
Mon Aug 9 11:24:20 PDT 2004


I don't know much about XIM, but the use of MAP_FIXED makes me uncomfortable
and the references to storing pointers in the data makes it sound like on
platforms which support multiple pointer size models (i.e. 32-bit and 64-bit
pointers), you'ld need different cache files for the different pointer
sizes, which I didn't see any provision for.

(I do remember sendmail doing something similar many years ago with frozen
  config files, which got dropped a while ago as a bad idea, though I don't
  remember why.)


	-Alan Coopersmith-           alan.coopersmith at sun.com
	 Sun Microsystems, Inc. - X Window System Engineering


Lubos Lunak wrote:
>  Hello,
> 
>  could somebody please review the attached XIM patches?
> 
>  First, the rationale: QApplication constructor (app class in Qt) takes about 
> 0.08s on my computer (850Mhz). But only in case I dump my Compose file and 
> all fonts. With the Compose file it's 0.31s  (and with fonts it goes up to 
> 0.47s, but that's fontconfig, so let's ignore that one). There's also about 
> 500KiB (unshared) memory difference. Multiply this quarter a second and half 
> a meg e.g. by the number of apps with X connection in normal KDE session to 
> get even worse numbers. All this just for the ability to compose accented 
> characters.
> 
>  The problem is basically that XIM parses the matching Compose file 
> from /usr/X11R6/lib/X11/locale (which happens to be the huge 
> en_US.UTF-8 one for UTF8 locales) and builds internal representation of this 
> data. The file is more or less constant, but XIM parses it once per 
> application startup.
> 
>  The imLcIm.c patch tries to solve this problem in similar way KDE's ksycoca 
> stores data. After XIM parses the Compose file, the code computes how much 
> memory the internal representation needs, allocates such block of memory, and 
> copies the internal representation there, including adjusting the pointers. 
> This avoids malloc() overhead, and allows read-only mmap()-ing of the block 
> (the internal representation doesn't appear to be ever modified after it's 
> created). Next time it's attempted to be mmap()-ed at the proper address, and 
> if that succeeds, bingo, QApplication time is again at 0.08s. Still without 
> the fonts of course, fontconfig needs something similar.
> 
>  The second patch caches a repeatedly requested information that is parsed 
> from another file.
> 
>  There are several things I'm unsure about the patch:
> 
> - It uses things like mmap() or posix_memalign() which could cause trouble to 
> all those crappy Nulix platforms out there. It should be just a matter of few 
> #ifdef's, but I don't know how you do checks for such features.
> 
> - I'm not sure where to store the cache file. KDE has special place for cache 
> files, but I don't know how to do that with non-KDE apps. The patch now 
> stores it in /tmp/xim, the /var/X11R6/cache/ or /var/cache are Linux-ism 
> AFAIK. Moreover this should be probably saved per-user for paranoia reasons, 
> or shouldn't it?
> 
>  Could somebody helps me with these issues? Comments?
> 
>  Thanks
> 
> PS: The patches are for libX11.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- lcFile.c.sav	2004-07-01 22:29:50.000000000 +0200
> +++ lcFile.c	2004-08-08 21:46:05.000000000 +0200
> @@ -491,7 +491,13 @@ _XlcLocaleDirName(dir_name, dir_len, lc_
>      static char locale_alias[] = LOCALE_ALIAS;
>      char *target_name = (char*)0;
>      char *target_dir = (char*)0;
> +    static char* last_dir_name = 0;
> +    static char* last_lc_name = 0;
>  
> +    if( last_lc_name != 0 && strcmp( last_lc_name, lc_name ) == 0 ) {
> +        strcpy( dir_name, last_dir_name );
> +        return dir_name;
> +    }
>      xlocaledir (dir, PATH_MAX);
>      n = _XlcParsePath(dir, args, 256);
>      for (i = 0; i < n; ++i) {
> @@ -550,5 +556,13 @@ _XlcLocaleDirName(dir_name, dir_len, lc_
>      }
>      if (target_name != lc_name)
>   	Xfree(target_name);
> +    if( last_dir_name != 0 )
> +        Xfree( last_dir_name );
> +    if( last_lc_name != 0 )
> +        Xfree( last_lc_name );
> +    last_dir_name = Xmalloc( strlen( dir_name ) + 1 );
> +    strcpy( last_dir_name, dir_name );
> +    last_lc_name = Xmalloc( strlen( lc_name ) + 1 );
> +    strcpy( last_lc_name, lc_name );
>      return dir_name;
>  }
> 
> 
> ------------------------------------------------------------------------
> 
> --- imLcIm.c.sav	2003-09-06 16:06:32.000000000 +0200
> +++ imLcIm.c	2004-08-09 19:04:13.191772440 +0200
> @@ -34,7 +34,21 @@ THIS SOFTWARE.
>  ******************************************************************/
>  /* $XFree86: xc/lib/X11/imLcIm.c,v 1.11 2003/04/13 19:22:21 dawes Exp $ */
>  
> +/* for posix_memalign */
> +#ifndef _XOPEN_SOURCE
> +#define _XOPEN_SOURCE 600
> +#elif _XOPEN_SOURCE < 600
> +#undef _XOPEN_SOURCE
> +#define _XOPEN_SOURCE 600
> +#endif
> +
>  #include <stdio.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +#include <stdlib.h>
> +
>  /*
>  #include <X11/Xlib.h>
>  */
> @@ -73,11 +87,30 @@ _XimCheckIfLocalProcessing(im)
>      return(False);
>  }
>  
> +struct _XimCachedDefaultTreeStruct
> +    {
> +    int id;
> +    off_t size;
> +    int count;
> +    void* addr;
> +    DefTree defs[ 1 ];
> +    };
> +
> +Private struct _XimCachedDefaultTreeStruct* _XimCachedDefaultTree_mmap = NULL;
> +Private FILE* _XimCachedDefaultTreeFile = NULL;
> +Private DefTree* _XimCachedDefaultTree = NULL;
> +Private int _XimCachedDefaultTreeRefcount = 0;
> +
>  Private void
>  XimFreeDefaultTree(
>      DefTree *top)
>  {
>      if (!top) return;
> +    if( top == _XimCachedDefaultTree ) {
> +        --_XimCachedDefaultTreeRefcount;
> +        /* No deleting, it's a cache after all. */
> +        return;
> +    }
>      if (top->succession) XimFreeDefaultTree(top->succession);
>      if (top->next) XimFreeDefaultTree(top->next);
>      if (top->mb) Xfree(top->mb);
> @@ -208,12 +241,242 @@ _XimLocalSetIMValues(
>      return(name);
>  }
>  
> +Private Bool
> +_XimReadCachedDefaultTree(
> +    FILE** fp_cache,
> +    off_t size)
> +{
> +    struct _XimCachedDefaultTreeStruct* m;
> +    m = mmap( NULL, size, PROT_READ, MAP_PRIVATE, fileno(*fp_cache), 0 );
> +    if( m == NULL || m == MAP_FAILED )
> +        return False;
> +    if( m->id != 0x3746 || m->size != size || m->count < 0 ) {
> +        munmap( m, size );
> +        return False;
> +    }
> +    /* map it to the requested address (so that pointers match), fail otherwise */
> +    if( m != m->addr ) {
> +        void* use_addr = m->addr;
> +        munmap( m, size );
> +        m = mmap( use_addr, size, PROT_READ, MAP_PRIVATE | MAP_FIXED, fileno(*fp_cache), 0 );
> +        if( m == NULL || m == MAP_FAILED )
> +            return False;
> +        if( m->id != 0x3746 || m->size != size || m->count < 0 || m != m->addr ) {
> +            munmap( m, size );
> +            return False;
> +        }
> +    }
> +    _XimCachedDefaultTree_mmap = m;
> +    _XimCachedDefaultTree = m->defs;
> +    _XimCachedDefaultTreeRefcount = 0;
> +    _XimCachedDefaultTreeFile = *fp_cache;
> +    *fp_cache = NULL;
> +    return True;
> +}
> +
> +/*#define CACHE_FILE_DIR "/var/X11R6/cache"*/
> +#define CACHE_FILE_DIR "/tmp/xim"
> +
> +Private char* _XimCachedDefaultTreeCacheFile( const char* name )
> +    {
> +    char* pos;
> +    char* ret = Xmalloc( strlen( name ) + strlen( CACHE_FILE_DIR ) + 1 );
> +    if( ret == NULL )
> +        return NULL;
> +    strcpy( ret, CACHE_FILE_DIR );
> +    pos = ret + strlen( ret ) + 1; /* 1 = not the first slash */
> +    strcat( ret, name );
> +    while( *pos )
> +        {
> +        if( *pos == '/' )
> +            *pos = '_';
> +        ++pos;
> +        }
> +    return ret;
> +    }
> +    
> +
> +Private int _XimCountItems(
> +DefTree* top,
> +int* chars_len,
> +int* wchars_len)
> +{
> +    int items = 0;
> +    DefTree* pos1;
> +    DefTree* pos2;
> +
> +    if( top->mb != NULL )
> +        *chars_len += strlen( top->mb ) + 1;
> +    if( top->wc != NULL )
> +        *wchars_len += _Xwcslen( top->wc ) + 1;
> +    if( top->utf8 != NULL )
> +        *chars_len += strlen( top->utf8 ) + 1;
> +    for( pos1 = top;
> +         pos1 != NULL;
> +         pos1 = pos1->next )
> +        {
> +        ++items;
> +        for( pos2 = pos1->succession;
> +             pos2 != NULL;
> +             pos2 = pos2->succession )
> +            items += _XimCountItems( pos2, chars_len, wchars_len );
> +        }
> +    return items;
> +}
> +
> +Private DefTree*
> +_XimAddCacheItem(
> +DefTree* top,
> +struct _XimCachedDefaultTreeStruct* data,
> +int* item_pos,
> +char* chars,
> +wchar_t* wchars)
> +{
> +    int my_item = (*item_pos)++;
> +
> +    data->defs[ my_item ] = *top;
> +    if( top->mb != NULL )
> +        {
> +        data->defs[ my_item ].mb = chars;
> +        strcpy( chars, top->mb );
> +        chars += strlen( top->mb ) + 1;
> +        }
> +    if( top->wc != NULL )
> +        {
> +        data->defs[ my_item ].wc = wchars;
> +        _Xwcscpy( wchars, top->wc );
> +        wchars += _Xwcslen( top->wc ) + 1;
> +        }
> +    if( top->utf8 != NULL )
> +        {
> +        data->defs[ my_item ].utf8 = chars;
> +        strcpy( chars, top->utf8 );
> +        chars += strlen( top->utf8 ) + 1;
> +        }
> +    if( top->next != NULL )
> +        data->defs[ my_item ].next =
> +            _XimAddCacheItem( top->next, data, item_pos, chars, wchars );
> +    if( top->succession != NULL )
> +        data->defs[ my_item ].succession =
> +            _XimAddCacheItem( top->succession, data, item_pos, chars, wchars );
> +    return data->defs + my_item;
> +}
> +
> +
> +Private Bool
> +_XimConvertDefaultTreeToCache(
> +    Xim		im)
> +{
> +    struct _XimCachedDefaultTreeStruct* data;
> +    int chars_len = 0;
> +    int wchars_len = 0;
> +    char* chars;
> +    wchar_t* wchars;
> +    char* wchars_tmp;
> +    int item_pos = 0;
> +    int size;
> +    int pagesize;
> +    void* mem;
> +    
> +    int items = _XimCountItems(im->private.local.top,
> +        &chars_len, &wchars_len );
> +    if( items == 0 )
> +        return False;
> +    size = sizeof( struct _XimCachedDefaultTreeStruct )
> +        + ( items - 1 ) * sizeof ( DefTree )
> +        + wchars_len * sizeof( wchar_t )
> +        + chars_len * sizeof( char );
> +    pagesize = sysconf( _SC_PAGE_SIZE );
> +    if( pagesize < 0 )
> +        pagesize = 4096;
> +    if( posix_memalign( &mem, pagesize, size ) != 0 )
> +        return False;
> +    data = _XimCachedDefaultTree_mmap = mem;
> +    if( data == NULL )
> +        return False;
> +    data->size = size;
> +    data->count = items;
> +    data->id = 0x3746;
> +    data->addr = (char*)data;
> +    wchars_tmp = ( char* )data
> +        + sizeof( struct _XimCachedDefaultTreeStruct )
> +        + ( items - 1 ) * sizeof ( DefTree );
> +    wchars = (wchar_t*)data;
> +    while( (char*)wchars < wchars_tmp )
> +        ++wchars;
> +    chars = (char*)(wchars+wchars_len);
> +    _XimAddCacheItem( im->private.local.top, data, &item_pos, chars, wchars );
> +    return True;
> +}
> +
> +#if 0
> +char aaa_off[] = "                                                                                    ";
> +Private void print_tr(
> +DefTree* top,
> +int dif)
> +{
> +    DefTree* pos1;
> +    DefTree* pos2;
> +
> +    fprintf( stderr, aaa_off + dif );
> +    fprintf( stderr, "+ %p\n", top );
> +    --dif;
> +    for( pos1 = top;
> +         pos1 != NULL;
> +         pos1 = pos1->next )
> +        {
> +        fprintf( stderr, aaa_off + dif );
> +        fprintf( stderr, "! %p\n", pos1 );
> +        for( pos2 = pos1->succession;
> +             pos2 != NULL;
> +             pos2 = pos2->succession )
> +            {
> +            fprintf( stderr, aaa_off + dif );
> +            fprintf( stderr, "* %p\n", pos2 );
> +            print_tr(pos2,dif);
> +            }
> +        }
> +    ++dif;
> +    fprintf( stderr, aaa_off + dif );
> +    fprintf( stderr, "- %p\n", top );        
> +}
> +#endif
> +
> +Private void
> +_XimWriteCachedDefaultTree(
> +    const char* name,
> +    Xim		im)
> +{
> +    FILE* fp_cache;
> +    char* tmp_name;
> +
> +#if 0
> +    print_tr(im->private.local.top,strlen(aaa_off));
> +#endif
> +
> +    if( !_XimConvertDefaultTreeToCache(im))
> +        return;
> +    tmp_name = Xmalloc( strlen( name ) + 20 );
> +    sprintf( tmp_name, "%s_%i", name, getpid());
> +    fp_cache = _XFopenFile( tmp_name, "w" );
> +    if( fp_cache != NULL ) {
> +        fwrite( _XimCachedDefaultTree_mmap,
> +            _XimCachedDefaultTree_mmap->size, 1, fp_cache );
> +        if( fclose( fp_cache ) == 0 )
> +            rename( tmp_name, name );
> +    }
> +    unlink( tmp_name );
> +    XFree( tmp_name );
> +}
> +
>  Private void
>  _XimCreateDefaultTree(
>      Xim		im)
>  {
>      FILE *fp = NULL;
>      char *name, *tmpname = NULL;
> +    FILE* fp_cache;
> +    char *cache_name;
>  
>      name = getenv("XCOMPOSEFILE");
>  
> @@ -243,13 +506,49 @@ _XimCreateDefaultTree(
>      if (fp == (FILE *) NULL) {
>          fp = _XFopenFile (name, "r");
>      }
> +    if (fp == (FILE *) NULL) {
> +        if (tmpname != (char *) NULL) {
> +            Xfree(tmpname);
> +        }
> +	return;
> +    }
> +    /* convert name to cached filename */
> +    cache_name = _XimCachedDefaultTreeCacheFile( name );
>      if (tmpname != (char *) NULL) {
>          Xfree(tmpname);
>      }
> -    if (fp == (FILE *) NULL)
> -	return;
> +    if( cache_name != NULL ) {
> +        fp_cache = _XFopenFile( cache_name, "r" );
> +        if( fp_cache != (FILE*)NULL ) {
> +            struct stat st, st_cache;
> +            if (fstat (fileno (fp), &st) != -1
> +                && fstat( fileno( fp_cache ), &st_cache ) != -1
> +                && st.st_mtime < st_cache.st_mtime
> +                && st_cache.st_mtime > time( NULL ) - 60 * 60 * 24 ) {
> +                if( _XimCachedDefaultTree != NULL
> +                    || _XimReadCachedDefaultTree(&fp_cache, st_cache.st_size)) {
> +                    im->private.local.top = _XimCachedDefaultTree;
> +#if 0
> +                    print_tr(im->private.local.top,strlen(aaa_off));
> +#endif
> +                    ++_XimCachedDefaultTreeRefcount;
> +                    fclose(fp);
> +                    if( fp_cache != NULL )
> +                        fclose(fp_cache);
> +                    Xfree(cache_name);
> +                    return;
> +                }
> +            }
> +            fclose(fp_cache);    
> +        }
> +    }
>      _XimParseStringFile(fp, im);
>      fclose(fp);
> +    if( cache_name != NULL ) {
> +        if( _XimCachedDefaultTree == NULL )
> +            _XimWriteCachedDefaultTree(cache_name,im);
> +        Xfree( cache_name );
> +    }
>  }
>  
>  Private XIMMethodsRec      Xim_im_local_methods = {
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> xorg mailing list
> xorg at freedesktop.org
> http://freedesktop.org/mailman/listinfo/xorg





More information about the xorg mailing list