PATCH: improve XIM initialization performance
Lubos Lunak <[email protected]> Wed, 18 Dec 2002 16:50:23 +0100
| Newsgroups | gmane.comp.xfree86.expert |
|---|---|
| Message-ID | <[email protected]> |
Hello, I hope this is the right place for this. Please review, comment and possibly apply the attached patches. With XFree4.2.x, XIM initialization takes about 66% of QApplication object initialization. This is mainly because of awful pthread mutexes performance (to express it decently), and repeated processing of the same data. The patches are done against XFree-4.2.x sources, but the affected files don't show any big changes in webcvs. All patches are for xc/lib/X11/ . The easy ones first: imLcPrs.patch - this one replaces getc() with getc_unlocked() - with LinuxThreads linked in (all KDE apps) getting rid of some locking always makes a noticeable difference :-/ . I don't think libX11 needs anywhere locked access to files, but this one is opened in a single thread in _XimCreateDefaultTree() anyway. Manpage says getc_unlocked() is POSIX.1, so I hope there's no problem with this one. lcFile.c.patch2 - the same, but this time it's gets() with gets_unlocked() - the manpages says that glibc includes it, but it's not standardized. Would it be possible to include this one together with some configure check (probably something like #ifdef __GLIBC__ )? I think LinuxThreads comes usually only with glibc. lcFile.c.patch - during QApplication construction, _XlcLocaleDirName() is called 5 times, everytime doing exactly the same, so the patch add the obvious caching imLcIm.c.patch - this one is definitely not meant to be applied, it's more a proof of concept patch. The idea is similar to the lcFile.c.patch, this time it's more complicated. Every time an application starts, it loads the same Compose file and start analyzing it, generating exactly the same structure. So the patch basically takes this generated structure, packs it into one piece of memory, and stores it to a file. Next time instead of generating the structure again, it's simply mapped into memory and all pointers are corrected if the address is different. Having the whole structure cached also helps with the fact that its generation is actually done twice for some reason, once when calling XOpenIM(), once when calling XRegisterIMInstantiateCallback(). Could something like that get into the XFree cvs, and what would I have to change? I have no idea how you handle portability when it comes to stuff like mmap(), and some of you probably won't like things like fixing the pointers in the structure or mapping into memory something possibly created by a different user (the whole things almost looks like an ugly hack, doesn't it ? ;) ). I'll change this one (and the others, if needed) to meet your requirements. Oh, BTW, maybe some numbers. With unmodified XFree-4.2.x, XIM stuff takes about 66% out of 100% QApplication initialization time, as I've already said. The change in XlcLocaleDirName (lcFile.c.patch) reduces these 100% to 90%. Changing getc() to getc_unlocked() (imLcPrs.c.patch) additionally reduces this to 75%. Saving the processed Compose file (imLcIm.c.patch) makes it 60%, and finally adding also the gets_unlocked() changes makes it about 55%. This is 1GHz Athlon, so the numbers aren't that accurate, but with profiling with valgrind makes me estimate the time approximately halved (= complete QApplication initialization time). Now XIM initialization takes only about 18% from QApplication initialization, which looks much better (congratulations :) ). -- Lubos Lunak KDE developer --------------------------------------------------------------------- SuSE CR, s.r.o. e-mail: [email protected] , [email protected] Drahobejlova 27 tel: +420 2 9654 2373 190 00 Praha 9 fax: +420 2 9654 2374 Czech Republic http://www.suse.cz/
imLcPrs.c.patch
(text/x-diff, 408 B)
--- imLcPrs.c.sav 2002-12-18 14:52:09.000000000 +0100
+++ imLcPrs.c 2002-12-18 14:53:57.000000000 +0100
@@ -90,11 +90,11 @@ nextch(fp, lastch)
c = *lastch;
*lastch = 0;
} else {
- c = getc(fp);
+ c = getc_unlocked(fp);
if (c == '\\') {
- c = getc(fp);
+ c = getc_unlocked(fp);
if (c == '\n') {
- c = getc(fp);
+ c = getc_unlocked(fp);
} else {
ungetc(c, fp);
c = '\\';
lcFile.c.patch
(text/x-diff, 1.4 KB)
--- lcFile.c.sav 2002-12-18 14:52:09.000000000 +0100
+++ lcFile.c 2002-12-18 15:18:33.000000000 +0100
@@ -439,6 +439,17 @@ _XlcLocaleDirName(dir_name, lc_name)
static char locale_alias[] = LOCALE_ALIAS;
char *target_name = (char*)0;
char *target_dir = (char*)0;
+
+ static char* lc_name_last = 0;
+ static char* dir_name_last = 0;
+ int lc_name_size;
+ int dir_name_size;
+
+
+ if( lc_name_last != 0 && strcmp( lc_name_last, lc_name ) == 0 ) {
+ strcpy( dir_name, dir_name_last );
+ return dir_name;
+ }
xlocaledir (dir, PATH_MAX);
n = _XlcParsePath(dir, args, 256);
@@ -491,5 +502,20 @@ _XlcLocaleDirName(dir_name, lc_name)
strcat(dir_name, target_name);
if (target_name != lc_name)
Xfree(target_name);
+
+ /* cache for repeated calls */
+ lc_name_size = strlen( lc_name );
+ if( lc_name_last != 0 && strlen( lc_name_last ) < lc_name_size )
+ Xfree( lc_name_last );
+ if( lc_name_last == 0 )
+ lc_name_last = Xmalloc( lc_name_size + 1 );
+ dir_name_size = strlen( dir_name );
+ if( dir_name_last != 0 && strlen( dir_name_last ) < dir_name_size )
+ Xfree( dir_name_last );
+ if( dir_name_last == 0 )
+ dir_name_last = Xmalloc( dir_name_size + 1 );
+ strcpy( lc_name_last, lc_name );
+ strcpy( dir_name_last, dir_name );
+
return dir_name;
}
lcFile.c.patch2
(text/x-diff, 341 B)
--- lcFile.c.sav2 2002-12-18 16:08:05.000000000 +0100
+++ lcFile.c 2002-12-18 16:08:17.000000000 +0100
@@ -227,7 +227,7 @@ resolve_name(
if (fp == NULL)
return NULL;
- while (fgets(buf, XLC_BUFSIZE, fp) != NULL) {
+ while (fgets_unlocked(buf, XLC_BUFSIZE, fp) != NULL) {
char *p = buf;
int n;
char *args[2], *from, *to;
imLcIm.c.patch
(text/x-diff, 8.7 KB)
--- imLcIm.c.sav 2002-12-18 15:31:52.000000000 +0100
+++ imLcIm.c 2002-12-18 15:38:51.000000000 +0100
@@ -73,11 +73,30 @@ _XimCheckIfLocalProcessing(im)
return(False);
}
+struct _XimCachedDefaultTreeStruct
+ {
+ int id;
+ off_t size;
+ int count;
+ char* 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(top)
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,22 +227,283 @@ _XimLocalSetIMValues(xim, values)
return(name);
}
+#include <sys/mman.h>
+/* TODO non-mmap() variant */
+
+Private Bool
+_XimReadCachedDefaultTree(
+ FILE** fp_cache,
+ off_t size)
+{
+ struct _XimCachedDefaultTreeStruct* m;
+ int addr_diff;
+ DefTree* pos;
+ int i;
+
+ m = mmap( NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fileno(*fp_cache), 0 );
+ if( m == NULL )
+ return False;
+ if( m->id != 0x3746 || m->size != size || m->count < 0 ) {
+ munmap( m, size );
+ }
+ _XimCachedDefaultTree_mmap = m;
+ _XimCachedDefaultTree = m->defs;
+ _XimCachedDefaultTreeRefcount = 0;
+ _XimCachedDefaultTreeFile = *fp_cache;
+ *fp_cache = NULL;
+ addr_diff = (char*)m - m->addr;
+ pos = m->defs;
+ for( i = 0;
+ i < m->count;
+ ++i, ++pos ) {
+#define FIXUP( el, type ) do { if( el != NULL ) el = (type*)((char*)el+addr_diff); } while( 0 )
+ FIXUP( pos->next, DefTree );
+ FIXUP( pos->succession, DefTree );
+ FIXUP( pos->mb, char );
+ FIXUP( pos->wc, wchar_t );
+ FIXUP( pos->utf8, char );
+ }
+ return True;
+}
+
+#define CACHE_FILE_DIR "/var/X11R6/cache"
+/*#define CACHE_FILE_DIR "/home/seli/zz/xim/wrk"*/
+
+
+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 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 + 1 ) * sizeof( wchar_t )
+ + chars_len;
+ data = _XimCachedDefaultTree_mmap = Xmalloc( size );
+ 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;
+
+#if 0
+ print_tr(im->private.local.top,strlen(aaa_off));
+#endif
+
+/* TODO mozna tady dalsi mutex zamykani? */
+ if( !_XimConvertDefaultTreeToCache(im))
+ return;
+ fp_cache = _XFopenFile( name, "w" );
+ if( fp_cache == NULL )
+ return;
+ fwrite( _XimCachedDefaultTree_mmap,
+ _XimCachedDefaultTree_mmap->size, 1, fp_cache );
+ fclose( fp_cache );
+}
+
Private void
_XimCreateDefaultTree(im)
Xim im;
{
FILE *fp;
char *name;
+ FILE* fp_cache;
+ char *cache_name;
name = _XlcFileName(im->core.lcd, COMPOSE_FILE);
if (name == (char *)NULL)
return;
fp = _XFopenFile (name, "r");
- Xfree(name);
- if (fp == (FILE *)NULL)
+ if (fp == (FILE *)NULL) {
+ Xfree(name);
return;
+ }
+ /* convert name to cached filename */
+ cache_name = _XimCachedDefaultTreeCacheFile( name );
+ Xfree(name);
+ 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
+ /* TODO test spravneho jmena locale souboru
+ pokud je jmeno spatne, a refcount == 0, jde nahradit */
+ || _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->private.local.top);
fclose(fp);
+ if( cache_name != NULL ) {
+ if( _XimCachedDefaultTree == NULL )
+ _XimWriteCachedDefaultTree(cache_name,im);
+ Xfree( cache_name );
+ }
}
Private XIMMethodsRec Xim_im_local_methods = {