[GNOME VFS] mime parse speedup, bug fix & crack removal ...
Michael Meeks <[email protected]>
| Newsgroups | gmane.comp.gnome.vfs |
|---|---|
| Message-ID | <1018374581.6622.8.camel@pc2-newm2-0-cust131> |
Hi there, After talking to Seth it seems random untested commits are appreciated here ;-) anyway, the attached patch (committed) does: a) shrinks mime parse from 120ms -> 45ms b) fixes the following issues: diff -u /tmp/mime.old /tmp/mime.new --- /tmp/mime.old Tue Apr 9 17:30:34 2002 +++ /tmp/mime.new Tue Apr 9 18:29:01 2002 @@ -1199,17 +1199,17 @@ Add key: 'icon_filename' '(null)' 'i-sock' 0 New context: 'x-special/symlink' Add key: 'description' '(null)' 'symbolic link' 0 -New context: 'application/x-core-file icon-filename=/opt/ngnome2/share/pixmaps/mc/i-core.png' -Add key: 'open' '(null)' 'bug-buddy --core=%f' 0 -New context: 'x-device/floppy open.format.Format floppy=gfloppy %f' +Add key: 'icon-filename' '(null)' '/opt/ngnome2/share/pixmaps/mc/i-core.png' 0 +New context: 'x-device/floppy' +Add key: 'open.format.Format floppy' '(null)' 'gfloppy %f' 0 Add key: 'default_action_type' '(null)' 'application' 0 Add key: 'short_list_application_ids_for_novice_user_level' '(null)' 'gnome-exe-handler' 0 Add key: 'short_list_application_ids_for_intermediate_user_level' '(null)' 'gnome-exe-handler' 0 Add key: 'short_list_application_ids_for_advanced_user_level' '(null)' 'gnome-exe-handler' 0 -New context: 'application/x-gnumeric open=gnumeric %f' +Add key: 'open' '(null)' 'gnumeric %f' 0 Add key: 'view' '(null)' 'gnumeric %f' 0 Add key: 'icon-filename' '(null)' '/gnome-application-x-gnumeric.png' 0 -New context: 'application/vnd.ms-excel open=gnumeric %f' +Add key: 'open' '(null)' 'gnumeric %f' 0 Add key: 'view' '(null)' 'gnumeric %f' 0 Add key: 'icon-filename' '(null)' '/gnome-application-x-xls.png' 0 New context: 'application/x-metatheme' @@ -1379,7 +1379,7 @@ Add key: 'ext' '(null)' 'tar' 0 New context: 'application/x-theme' Add key: 'ext' '(null)' 'theme' 0 -New context: '# FIXME bugzilla.eazel.com 5372application/x-troff-man-compressed' +New context: 'application/x-troff-man-compressed' Add key: 'regex' '(null)' '([^0-9]|^[^\.]*)\.([1-9][a-z]?|n)\.g?[Zz]$' 0 New context: 'application/x-unix-archive' Add key: 'ext' '(null)' 'a' 0 @@ -1499,7 +1499,8 @@ New context: 'model/vrml' Add key: 'ext' '(null)' 'wrl' 0 New context: 'text/abiword' -Add key: 'text/bib ext' '(null)' 'bib' 0 +New context: 'text/bib' +Add key: 'ext' '(null)' 'bib' 0 New context: 'text/calendar' Add key: 'ext' '(null)' 'ics' 0 New context: 'text/css' @@ -1578,7 +1579,7 @@ Add key: 'ext' '(null)' 'texi texinfo' 0 New context: 'text/x-troff' Add key: 'ext' '(null)' 'roff t tr' 0 -New context: '# FIXME bugzilla.eazel.com 5372text/x-troff-man' +New context: 'text/x-troff-man' Add key: 'regex' '(null)' '(([^0-9]|^[^\.]*)\.([1-9][A-Za-z]*|n)|\.man)$' 0 Add key: 'ext' '(null)' 'man' 0 New context: 'text/x-troff-me' c) Removes a big chunk of cut and paste coding, that was pretty un-maintainable. HTH, Michael. Index: ChangeLog =================================================================== RCS file: /cvs/gnome/gnome-vfs/ChangeLog,v retrieving revision 1.1253 diff -u -p -u -r1.1253 ChangeLog --- ChangeLog 1 Apr 2002 02:01:14 -0000 1.1253 +++ ChangeLog 9 Apr 2002 17:48:53 -0000 @@ -1,3 +1,28 @@ +2002-04-07 Michael Meeks <[email protected]> + + * test/test-performance.c (main): upd. + + * libgnomevfs/gnome-vfs-mime-info.c + (context_new): use g_hash_table_new_full. + (context_add_key): make the logic more sane, + don't do stupid things keeping 'previous_key' + around, since it's always the same (current) key. + (load_mime_list_info_from, load_mime_type_info_from): + expunge previous_key inefficiency. + (load_mime_type_info_from): don't keep dupping stuff + we don't need to, instead build the several strings + we need in the GString. + (load_mime_list_info_from): ditto. + (release_key_and_value): kill. + (context_destroy): upd. + use g_string_insert_c instead of append_char. + (load_mime_type_info_from): rename to + (load_type_info_from): and unify from giant ugly cut and paste + (load_mime_list_info_from): kill junk. + use defines to map to old names. + (load_type_info_from): ignore lines that don't have the + right lang before doing much slower parsing on them. + 2002-03-30 Jody Goldberg <[email protected]> http://bugzilla.gnome.org/show_bug.cgi?id=71391 Index: libgnomevfs/gnome-vfs-mime-info.c =================================================================== RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-mime-info.c,v retrieving revision 1.58 diff -u -p -u -r1.58 gnome-vfs-mime-info.c --- libgnomevfs/gnome-vfs-mime-info.c 20 Feb 2002 09:25:36 -0000 1.58 +++ libgnomevfs/gnome-vfs-mime-info.c 9 Apr 2002 17:48:54 -0000 @@ -133,12 +133,8 @@ static int should_write_file_back = 0; static GList *current_lang = NULL; /* we want to replace the previous key if the current key has a higher language level */ -static char *previous_key = NULL; static int previous_key_lang_level = -1; - - - /* * A hash table containing all of the Mime records for specific * mime types (full description, like image/png) @@ -168,7 +164,6 @@ static GnomeVFSResult write_back_keys_us static const char * gnome_vfs_mime_get_registered_mime_type_key (const char *mime_type, const char *key); - static gboolean does_string_contain_caps (const char *string) { @@ -185,8 +180,6 @@ does_string_contain_caps (const char *st return FALSE; } - - static GnomeMimeContext * context_new (GHashTable *hash_table, GString *str) { @@ -207,31 +200,24 @@ context_new (GHashTable *hash_table, GSt g_free (mime_type); return context; } + +/* fprintf (stderr, "New context: '%s'\n", mime_type); */ context = g_new (GnomeMimeContext, 1); context->mime_type = mime_type; - context->keys = g_hash_table_new (g_str_hash, g_str_equal); + context->keys = g_hash_table_new_full ( + g_str_hash, g_str_equal, + (GDestroyNotify) g_free, + (GDestroyNotify) g_free); g_hash_table_insert (hash_table, context->mime_type, context); - return context; -} - -static gboolean -release_key_and_value (gpointer key, gpointer value, gpointer user_data) -{ - g_free (key); - g_free (value); - return TRUE; + return context; } static void context_destroy (GnomeMimeContext *context) { - /* - * Destroy it - */ - g_hash_table_foreach_remove (context->keys, release_key_and_value, NULL); g_hash_table_destroy (context->keys); g_free (context->mime_type); g_free (context); @@ -276,53 +262,23 @@ language_level (const char *langage) static void context_add_key (GnomeMimeContext *context, char *key, char *lang, char *value) { - char *v; - char *orig_key; int lang_level; - lang_level = language_level(lang); + lang_level = language_level (lang); /* wrong language completely */ if (lang_level < 0) return; -/* fprintf (stderr, "Add key: '%s' '%s' '%s'\n", key, lang, value); */ - - /* if we have some language defined and - if there was a previous_key */ - if (lang_level > 0 && previous_key) { - /* if our new key has a better lang_level then remove the - previous key */ - if (previous_key_lang_level <= lang_level) { - if (g_hash_table_lookup_extended (context->keys, - previous_key, - (gpointer *)&orig_key, - (gpointer *)&v)) { - g_hash_table_remove (context->keys, orig_key); - g_free(orig_key); - g_free(v); - } - /* else, our language level really sucks and the previous - translation was of better language quality so just - ignore us */ - } else { - return; - } + /* if a previous key in the hash had a better lang_level don't do anything */ + if (lang_level > 0 && + previous_key_lang_level > lang_level) { + return; } - if (g_hash_table_lookup_extended (context->keys, key, - (gpointer *)&orig_key, - (gpointer *)&v)) { - /* if we found it in the database already, just replace it here */ - g_free (v); - g_hash_table_insert (context->keys, orig_key, - g_strdup (value)); - } else { - g_hash_table_insert (context->keys, g_strdup(key), - g_strdup (value)); - } - /* set this as the previous key */ - g_free(previous_key); - previous_key = g_strdup(key); +/* fprintf (stderr, "Add key: '%s' '%s' '%s' %d\n", key, lang, value, lang_level); */ + + g_hash_table_replace (context->keys, g_strdup (key), g_strdup (value)); + previous_key_lang_level = lang_level; } @@ -335,290 +291,120 @@ typedef enum { STATE_ON_VALUE } ParserState; +#define APPEND_CHAR(gstr,c) g_string_insert_c ((gstr), -1, (c)) + +typedef enum { + FORMAT_MIME, + FORMAT_KEYS +} Format; + +#define load_mime_type_info_from(a,b) load_type_info_from ((a), (b), FORMAT_MIME) +#define load_mime_list_info_from(a,b) load_type_info_from ((a), (b), FORMAT_KEYS) + static void -load_mime_type_info_from (const char *filename, GHashTable *hash_table) +load_type_info_from (const char *filename, + GHashTable *hash_table, + Format format) { - FastFile mime_file; - gboolean in_comment, context_used; GString *line; int column, c; ParserState state; + FastFile mime_file; + gboolean skip_line; GnomeMimeContext *context; - char *key; - char *lang; + int key, lang, last_str_end; /* offsets */ if (!fast_file_open (&mime_file, filename)) { return; } - in_comment = FALSE; - context_used = FALSE; + skip_line = FALSE; column = -1; context = NULL; - key = NULL; - lang = NULL; line = g_string_sized_new (120); + key = lang = last_str_end = 0; state = STATE_NONE; while ((c = fast_file_getc (&mime_file)) != EOF) { + handle_char: column++; if (c == '\r') continue; if (c == '#' && column == 0) { - in_comment = TRUE; + skip_line = TRUE; continue; } - if (in_comment) { + if (skip_line) { if (c == '\n') { - in_comment = FALSE; + skip_line = FALSE; column = -1; + g_string_assign (line, ""); + key = lang = last_str_end = 0; } continue; } if (c == '\n') { - in_comment = FALSE; + skip_line = FALSE; column = -1; if (state == STATE_ON_MIME_TYPE) { - - /* set previous key to nothing - for this mime type */ - g_free(previous_key); - previous_key = NULL; + /* setup for a new key */ previous_key_lang_level = -1; - context = context_new (hash_table, line); - context_used = FALSE; - g_string_assign (line, ""); - state = STATE_LOOKING_FOR_KEY; - continue; - } - if (state == STATE_ON_VALUE) { - context_used = TRUE; - context_add_key (context, key, lang, line->str); - g_string_assign (line, ""); - g_free (key); - key = NULL; - g_free (lang); - lang = NULL; - state = STATE_LOOKING_FOR_KEY; - continue; - } + + } else if (state == STATE_ON_VALUE) { + APPEND_CHAR (line, '\0'); + context_add_key (context, + line->str + key, + lang ? line->str + lang : NULL, + line->str + last_str_end); + key = lang = 0; + } + g_string_assign (line, ""); + last_str_end = 0; + state = STATE_LOOKING_FOR_KEY; continue; } switch (state) { case STATE_NONE: - if (c != ' ' && c != '\t') - state = STATE_ON_MIME_TYPE; - else - break; - /* fall down */ - - case STATE_ON_MIME_TYPE: - if (c == ':') { - in_comment = TRUE; + if (c == ' ' || c == '\t') { break; - } - g_string_append_c (line, c); - break; - - case STATE_LOOKING_FOR_KEY: - if (c == '\t' || c == ' ') + } else if (c == ':') { + skip_line = TRUE; break; - - if (c == '[') { - state = STATE_LANG; - break; - } - - if (column == 0) { + } else { state = STATE_ON_MIME_TYPE; - g_string_append_c (line, c); - break; - } - state = STATE_ON_KEY; - /* falldown */ - - case STATE_ON_KEY: - if (c == '\\') { - c = fast_file_getc (&mime_file); - if (c == EOF) - break; - } - if (c == '=') { - key = g_strdup (line->str); - g_string_assign (line, ""); - state = STATE_ON_VALUE; - break; - } - g_string_append_c (line, c); - break; - - case STATE_ON_VALUE: - g_string_append_c (line, c); - break; - - case STATE_LANG: - if (c == ']') { - state = STATE_ON_KEY; - if (line->str [0]) { - g_free(lang); - lang = g_strdup(line->str); - } else { - in_comment = TRUE; - state = STATE_LOOKING_FOR_KEY; - } - g_string_assign (line, ""); - break; - } - g_string_append_c (line, c); - break; - } - } - - if (context != NULL) { - if (key && line->str [0]) - context_add_key (context, key, lang, line->str); - else - if (!context_used) - context_destroy_and_unlink (context); - } - - g_string_free (line, TRUE); - g_free (key); - g_free (lang); - - /* free the previous_key stuff */ - g_free(previous_key); - previous_key = NULL; - previous_key_lang_level = -1; - - fast_file_close (&mime_file); -} - -/* - * load_mime_list_info_from - * - * Why this special function when a similar one is already in the code? - * Because we need to handle the case where ':' is used to delimit - * the start of a key in a .mime file instead of '=' as is used in - * the .key file. Why is this done? Why are there two mime database - * files with differing standards? We may never know. - * Until we have a better solution, this will suffice. - * - * Both ':' and '=' are used to delimit the start of a key. - */ - -static void -load_mime_list_info_from (const char *filename, GHashTable *hash_table) -{ - FastFile mime_file; - gboolean in_comment, context_used; - GString *line; - int column, c; - ParserState state; - GnomeMimeContext *context; - char *key; - char *lang; - - if (!fast_file_open (&mime_file, filename)) { - return; - } - - in_comment = FALSE; - context_used = FALSE; - column = -1; - context = NULL; - key = NULL; - lang = NULL; - line = g_string_sized_new (120); - state = STATE_NONE; - - while ((c = fast_file_getc (&mime_file)) != EOF) { - handle_char: - column++; - if (c == '\r') - continue; - - if (c == '#' && column == 0) { - in_comment = TRUE; - continue; - } - - if (c == '#' && column == 0) { - in_comment = TRUE; - continue; - } - - if (in_comment) { - if (c == '\n') { - in_comment = FALSE; - column = -1; - } - continue; - } - - if (c == '\n') { - in_comment = FALSE; - column = 0; - if (state == STATE_ON_MIME_TYPE) { - /* set previous key to nothing - for this mime type */ - g_free(previous_key); - previous_key = NULL; - previous_key_lang_level = -1; - - context = context_new (hash_table, line); - context_used = FALSE; - g_string_assign (line, ""); - state = STATE_LOOKING_FOR_KEY; - continue; } - if (state == STATE_ON_VALUE) { - context_used = TRUE; - context_add_key (context, key, lang, line->str); - g_string_assign (line, ""); - g_free (key); - key = NULL; - g_free (lang); - lang = NULL; - state = STATE_LOOKING_FOR_KEY; - continue; - } - continue; - } - - switch (state) { - case STATE_NONE: - if (c != ' ' && c != '\t') - state = STATE_ON_MIME_TYPE; - else - break; /* fall down */ case STATE_ON_MIME_TYPE: if (c == ':') { - in_comment = TRUE; + skip_line = TRUE; + /* setup for a new key */ + previous_key_lang_level = -1; + context = context_new (hash_table, line); + state = STATE_LOOKING_FOR_KEY; break; } - g_string_append_c (line, c); + APPEND_CHAR (line, c); break; case STATE_LOOKING_FOR_KEY: - if (c == '\t' || c == ' ') + if (c == '\t' || c == ' ') { break; + } if (c == '[') { state = STATE_LANG; break; } - if (column == 1) { + if (column == 0) { state = STATE_ON_MIME_TYPE; - g_string_append_c (line, c); + APPEND_CHAR (line, c); break; } state = STATE_ON_KEY; @@ -627,19 +413,22 @@ load_mime_list_info_from (const char *fi case STATE_ON_KEY: if (c == '\\') { c = fast_file_getc (&mime_file); - if (c == EOF) + if (c == EOF) { break; - } + } + } if (c == '=') { - key = g_strdup (line->str); - g_string_assign (line, ""); + key = last_str_end; + APPEND_CHAR (line, '\0'); + last_str_end = line->len; state = STATE_ON_VALUE; break; } - if (c == ':') { - key = g_strdup (line->str); - g_string_assign (line, ""); + if (format == FORMAT_KEYS && c == ':') { + key = last_str_end; + APPEND_CHAR (line, '\0'); + last_str_end = line->len; /* Skip space after colon. There should be one * there. That is how the file is defined. */ @@ -658,46 +447,51 @@ load_mime_list_info_from (const char *fi break; } - g_string_append_c (line, c); + APPEND_CHAR (line, c); break; case STATE_ON_VALUE: - g_string_append_c (line, c); + APPEND_CHAR (line, c); break; case STATE_LANG: if (c == ']') { state = STATE_ON_KEY; - if (line->str [0]) { - g_free(lang); - lang = g_strdup(line->str); - } else { - in_comment = TRUE; + + lang = last_str_end; + APPEND_CHAR (line, '\0'); + last_str_end = line->len; + + if (!line->str [0] || + language_level (line->str + lang) < 0) { + skip_line = TRUE; + key = lang = last_str_end = 0; + g_string_assign (line, ""); state = STATE_LOOKING_FOR_KEY; } - g_string_assign (line, ""); - break; + } else { + APPEND_CHAR (line, c); } - g_string_append_c (line, c); break; } } if (context != NULL) { - if (key && line->str [0]) - context_add_key (context, key, lang, line->str); - else - if (!context_used) + if (key && line->str [0]) { + APPEND_CHAR (line, '\0'); + context_add_key (context, + line->str + key, + lang ? line->str + lang : NULL, + line->str + last_str_end); + } else { + if (g_hash_table_size (context->keys) < 1) { context_destroy_and_unlink (context); + } + } } g_string_free (line, TRUE); - g_free (key); - g_free (lang); - /* free the previous_key stuff */ - g_free(previous_key); - previous_key = NULL; previous_key_lang_level = -1; fast_file_close (&mime_file); @@ -843,11 +637,11 @@ gnome_vfs_mime_init (void) /* * Setup the descriptors for the information loading */ - gnome_mime_dir.dirname = g_strdup (DATADIR "/mime-info"); gnome_mime_dir.system_dir = TRUE; - user_mime_dir.dirname = g_strconcat (g_get_home_dir(), "/.gnome/mime-info", NULL); + user_mime_dir.dirname = g_strconcat + (g_get_home_dir (), "/.gnome/mime-info", NULL); user_mime_dir.system_dir = FALSE; /* Index: test/test-mime-info.c =================================================================== RCS file: /cvs/gnome/gnome-vfs/test/test-mime-info.c,v retrieving revision 1.9 diff -u -p -u -r1.9 test-mime-info.c --- test/test-mime-info.c 3 Aug 2001 19:04:43 -0000 1.9 +++ test/test-mime-info.c 9 Apr 2002 17:48:54 -0000 @@ -225,7 +225,6 @@ main (int argc, char **argv) } - /* do hard stuff on the API */ printf ("all mime-info-related tests succeeded\n"); Index: test/test-performance.c =================================================================== RCS file: /cvs/gnome/gnome-vfs/test/test-performance.c,v retrieving revision 1.1 diff -u -p -u -r1.1 test-performance.c --- test/test-performance.c 20 Feb 2002 09:25:37 -0000 1.1 +++ test/test-performance.c 9 Apr 2002 17:48:54 -0000 @@ -8,16 +8,20 @@ int main (int argc, char **argv) { + int i; GTimer *timer = g_timer_new (); g_type_init (); + gnome_vfs_init (); /* start threads */ g_timer_start (timer); - gnome_vfs_mime_info_reload (); + for (i = 0; i < 10; i++) { + gnome_vfs_mime_info_reload (); + } fprintf (stderr, "mime parse took %g(ms)\n", - g_timer_elapsed (timer, NULL) * 500); + g_timer_elapsed (timer, NULL) * 100); return 0; } -- [email protected] <><, Pseudo Engineer, itinerant idiot _______________________________________________ gnome-vfs maillist - [email protected] http://lists.ximian.com/mailman/listinfo/gnome-vfs