Created attachment 32094 [details]
testcase program

small testcase program.

I compiled with:

gcc `pkg-config --cflags --libs gtk+-2.0` -o prog prog.c

Created attachment 32095 [details]
gzipped strace output

This is a the output from strace (gzipped), from what happens when I press f10
to open the menu.

It shows a lot of stats (1222), and quite a few open and fstats(387).

Obviously, this is not going to be fast.

Some time ago, Owen proposed a mmapped caching scheme for icon theme information
http://mail.gnome.org/archives/gtk-devel-list/2004-April/msg00065.html

I'll attach a first attempt at implementing this scheme which was done by Anders
Carlsson. It seems to work, but needs 

- testing
- documentation (xml man page for gtk-update-icon-cache, cache file format
description)
- the cache file format should ideally moved to freedesktop.org, possibly as an
appendix to the icon theme spec

To test this, you need to run gtk-update-icon-cache on the toplevel theme
directories, like:

for i in /usr/share/icons/*; do gtk-update-icon-cache $i; done

Created attachment 32145 [details] [review]
an icon cache

forgot to mention that the patch doesn't exactly implement the file format
proposed by owen. it leaves out one indirection in the list of directories.
The offsets in the directory list point directly to the names, not to offsets to
the names.

I tried out the patch and ran gtk-update-icon-cache under valgrind.  When I did
so, I saw a "Conditional jump or move depends on uninitialised value(s)"
warning.  So, I tried recompiling with -Os instead of -O2, and now
gtk-update-icon-cache segfaults and valgrind gives an invalid read with almost
the same stack trace as the previous uninitialized value usage.  I'll attach the
valgrind log in just a minute...

Created attachment 32150 [details]
Short valgrind log

Eeek, please replace

    subsubdir = g_build_filename (subdir, name);

with 

    subsubdir = g_build_filename (subdir, name, NULL);

and try again.

That fixes the error.  There appear to be some memory leaks; I'll attach the new
valgrind log in a minute.

Created attachment 32151 [details]
Memory leaks being detected by valgrind

Nice idea. Some thoughts:

- In Linux, if you would put the cache in a subdir, say
cache/gtk-icon-theme.cache ,the original directory mtime is not updated when you
manipulate the cache file. I don't know if that is portable though. Anyway, if
you require people to manually update the caches it doesn't really matter.

- Versioning is a smart thing to do, but how are you going to manage
homedirectories that are shared between machines that want different versions?
Perhaps it's better to put the version in the filename so they can coexist.

- It might be wise to use a magic number or word so these files can be identified.

There are a few rejects when I apply this to gtk-2-4, but only one I wonder if
matters:

***************
*** 1349,1356 ****
        for (d = theme->dirs; d; d = d->next)
        {
          IconThemeDir *dir = d->data;
-
-         suffix = GPOINTER_TO_UINT (g_hash_table_lookup (dir->icons, icon_name));
          if (suffix != ICON_SUFFIX_NONE)
            {
              if (suffix == ICON_SUFFIX_SVG)
--- 1391,1398 ----
        for (d = theme->dirs; d; d = d->next)
        {
          IconThemeDir *dir = d->data;
+
+         suffix = theme_dir_get_icon_suffix (dir, icon_name, NULL);
          if (suffix != ICON_SUFFIX_NONE)
            {
              if (suffix == ICON_SUFFIX_SVG)


Can I use the patch even with this reject, or should this be fixed manually?

Martijn: it is just a cache. not using it because it is an unsupported version
won't kill you. Inventing magic numbers for files with a fixed name seems a bit
pointless. 

Kjartan: I think theme_dir_get_icon_suffix may be a 2.6.x addition, so the
reject should be harmless. Compiling and running the patched gtk will tell you...

Here is a new patch, which contains a couple of changes. Among other things,
it uses icon-theme.cache as filename and adds an IMAGE_DATA_OFFSET field to 
images, so that caching of image contents can be added compatibly later.

Created attachment 32579 [details] [review]
a new patch

Created attachment 32616 [details]
a man page for gtk-update-icon-cache

I just compiled gtk+ with the lastest patch. If i generate the cache using
gtk-update-icon-cache gnome-session hangs while gnome is starting up (no icons
are shown) if i delete all the caches that i generated everything works.

Created attachment 32684 [details] [review]
new patch

Here is a slightly updated version of the icon cache, which actually survives
theme changes without crashing or leaking.

Created attachment 32754 [details] [review]
another update

Another update, which makes testicontheme work as before and fixes a crash.
Note that this patch has a USE_ICON_CACHE env var which allows to turn the use
of an icon cache on/off

Created attachment 32755 [details] [review]
testing the icon cache

Here is a patch which makes testicontheme display the found icon if you use
testicontheme display themename iconname [iconsize].
This makes use of gtk_image_new_from_icon_name() which is not in cvs yet.

2004-10-19  Matthias Clasen  <mclasen@redhat.com>

	Implement icon theme caching.  (#154034, Martijn Vernooij,
	caching schema proposed by Owen Taylor, initial implementation
	by Anders Carlsson)
	
	* gtk/gtkdebug.h: 
	* gtk/gtkmain.c: Add a "icontheme" debug flag.

	* gtk/Makefile.am (gtk_c_sources): Add gtkiconcache.c
	(gtk_private_h_sources): Add gtkiconcache.h
	(bin_PROGRAMS): Add gtk-update-icon-cache

	* gtk/gtkicontheme.c: Use icon caches if they are available.
	Currently, GTK+ uses the cache to get information about the
	available sizes, image file formats and .icon files. The
	actual image data, and the .icon file contents are not 
	cached yet.

	* gtk/updateiconcache.c: A cmdline utility for generating
	icon cache files.

	* gtk/gtkiconcache.h: 
	* gtk/gtkiconcache.c: The glue code to mmap an icon cache
	file and manage the information it contains.

	* tests/testicontheme.c: Add a "display" option.

with this patch (the one on CVS) nautilus show the same icon for all mime types,
devices, folders, etc. screenshots attached

Created attachment 32801 [details]
screenshot showing the problem

'If expects to be given the path to a icon theme directory,'

should be

'It expects to be given the path to a icon theme directory,'

in the manpage.

Diego, as a temporary workaround, you can disable the caching by starting nautilus
with GTK_NO_ICON_CACHE=1 in the environment. 

Diego, do you get error messages if you run nautilus on the commandline ?

it show no error just this: 
diego@helios:~$ nautilus
Initializing epittance nautilus extension

I compiled gtk with this flags:
--disable-static        \
--prefix=/opt/gnome2    \
--enable-debug=yes      \
--disable-gtk-doc

if you need something else...

by the way, the thing that seems to be messed up is all the icons related to
mime types, because the icons are all the same (white page icons) for all the
files that are shown in *any* file chooser dialog.

I have put a print in gtk_icon_theme_lookup_icon, the results are quite interesting:

Without GTK_NO_ICON_CACHE=1, on startup nautilus asks for:
icon-name: gnome-fs-desktop
icon-name: gnome-fs-desktop
icon-name: gnome-fs-regular
icon-name: emblem-documents
icon-name: emblem-symbolic-link
icon-name: emblem-web
icon-name: emblem-pictures
icon-name: gnome-fs-desktop
icon-name: gnome-fs-regular

With GTK_NO_ICON_CACHE it asks for:
icon-name: gnome-fs-desktop
icon-name: gnome-fs-desktop
icon-name: gdm-xnest
icon-name: gnome-fs-network
icon-name: gnome-dev-removable
icon-name: gnome-fs-directory
icon-name: gnome-mime-application-x-abiword
icon-name: gnome-mime-application-x-compressed-tar
icon-name: gnome-fs-regular
icon-name: emblem-documents
icon-name: emblem-symbolic-link
icon-name: emblem-web
icon-name: emblem-pictures
icon-name: gnome-fs-ssh
icon-name: gnome-fs-share
icon-name: gnome-dev-cdrom
icon-name: gnome-fs-trash-full
icon-name: gnome-fs-client
icon-name: gnome-fs-home
icon-name: gnome-fs-desktop
icon-name: gnome-mime-application-x-abiword

Should be fixed now:

2004-10-21  Matthias Clasen  <mclasen@redhat.com>


	* gtk/gtkicontheme.c (gtk_icon_theme_has_icon): Implement for
	cached themes.

	* gtk/gtkiconcache.h: 
	* gtk/gtkiconcache.c (_gtk_icon_cache_has_icon): New function.

	* gtk/updateiconcache.c (scan_directory): Don't skip .icon 
	files which are listed before their images.
	(foreach_remove_func): Instead filter lonely .icon files out
	here.

	* gtk/gtkicontheme.c (theme_dir_get_icon_suffix): Filter out
	the HAS_ICON_FILE flag.

*** Bug 158962 has been marked as a duplicate of this bug. ***