[PATCH] gtkicontheme.c: optimization

[Kajtár Zsolt <kajtarzsolt googlemail com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hi all!

After doing some research on the gtk icon loading, I've changed a few things:

- - struct _GtkIconThemePrivate:
Changed dir_mtimes from GList to SList. The doubly-linking feature is not used,
so a single linked list is enough. Changed all users accordingly.
- - insert_theme:
The icon dir path checking loop and the index.theme key loading loops
got merged. Now the index.theme file is not tried if the directory does not
exists. Non-existing directories are not added to dir_mtime anymore, they where
skipped several times later in theme_subdir_load anyway. The cache checking
(_gtk_icon_cache_new_for_path) is done here now, instead of theme_subdir_load,
this spares a lot of useless icon-cache checking later in theme_subdir_load
(one for each subdir in a theme), when there's no icon cache present. There's
a new variable "new_mtimes" which remembers the start of list where the new
directories were added. This is passed later to theme_subdir_load to only
check newly added directories, and not all previously added. The dir_mtimes
list is appended now, so that the new_mtimes pointer works as expected.
Prepending and reversing does not give any measurable performance improvement
anyway, as this list only contains a few directory entries.
- - load_themes:
The directory is only added to dir_mtimes when it exists.
- - theme_subdir_load:
Added new parameter "new_mtimes", so that only the recently added part of the
dir_mtimes list is checked. _gtk_icon_cache_new_for_path is gone now. Only
stat the subdirectories when there's no cache. No dir_mtime->mtime check for
non-existing directories anymore, as this was taken care in insert_theme and
load_themes.

Now the strace of icon loading looks a bit more sane ;)

Patch in attachment and below.

- -- 
						    Zsolt Kajtar

- --------------
- --- gtk+2.0-2.14.4/gtk/gtkicontheme.c.orig	2008-10-17 06:06:19.000000000 +0200
+++ gtk+2.0-2.14.4/gtk/gtkicontheme.c	2008-11-22 14:20:36.000000000 +0100
@@ -100,7 +100,7 @@
   
   /* time when we last stat:ed for theme changes */
   long last_stat_time;
- -  GList *dir_mtimes;
+  GSList *dir_mtimes;
 
   gulong reset_styles_idle;
 };
@@ -215,7 +215,8 @@
 static void         theme_subdir_load (GtkIconTheme     *icon_theme,
 				       IconTheme        *theme,
 				       GKeyFile         *theme_file,
- -				       char             *subdir);
+				       char             *subdir,
+				       GSList           *new_mtimes);
 static void         do_theme_change   (GtkIconTheme     *icon_theme);
 
 static void     blow_themes               (GtkIconTheme    *icon_themes);
@@ -649,8 +650,8 @@
       g_hash_table_destroy (priv->all_icons);
       g_list_foreach (priv->themes, (GFunc)theme_destroy, NULL);
       g_list_free (priv->themes);
- -      g_list_foreach (priv->dir_mtimes, (GFunc)free_dir_mtime, NULL);
- -      g_list_free (priv->dir_mtimes);
+      g_slist_foreach (priv->dir_mtimes, (GFunc)free_dir_mtime, NULL);
+      g_slist_free (priv->dir_mtimes);
       g_hash_table_destroy (priv->unthemed_icons);
     }
   priv->themes = NULL;
@@ -900,6 +901,7 @@
   GError *error = NULL;
   IconThemeDirMtime *dir_mtime;
   struct stat stat_buf;
+  GSList *new_mtimes = NULL;
   
   priv = icon_theme->priv;
 
@@ -910,44 +912,45 @@
 	return;
     }
   
+  theme_file = NULL;
   for (i = 0; i < priv->search_path_len; i++)
     {
       path = g_build_filename (priv->search_path[i],
 			       theme_name,
 			       NULL);
- -      dir_mtime = g_slice_new (IconThemeDirMtime);
- -      dir_mtime->cache = NULL;
- -      dir_mtime->dir = path;
       if (g_stat (path, &stat_buf) == 0 && S_ISDIR (stat_buf.st_mode))
- -	dir_mtime->mtime = stat_buf.st_mtime;
- -      else
- -	dir_mtime->mtime = 0;
- -
- -      priv->dir_mtimes = g_list_prepend (priv->dir_mtimes, dir_mtime);
- -    }
- -  priv->dir_mtimes = g_list_reverse (priv->dir_mtimes);
+        {
+	  dir_mtime = g_slice_new (IconThemeDirMtime);
+	  dir_mtime->cache = _gtk_icon_cache_new_for_path (path);
+	  dir_mtime->dir = path;
+	  dir_mtime->mtime = stat_buf.st_mtime; /* dir_mtimes is short */
+	  priv->dir_mtimes = g_slist_append (priv->dir_mtimes, dir_mtime);
+	  if (!new_mtimes) new_mtimes = g_slist_last(priv->dir_mtimes);
 
- -  theme_file = NULL;
- -  for (i = 0; i < priv->search_path_len && !theme_file; i++)
- -    {
- -      path = g_build_filename (priv->search_path[i],
- -			       theme_name,
- -			       "index.theme",
- -			       NULL);
- -      if (g_file_test (path, G_FILE_TEST_IS_REGULAR)) 
- -	{
- -	  theme_file = g_key_file_new ();
- -	  g_key_file_set_list_separator (theme_file, ',');
- -	  g_key_file_load_from_file (theme_file, path, 0, &error);
- -	  if (error)
+	  if (!theme_file)
 	    {
- -	      g_key_file_free (theme_file);
- -	      theme_file = NULL;
- -	      g_error_free (error);
- -	      error = NULL;
+	      path = g_build_filename (priv->search_path[i],
+				       theme_name,
+				       "index.theme",
+				       NULL);
+	      if (g_file_test (path, G_FILE_TEST_IS_REGULAR)) 
+		{
+		  theme_file = g_key_file_new ();
+		  g_key_file_set_list_separator (theme_file, ',');
+		  g_key_file_load_from_file (theme_file, path, 0, &error);
+		  if (error)
+		    {
+		      g_key_file_free (theme_file);
+		      theme_file = NULL;
+		      g_error_free (error);
+		      error = NULL;
+		    }
+		}
+	      g_free (path);
 	    }
 	}
- -      g_free (path);
+	else
+	  g_free(path);
     }
 
   if (theme_file || strcmp (theme_name, DEFAULT_THEME_NAME) == 0)
@@ -988,7 +991,7 @@
 
   theme->dirs = NULL;
   for (i = 0; dirs[i] != NULL; i++)
- -    theme_subdir_load (icon_theme, theme, theme_file, dirs[i]);
+    theme_subdir_load (icon_theme, theme, theme_file, dirs[i], new_mtimes);
 
   g_strfreev (dirs);
 
@@ -1066,15 +1069,13 @@
     {
       dir = icon_theme->priv->search_path[base];
 
+      if (g_stat (dir, &stat_buf) != 0 || !S_ISDIR (stat_buf.st_mode))
+	continue;
+
       dir_mtime = g_slice_new (IconThemeDirMtime);
- -      priv->dir_mtimes = g_list_append (priv->dir_mtimes, dir_mtime);
+      priv->dir_mtimes = g_slist_append (priv->dir_mtimes, dir_mtime);
       
       dir_mtime->dir = g_strdup (dir);
- -      dir_mtime->mtime = 0;
- -      dir_mtime->cache = NULL;
- -
- -      if (g_stat (dir, &stat_buf) != 0 || !S_ISDIR (stat_buf.st_mode))
- -	continue;
       dir_mtime->mtime = stat_buf.st_mtime;
 
       dir_mtime->cache = _gtk_icon_cache_new_for_path (dir);
@@ -1568,7 +1569,7 @@
 			 const char   *icon_name)
 {
   GtkIconThemePrivate *priv;
- -  GList *l;
+  GSList *l;
 
   g_return_val_if_fail (GTK_IS_ICON_THEME (icon_theme), FALSE);
   
@@ -1883,7 +1884,7 @@
 {
   GtkIconThemePrivate *priv;
   IconThemeDirMtime *dir_mtime;
- -  GList *d;
+  GSList *d;
   int stat_res;
   struct stat stat_buf;
   GTimeVal tv;
@@ -2465,9 +2466,10 @@
 theme_subdir_load (GtkIconTheme *icon_theme,
 		   IconTheme    *theme,
 		   GKeyFile     *theme_file,
- -		   char         *subdir)
+		   char         *subdir,
+		   GSList       *new_mtimes)
 {
- -  GList *d;
+  GSList *d;
   char *type_string;
   IconThemeDir *dir;
   IconThemeDirType type;
@@ -2539,23 +2541,19 @@
       error = NULL;
     }
 
- -  for (d = icon_theme->priv->dir_mtimes; d; d = d->next)
+  for (d = new_mtimes; d; d = d->next)
     {
       dir_mtime = (IconThemeDirMtime *)d->data;
 
- -      if (dir_mtime->mtime == 0)
- -	continue; /* directory doesn't exist */
- -
        full_dir = g_build_filename (dir_mtime->dir, subdir, NULL);
 
- -      /* First, see if we have a cache for the directory */
- -      if (dir_mtime->cache != NULL || g_file_test (full_dir, G_FILE_TEST_IS_DIR))
- -	{
+      /* No cache, check directory */
 	  if (dir_mtime->cache == NULL)
- -	    {
- -	      /* This will return NULL if the cache doesn't exist or is outdated */
- -	      dir_mtime->cache = _gtk_icon_cache_new_for_path (dir_mtime->dir);
- -	    }
+	    if (!g_file_test (full_dir, G_FILE_TEST_IS_DIR))
+	      {
+		g_free(full_dir);
+		continue;
+	      }
 	  
 	  dir = g_new (IconThemeDir, 1);
 	  dir->type = type;
@@ -2580,9 +2578,6 @@
 	    }
 
 	  theme->dirs = g_list_prepend (theme->dirs, dir);
- -	}
- -      else
- -	g_free (full_dir);
     }
 }
 
- --------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkkoINoACgkQyBOVYiJltRZpbgCgyLPdzSDWM1I/1L5DoXeMdUNq
SfIAn2eMycsg1OAU+F4yyiUaCj2lKXOf
=1IvH
-----END PGP SIGNATURE-----

--- gtk+2.0-2.14.4/gtk/gtkicontheme.c.orig	2008-10-17 06:06:19.000000000 +0200
+++ gtk+2.0-2.14.4/gtk/gtkicontheme.c	2008-11-22 14:20:36.000000000 +0100
@@ -100,7 +100,7 @@
   
   /* time when we last stat:ed for theme changes */
   long last_stat_time;
-  GList *dir_mtimes;
+  GSList *dir_mtimes;
 
   gulong reset_styles_idle;
 };
@@ -215,7 +215,8 @@
 static void         theme_subdir_load (GtkIconTheme     *icon_theme,
 				       IconTheme        *theme,
 				       GKeyFile         *theme_file,
-				       char             *subdir);
+				       char             *subdir,
+				       GSList           *new_mtimes);
 static void         do_theme_change   (GtkIconTheme     *icon_theme);
 
 static void     blow_themes               (GtkIconTheme    *icon_themes);
@@ -649,8 +650,8 @@
       g_hash_table_destroy (priv->all_icons);
       g_list_foreach (priv->themes, (GFunc)theme_destroy, NULL);
       g_list_free (priv->themes);
-      g_list_foreach (priv->dir_mtimes, (GFunc)free_dir_mtime, NULL);
-      g_list_free (priv->dir_mtimes);
+      g_slist_foreach (priv->dir_mtimes, (GFunc)free_dir_mtime, NULL);
+      g_slist_free (priv->dir_mtimes);
       g_hash_table_destroy (priv->unthemed_icons);
     }
   priv->themes = NULL;
@@ -900,6 +901,7 @@
   GError *error = NULL;
   IconThemeDirMtime *dir_mtime;
   struct stat stat_buf;
+  GSList *new_mtimes = NULL;
   
   priv = icon_theme->priv;
 
@@ -910,44 +912,45 @@
 	return;
     }
   
+  theme_file = NULL;
   for (i = 0; i < priv->search_path_len; i++)
     {
       path = g_build_filename (priv->search_path[i],
 			       theme_name,
 			       NULL);
-      dir_mtime = g_slice_new (IconThemeDirMtime);
-      dir_mtime->cache = NULL;
-      dir_mtime->dir = path;
       if (g_stat (path, &stat_buf) == 0 && S_ISDIR (stat_buf.st_mode))
-	dir_mtime->mtime = stat_buf.st_mtime;
-      else
-	dir_mtime->mtime = 0;
-
-      priv->dir_mtimes = g_list_prepend (priv->dir_mtimes, dir_mtime);
-    }
-  priv->dir_mtimes = g_list_reverse (priv->dir_mtimes);
+        {
+	  dir_mtime = g_slice_new (IconThemeDirMtime);
+	  dir_mtime->cache = _gtk_icon_cache_new_for_path (path);
+	  dir_mtime->dir = path;
+	  dir_mtime->mtime = stat_buf.st_mtime; /* dir_mtimes is short */
+	  priv->dir_mtimes = g_slist_append (priv->dir_mtimes, dir_mtime);
+	  if (!new_mtimes) new_mtimes = g_slist_last(priv->dir_mtimes);
 
-  theme_file = NULL;
-  for (i = 0; i < priv->search_path_len && !theme_file; i++)
-    {
-      path = g_build_filename (priv->search_path[i],
-			       theme_name,
-			       "index.theme",
-			       NULL);
-      if (g_file_test (path, G_FILE_TEST_IS_REGULAR)) 
-	{
-	  theme_file = g_key_file_new ();
-	  g_key_file_set_list_separator (theme_file, ',');
-	  g_key_file_load_from_file (theme_file, path, 0, &error);
-	  if (error)
+	  if (!theme_file)
 	    {
-	      g_key_file_free (theme_file);
-	      theme_file = NULL;
-	      g_error_free (error);
-	      error = NULL;
+	      path = g_build_filename (priv->search_path[i],
+				       theme_name,
+				       "index.theme",
+				       NULL);
+	      if (g_file_test (path, G_FILE_TEST_IS_REGULAR)) 
+		{
+		  theme_file = g_key_file_new ();
+		  g_key_file_set_list_separator (theme_file, ',');
+		  g_key_file_load_from_file (theme_file, path, 0, &error);
+		  if (error)
+		    {
+		      g_key_file_free (theme_file);
+		      theme_file = NULL;
+		      g_error_free (error);
+		      error = NULL;
+		    }
+		}
+	      g_free (path);
 	    }
 	}
-      g_free (path);
+	else
+	  g_free(path);
     }
 
   if (theme_file || strcmp (theme_name, DEFAULT_THEME_NAME) == 0)
@@ -988,7 +991,7 @@
 
   theme->dirs = NULL;
   for (i = 0; dirs[i] != NULL; i++)
-    theme_subdir_load (icon_theme, theme, theme_file, dirs[i]);
+    theme_subdir_load (icon_theme, theme, theme_file, dirs[i], new_mtimes);
 
   g_strfreev (dirs);
 
@@ -1066,15 +1069,13 @@
     {
       dir = icon_theme->priv->search_path[base];
 
+      if (g_stat (dir, &stat_buf) != 0 || !S_ISDIR (stat_buf.st_mode))
+	continue;
+
       dir_mtime = g_slice_new (IconThemeDirMtime);
-      priv->dir_mtimes = g_list_append (priv->dir_mtimes, dir_mtime);
+      priv->dir_mtimes = g_slist_append (priv->dir_mtimes, dir_mtime);
       
       dir_mtime->dir = g_strdup (dir);
-      dir_mtime->mtime = 0;
-      dir_mtime->cache = NULL;
-
-      if (g_stat (dir, &stat_buf) != 0 || !S_ISDIR (stat_buf.st_mode))
-	continue;
       dir_mtime->mtime = stat_buf.st_mtime;
 
       dir_mtime->cache = _gtk_icon_cache_new_for_path (dir);
@@ -1568,7 +1569,7 @@
 			 const char   *icon_name)
 {
   GtkIconThemePrivate *priv;
-  GList *l;
+  GSList *l;
 
   g_return_val_if_fail (GTK_IS_ICON_THEME (icon_theme), FALSE);
   
@@ -1883,7 +1884,7 @@
 {
   GtkIconThemePrivate *priv;
   IconThemeDirMtime *dir_mtime;
-  GList *d;
+  GSList *d;
   int stat_res;
   struct stat stat_buf;
   GTimeVal tv;
@@ -2465,9 +2466,10 @@
 theme_subdir_load (GtkIconTheme *icon_theme,
 		   IconTheme    *theme,
 		   GKeyFile     *theme_file,
-		   char         *subdir)
+		   char         *subdir,
+		   GSList       *new_mtimes)
 {
-  GList *d;
+  GSList *d;
   char *type_string;
   IconThemeDir *dir;
   IconThemeDirType type;
@@ -2539,23 +2541,19 @@
       error = NULL;
     }
 
-  for (d = icon_theme->priv->dir_mtimes; d; d = d->next)
+  for (d = new_mtimes; d; d = d->next)
     {
       dir_mtime = (IconThemeDirMtime *)d->data;
 
-      if (dir_mtime->mtime == 0)
-	continue; /* directory doesn't exist */
-
        full_dir = g_build_filename (dir_mtime->dir, subdir, NULL);
 
-      /* First, see if we have a cache for the directory */
-      if (dir_mtime->cache != NULL || g_file_test (full_dir, G_FILE_TEST_IS_DIR))
-	{
+      /* No cache, check directory */
 	  if (dir_mtime->cache == NULL)
-	    {
-	      /* This will return NULL if the cache doesn't exist or is outdated */
-	      dir_mtime->cache = _gtk_icon_cache_new_for_path (dir_mtime->dir);
-	    }
+	    if (!g_file_test (full_dir, G_FILE_TEST_IS_DIR))
+	      {
+		g_free(full_dir);
+		continue;
+	      }
 	  
 	  dir = g_new (IconThemeDir, 1);
 	  dir->type = type;
@@ -2580,9 +2578,6 @@
 	    }
 
 	  theme->dirs = g_list_prepend (theme->dirs, dir);
-	}
-      else
-	g_free (full_dir);
     }
 }