After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 562174 - gtkicontheme.c: optimization (excess work)
gtkicontheme.c: optimization (excess work)
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: Other
2.14.x
Other Linux
: Normal enhancement
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2008-11-24 20:46 UTC by Kajtár Zsolt
Modified: 2018-02-10 03:43 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
Patch for gtkicontheme.c (6.55 KB, patch)
2008-11-24 20:54 UTC, Kajtár Zsolt
none Details | Review

Description Kajtár Zsolt 2008-11-24 20:46:46 UTC
Hi all!

From original mail:
-------------
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.
---------------------
Details:

All tests done with svn checkouts of atk (1302), glib (7681), gtk (21806) and pango (2744). Libraries installed to /usr.

First I was running without icon caches and did an strace on "examples/menu/itemfactory". gtk-icon-theme-name = "gnome". Used the file menu to trigger icon loading, and got the following:

stat64("/home/soci/.icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or direc
stat64("/home/soci/.local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such f
stat64("/usr/local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat64("/usr/local/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file o
stat64("/usr/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file or dire

Ok so far.

stat64("/home/soci/.icons/gnome/index.theme", 0x77ffda3c) = -1 ENOENT (No such f
stat64("/home/soci/.local/share/icons/gnome/index.theme", 0x77ffda3c) = -1 ENOEN
stat64("/usr/local/share/icons/gnome/index.theme", 0x77ffda3c) = -1 ENOENT (No s

The directories didn't existed above, makes no sense to look for index.theme in them.

stat64("/usr/share/icons/gnome/index.theme", {st_mode=S_IFREG|0644, st_size=1207
open("/usr/share/icons/gnome/index.theme", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=12074, ...}) = 0
read(4, "[Icon Theme]\nName=GNOME\nName[af]"..., 4096) = 4096
read(4, "t\304\203 GNOME\nComment[ru]=\320\241\321\202\320\260\320"..., 4096) =
read(4, "ixed\n\n[22x22/apps]\nSize=22\nConte"..., 4096) = 3882
read(4, "", 4096)                       = 0
close(4)                                = 0

Ok, index found.

stat64("/usr/share/icons/gnome/8x8/emblems", {st_mode=S_IFDIR|0755, st_size=4096
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = -1 ENOEN
open("/usr/share/icons/gnome/8x8/emblems", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIR
fstat64(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getdents64(4, /* 9 entries */, 4096)    = 344
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0
stat64("/usr/share/icons/gnome/16x16/actions", {st_mode=S_IFDIR|0755, st_size=20
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = -1 ENOEN
open("/usr/share/icons/gnome/16x16/actions", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_D
fstat64(4, {st_mode=S_IFDIR|0755, st_size=20480, ...}) = 0
getdents64(4, /* 102 entries */, 4096)  = 4072
getdents64(4, /* 99 entries */, 4096)   = 4096
getdents64(4, /* 101 entries */, 4096)  = 4072
getdents64(4, /* 39 entries */, 4096)   = 1616
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0

Goes through all subdirectories, for each it tries to open the same icon-cache, which of course still doesn't exists... Now a lot more directories later hicolor comes:

stat64("/home/soci/.icons/hicolor", 0x77ffdb54) = -1 ENOENT (No such file or dir
stat64("/home/soci/.local/share/icons/hicolor", 0x77ffdb54) = -1 ENOENT (No such
stat64("/usr/local/share/icons/hicolor", 0x77ffdb54) = -1 ENOENT (No such file o
stat64("/usr/share/icons/hicolor", {st_mode=S_IFDIR|0755, st_size=4096, ...}) =
stat64("/usr/local/share/pixmaps/hicolor", 0x77ffdb54) = -1 ENOENT (No such file
stat64("/usr/share/pixmaps/hicolor", 0x77ffdb54) = -1 ENOENT (No such file or di
stat64("/home/soci/.icons/hicolor/index.theme", 0x77ffda3c) = -1 ENOENT (No such
stat64("/home/soci/.local/share/icons/hicolor/index.theme", 0x77ffda3c) = -1 ENO
stat64("/usr/local/share/icons/hicolor/index.theme", 0x77ffda3c) = -1 ENOENT (No

But actually we already tried and failed these directories above.

stat64("/usr/share/icons/hicolor/index.theme", {st_mode=S_IFREG|0644, st_size=22
open("/usr/share/icons/hicolor/index.theme", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=22251, ...}) = 0
read(4, "[Icon Theme]\nName=Hicolor\nCommen"..., 4096) = 4096
read(4, "ems,192x192/intl,192x192/mimetyp"..., 4096) = 4096
brk(0x8222000)                          = 0x8222000
brk(0x8221000)                          = 0x8221000
read(4, "eTypes\nType=Threshold\n\n[24x24/pl"..., 4096) = 4096
read(4, "=International\nType=Threshold\n\n["..., 4096) = 4096
read(4, "96\nContext=FileSystems\nType=Thre"..., 4096) = 4096
read(4, "ntext=Applications\nType=Scalable"..., 4096) = 1771
read(4, "", 4096)                       = 0
close(4)                                = 0

Ok, index loading. Now it gets interesting:

stat64("/usr/share/icons/gnome/16x16/actions", {st_mode=S_IFDIR|0755, st_size=20
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = -1 ENOEN
open("/usr/share/icons/gnome/16x16/actions", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_D
fstat64(4, {st_mode=S_IFDIR|0755, st_size=20480, ...}) = 0
getdents64(4, /* 102 entries */, 4096)  = 4072
getdents64(4, /* 99 entries */, 4096)   = 4096
getdents64(4, /* 101 entries */, 4096)  = 4072
getdents64(4, /* 39 entries */, 4096)   = 1616
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0
stat64("/usr/share/icons/hicolor/16x16/actions", {st_mode=S_IFDIR|0755, st_size=
stat64("/usr/share/icons/hicolor", {st_mode=S_IFDIR|0755, st_size=4096, ...}) =
open("/usr/share/icons/hicolor/icon-theme.cache", O_RDONLY|O_LARGEFILE) = -1 ENO
open("/usr/share/icons/hicolor/16x16/actions", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O
fstat64(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getdents64(4, /* 17 entries */, 4096)   = 728
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0

This repeates for all subdirectories. Any reason to check gnome again? After these some other directories are checked for pixmaps.

To see something I've added an access mark in "theme_subdir_load" right after "for (d = icon_theme...".

stat64("/home/soci/.icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or direc
stat64("/home/soci/.local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such f
stat64("/usr/local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat64("/usr/local/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file o
stat64("/usr/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file or dire
stat64("/home/soci/.icons/gnome/index.theme", 0x77ffda3c) = -1 ENOENT (No such f
stat64("/home/soci/.local/share/icons/gnome/index.theme", 0x77ffda3c) = -1 ENOEN
stat64("/usr/local/share/icons/gnome/index.theme", 0x77ffda3c) = -1 ENOENT (No s
stat64("/usr/share/icons/gnome/index.theme", {st_mode=S_IFREG|0644, st_size=1207
open("/usr/share/icons/gnome/index.theme", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=12074, ...}) = 0
read(4, "[Icon Theme]\nName=GNOME\nName[af]"..., 4096) = 4096
read(4, "t\304\203 GNOME\nComment[ru]=\320\241\321\202\320\260\320"..., 4096) =
read(4, "ixed\n\n[22x22/apps]\nSize=22\nConte"..., 4096) = 3882
read(4, "", 4096)                       = 0
close(4)                                = 0

Same sofar.

access("/home/soci/.icons/gnome/8x8/emblems", F_OK) = -1 ENOENT (No such file or
access("/home/soci/.local/share/icons/gnome/8x8/emblems", F_OK) = -1 ENOENT (No
access("/usr/local/share/icons/gnome/8x8/emblems", F_OK) = -1 ENOENT (No such fi
access("/usr/share/icons/gnome/8x8/emblems", F_OK) = 0
stat64("/usr/share/icons/gnome/8x8/emblems", {st_mode=S_IFDIR|0755, st_size=4096

What's this? I've got a cache.

stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 5
fstat64(5, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
mmap2(NULL, 7198228, PROT_READ, MAP_PRIVATE, 5, 0) = 0x6eb70000
close(5)                                = 0
close(4)                                = 0

Ok, cache found.

access("/usr/local/share/pixmaps/gnome/8x8/emblems", F_OK) = -1 ENOENT (No such
access("/usr/share/pixmaps/gnome/8x8/emblems", F_OK) = -1 ENOENT (No such file o

First tries continue.

access("/home/soci/.icons/gnome/16x16/actions", F_OK) = -1 ENOENT (No such file
access("/home/soci/.local/share/icons/gnome/16x16/actions", F_OK) = -1 ENOENT (N
access("/usr/local/share/icons/gnome/16x16/actions", F_OK) = -1 ENOENT (No such
access("/usr/share/icons/gnome/16x16/actions", F_OK) = 0
access("/usr/local/share/pixmaps/gnome/16x16/actions", F_OK) = -1 ENOENT (No suc
access("/usr/share/pixmaps/gnome/16x16/actions", F_OK) = -1 ENOENT (No such file

Now a this repeats for all subdirectories. Fortunately five from six checks are catched by the mtime=0 check for non-existing directories early, but why try at all?

Now I've put the mark a bit later to "theme->dirs = g_list_prepend".

access("/usr/share/icons/gnome/8x8/emblems", F_OK) = 0
access("/usr/share/icons/gnome/16x16/actions", F_OK) = 0
access("/usr/share/icons/gnome/16x16/animations", F_OK) = 0
access("/usr/share/icons/gnome/16x16/apps", F_OK) = 0
access("/usr/share/icons/gnome/16x16/categories", F_OK) = 0

After lot more lines gnome finishes, now hicolor starts:

access("/usr/share/icons/hicolor/16x16/actions", F_OK) = 0
access("/usr/share/icons/gnome/16x16/animations", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/animations", F_OK) = 0
access("/usr/share/icons/gnome/16x16/apps", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/apps", F_OK) = 0
access("/usr/share/icons/gnome/16x16/categories", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/categories", F_OK) = 0

Hey, duplicate entries added to the list for gnome?

access("/usr/share/icons/gnome/96x96/places", F_OK) = -1 ENOENT (No such file or
access("/usr/share/icons/hicolor/96x96/places", F_OK) = 0

Also non-existing directories are added for gnome...

Now after some patching the cached case will look like below. The access mark is installed just right at the start of loop, just like above.

stat64("/home/soci/.icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or direc
stat64("/home/soci/.local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such f
stat64("/usr/local/share/icons/gnome", 0x77ffdb54) = -1 ENOENT (No such file or
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0

Now index.theme is not checked in non-existing directories.

stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 5
fstat64(5, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
mmap2(NULL, 7198228, PROT_READ, MAP_PRIVATE, 5, 0) = 0x6eb70000
close(5)                                = 0
close(4)                                = 0

Icon cache is opened early. No tries for some directories before.

stat64("/usr/share/icons/gnome/index.theme", {st_mode=S_IFREG|0644, st_size=1207
open("/usr/share/icons/gnome/index.theme", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=12074, ...}) = 0
read(4, "[Icon Theme]\nName=GNOME\nName[af]"..., 4096) = 4096
read(4, "t\304\203 GNOME\nComment[ru]=\320\241\321\202\320\260\320"..., 4096) =
read(4, "ixed\n\n[22x22/apps]\nSize=22\nConte"..., 4096) = 3882
read(4, "", 4096)                       = 0
close(4)                                = 0

Try index theme for content.

stat64("/usr/local/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file o
stat64("/usr/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file or dire

More directories tried.

access("/usr/share/icons/gnome/8x8/emblems", F_OK) = 0
access("/usr/share/icons/gnome/16x16/actions", F_OK) = 0
access("/usr/share/icons/gnome/16x16/animations", F_OK) = 0
access("/usr/share/icons/gnome/16x16/apps", F_OK) = 0
access("/usr/share/icons/gnome/16x16/categories", F_OK) = 0

Ok, the mark shown directories added to the list. No more non-existing directories tried in the loop.

Now lots of directories added, and hicolor comes. Directory checks and cache open as for gnome.

access("/usr/share/icons/hicolor/16x16/actions", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/animations", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/apps", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/categories", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/devices", F_OK) = 0
access("/usr/share/icons/hicolor/16x16/emblems", F_OK) = 0

Only hicolor directories added here.

OK, now let's see the case without a cache. I'll put some icon overrides for gnome in my home directory. Also the access mark was moved after the g_file_test to only show entries which get added to the list.

stat64("/home/soci/.icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0

Ok, found. 

stat64("/home/soci/.icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/home/soci/.icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = -1 ENOE

Non cache.

stat64("/home/soci/.icons/gnome/index.theme", 0x77ffda1c) = -1 ENOENT (No such f

No index. That'll comes from the original gnome theme.

stat64("/home/soci/.local/share/icons/gnome", 0x77ffdb44) = -1 ENOENT (No such f
stat64("/usr/local/share/icons/gnome", 0x77ffdb44) = -1 ENOENT (No such file or

Some non existing directories.

stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat64("/usr/share/icons/gnome", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
open("/usr/share/icons/gnome/icon-theme.cache", O_RDONLY|O_LARGEFILE) = 5
fstat64(5, {st_mode=S_IFREG|0644, st_size=7198228, ...}) = 0
mmap2(NULL, 7198228, PROT_READ, MAP_PRIVATE, 5, 0) = 0x6eb70000
close(5)                                = 0
close(4)                                = 0

Ok, cache found.

stat64("/usr/share/icons/gnome/index.theme", {st_mode=S_IFREG|0644, st_size=1207
open("/usr/share/icons/gnome/index.theme", O_RDONLY|O_LARGEFILE) = 4
fstat64(4, {st_mode=S_IFREG|0644, st_size=12074, ...}) = 0
read(4, "[Icon Theme]\nName=GNOME\nName[af]"..., 4096) = 4096
read(4, "t\304\203 GNOME\nComment[ru]=\320\241\321\202\320\260\320"..., 4096) =
read(4, "ixed\n\n[22x22/apps]\nSize=22\nConte"..., 4096) = 3882
read(4, "", 4096)                       = 0
close(4)                                = 0

Ok, index found.

stat64("/usr/local/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file o
stat64("/usr/share/pixmaps/gnome", 0x77ffdb54) = -1 ENOENT (No such file or dire

More directories, but nothing there.

stat64("/home/soci/.icons/gnome/8x8/emblems", 0x77ffda2c) = -1 ENOENT (No such f
access("/usr/share/icons/gnome/8x8/emblems", F_OK) = 0

No overrides for 8x8 icons, only the system theme from cache gets added.

stat64("/home/soci/.icons/gnome/16x16/actions", {st_mode=S_IFDIR|0755, st_size=2
access("/home/soci/.icons/gnome/16x16/actions", F_OK) = 0
open("/home/soci/.icons/gnome/16x16/actions", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_
fstat64(4, {st_mode=S_IFDIR|0755, st_size=20480, ...}) = 0
getdents64(4, /* 102 entries */, 4096)  = 4072
getdents64(4, /* 99 entries */, 4096)   = 4096
getdents64(4, /* 101 entries */, 4096)  = 4072
getdents64(4, /* 39 entries */, 4096)   = 1616
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0
access("/usr/share/icons/gnome/16x16/actions", F_OK) = 0
stat64("/home/soci/.icons/gnome/16x16/animations", {st_mode=S_IFDIR|0755, st_siz
access("/home/soci/.icons/gnome/16x16/animations", F_OK) = 0
open("/home/soci/.icons/gnome/16x16/animations", O_RDONLY|O_NONBLOCK|O_LARGEFILE
fstat64(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getdents64(4, /* 6 entries */, 4096)    = 216
getdents64(4, /* 0 entries */, 4096)    = 0
close(4)                                = 0

Both the system and local icons get added. No more tries for the non-existing cache in the home directory.

The rest of the log is similar. The patch:

Index: gtk/gtkicontheme.c
===================================================================
--- gtk/gtkicontheme.c	(revision 21806)
+++ gtk/gtkicontheme.c	(working copy)
@@ -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;
+        {
+	  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);
 
-      priv->dir_mtimes = g_list_prepend (priv->dir_mtimes, dir_mtime);
-    }
-  priv->dir_mtimes = g_list_reverse (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);
     }
 }
 
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 21806)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2008-11-24  Kajtar Zsolt <kajtarzsolt@googlemail.com>
+
+	* gtk/gtkicontheme.c: Optimization of insert_theme, load_themes,
+	theme_subdir_load.
+
 2008-11-24  Tristan Van Berkom <tvb@gnome.org>
 
 	* gtk/gtkalignment.c: Bug 561539 - Fix warnings when size allocations
Comment 1 Kajtár Zsolt 2008-11-24 20:54:06 UTC
Created attachment 123336 [details] [review]
Patch for gtkicontheme.c

Proposed patch to fix the excess work done during icon subdirectory scanning. Improves the cached case as well.
Comment 2 Matthias Clasen 2018-02-10 03:43:49 UTC
We're moving to gitlab! As part of this move, we are closing bugs that haven't seen activity in more than 5 years. If this issue is still imporant to you and
still relevant with GTK+ 3.22 or master, please consider creating a gitlab issue
for it.