(In reply to comment #0)
> Under strace, applications that use GtkIconTheme can be seen re-stat()ing icon
> directories every so often:
> 
> 20685 1352746286.879498 stat("/usr/share/pixmaps/gnome", 0x7ffff99e8640) = -1
> ENOENT (No such file or directory) <0.000074>
> 20685 1352746286.879696 stat("/home/jhbuild/usr/share/pixmaps/gnome",
> 0x7ffff99e8640) = -1 ENOENT (No such file or directory) <0.000048>
> 20685 1352746286.882252 stat("/home/jhbuild/usr/share/pixmaps/hicolor",
> 0x7ffff99e8640) = -1 ENOENT (No such file or directory) <0.000097>
> 20685 1352746286.882510 stat("/usr/share/pixmaps/hicolor", 0x7ffff99e8640) = -1
> 20685 1352746286.883499 stat("/home/jhbuild/usr/share/pixmaps",
> {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0 <0.000046>
> 20685 1352746286.883693 stat("/usr/share/pixmaps", {st_mode=S_IFDIR|0755,
> st_size=4096, ...}) = 0 <0.000045>

*really*?

> One possibility would be to schedule the rescan (if one is needed) in an idle,
> and give the application an answer based on old information; then rescan when
> we're back in the main loop, and in the unlikely event that the theme has
> changed since the last iteration, the application will have to respond to the
> ::changed signal. (Gtk assumes that g_main_context_default() is the only main
> context that can touch the Gtk thread, right?)

only the thread that called gtk_main() can touch gtk.
 
> Another possibility would be to use a GFileMonitor, so we inotify on the theme
> directories; that would effectively also move responding to changes into an
> idle.

with inotify fds being the scarce resource that they are, I'd be wary of adding one for each directory we have to watch - especially when we still watch crappy places like XDG_DATA_DIR/pixmaps.

(In reply to comment #1)
> *really*?

I'm trying to reduce the longest times the Shell compositor spends unresponsive, e.g. when opening the overview. This particular bit isn't high-priority or anything, but avoiding it could cut a couple of percent off that time (or more, if heavy I/O makes stat() slow).

If it's difficult to fix then it's not worth doing, but if it's as easy as deferring it to an idle, it could be among the better cost/benefit ratios available.

Making the Shell defer all calls to gtk_icon_theme_lookup_by_gicon() into an idle would also work (it loads icons asynchronously anyway), but it looks to me as though that function is only "normally" meant to perform I/O the first time you use the icon theme?

> > Gtk assumes that g_main_context_default() is the only main
> > context that can touch the Gtk thread, right?)
> 
> only the thread that called gtk_main() can touch gtk.

... and gtk_main() does indeed iterate g_main_context_default() (aka the NULL GMainContext), so scheduling an idle in that main context is a safe thing to do.

> with inotify fds being the scarce resource that they are, I'd be wary of adding
> one for each directory we have to watch

Fair enough.

sorry, my "really" power was about the fact that we still search under the pixmap directory. It turns out that the spec mandates that location - probably to cope with XPM assets because this is still 1997.

Created attachment 228973 [details] [review]
[rfc] GtkIconTheme: rescan themes in an idle

gtk_icon_theme_lookup_by_gicon() doesn't normally perform I/O,
assuming the one-off cost of loading the theme in the first place has
already been paid. However, if you leave GNOME Shell idle for more
than 5 seconds then do anything that will trigger a call to
gtk_icon_theme_lookup_by_gicon(), it will spend time stat()ing all the
icon theme directories before returning anything.

Moving the rescan into an idle means we still block on that I/O in the
main thread, but at least we get a chance to respond to user action
right now, and sort out whether our cached icon theme is still valid
later.

---

In my test environment (opening the overview for the first time after logging into gnome-shell and waiting for it to be idle), this does not have a significant effect on login times, but it does move the stat() calls out of the critical path.

Timings done with gnome-shell master + my patches from Bug #687465, under a GLib which measures the length of each dispatch() in the main main-context and issues a g_warning() if it's more than 20ms:

dispatch() blocking for 64.9ms -> 63.8ms with standard deviation 0.5ms -> 2.4ms, then for 156.0ms -> 155.1ms with std.dev. 1.8ms -> 1.8ms. Statistics from 3 runs without this patch -> 4 runs with it.

stat() calls confirmed to have moved later via strace: under a GLib with more profiling overhead, which does an access() call at the beginning and end of each dispatch() (so, not directly comparable), the stat() calls are delayed to a subsequent dispatch(), 359ms after the beginning of the first of the two long dispatch() calls.

Why does stat() have an effect? Shouldn't all those directories be cached in the kernel?

We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.

Probably still a problem.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/410.