GNOME Bugzilla – Bug 663570
Set file type without stat
Last modified: 2011-11-16 16:31:27 UTC
I've just pushed a bunch of patches to the filechooserentry branch: http://git.gnome.org/browse/glib/log/?h=filechooserentry They add some infrastructure APIs and use them to make g_file_enumerate_children() on local files not stat every child for a few cases where it's unnecessary but was still done before for lack of detecting that case. In particular: - names (display name, edit name, ...) - file type (only when readdir() supports this) This is useful because a lot of enumerations only care about those and we can then really speed things up. The speedup seems to roughly 50% on my F16 ext3 machine for cached directoies. For uncached directories, I can probably easy get to a factor of 10 win. Some examples (first warm, then cold): - $HOME (4200 files): 50ms => 25ms --- 2,475ms => 343ms - /usr/bin (3200 files): 39ms => 22ms --- 1,326ms => 1,305ms - /usr/lib64 (3900 files): 57ms => 44ms --- 2,567ms => 1,927ms - /etc (300 files): 4ms => 2ms --- 28ms => 18ms - / (30 files): 1ms => 0ms --- 19ms => 19ms The example I was looking at in this case was the autocomplete of GtkFileChooser's entry, both for the completion popup and for completing when the tab key is pressed. So when I want to open a file, I often type "/u<tab>/li<tab>6<tab>" to get to "/usr/lib64", so the completions should be ready when I press tab. With 20ms between keystrokes (I measured this, I'm usually a bit slower, but I err on the safe side), this patch makes the difference between a successful completion and a *beep*, which requires me to backtrack and therefore slows me down. The patchset comes complete with tests for the new APIs. I'm not sure if we should export them as public APIs, but I kept them public as I thought they'd be useful in other implementations. Also, there's no way to write tests for them otherwise...
those are great number, and I love that you are adding tests.
From: cb6c267bbb3db41fa7870f8a90fddd7390c77c4b + guint32 diff; + + diff = suba->id - subb->id; Why is the diff unsigned? Also, the sort is reversed to what you typically expect (highest values first), which should perhaps be documented. + /* remove sub_matchers if we match everything anyway */ + if (matcher->all) { + g_array_free (matcher->sub_matchers, TRUE); + matcher->sub_matchers = NULL; This always frees, but it might be NULL already? 6db3cfc9d3dc3783faff623b2177a2ce5d611eeb: +char * +g_file_attribute_matcher_print (GFileAttributeMatcher *matcher) => g_file_attribute_matcher_to_string (it doesn't actually print anything!) 14415bd1cf0f24a3922eafbfcbeaa27cfa5eee71: + local->entries[i].type = file_type_from_dirent (entry->d_type); d_type is a nonstandard extension, you need some autoconf magic here. #ifdef USE_GDIR filename = g_dir_read_name (local->dir); #else - filename = next_file_helper (local); + filename = next_file_helper (local, &file_type); #endif Need to initialize file_type in the USE_GDIR case. Also, in general I worry about the allocations for the AttributeMatcher. Maybe its just silly early optimization, but we're allocating in three levels (AttributeMatcher->GArray->data) in something that is often very small. Maybe we can stuff the GRealArray into the AttributeMatcher struct to avoid some allocation, maybe even having it initially point to some small in-struct array which we stop using if it grows past some fixed size...
(In reply to comment #2) > Why is the diff unsigned? > I don't know. I suppose I realized that it doesn't matter and just did something. Made it gint now. > Also, the sort is reversed to what you typically expect (highest values first), > which should perhaps be documented. > It should be lowest values first and afaics it does that? That's kinda important because the "namespace::*" should be sorted before "namespace::anything" which is required when subtracting. > This always frees, but it might be NULL already? > It's supposed to never be NULL (I allocate the sub_matchers array in new() now.) Anyway, fixed by add a check. > d_type is a nonstandard extension, you need some autoconf magic here. > Fixed. Seems I didn't read the readdir() manpage carefully enough. > #ifdef USE_GDIR > filename = g_dir_read_name (local->dir); > #else > - filename = next_file_helper (local); > + filename = next_file_helper (local, &file_type); > #endif > > Need to initialize file_type in the USE_GDIR case. > Fixed. > > Also, in general I worry about the allocations for the AttributeMatcher. Maybe > its just silly early optimization, but we're allocating in three levels > (AttributeMatcher->GArray->data) in something that is often very small. Maybe > we can stuff the GRealArray into the AttributeMatcher struct to avoid some > allocation, maybe even having it initially point to some small in-struct array > which we stop using if it grows past some fixed size... > I thought I'd care about it when somebody shows it to me on profiles. It inceases code complexity, so I'd like to only worry about it when there's a real need. I've updated the branch with the fixes from above.
> +char * > +g_file_attribute_matcher_print (GFileAttributeMatcher *matcher) > > => g_file_attribute_matcher_to_string (it doesn't actually print anything!) > I followed GVariant here, and it's g_variant_print() to get a string containing printable text. I'm not sure, we don't have established policy on what to call a function that converts a struct into a text representation. I've cc'ed Ryan so he can give an opinion.
I just use 'print' as the opposite of 'parse'. I'm not really sure that I gave it much more thought than that...
no strong opinion here. going by existing practice, 'to_string' wins 5:1 over 'print': gio/gcredentials.h:gchar *g_credentials_to_string (GCredentials *credentials); gio/gdbusprivate.h:gchar *_g_dbus_enum_to_string (GType enum_type, gint value); gio/gicon.h:gchar *g_icon_to_string (GIcon *icon); gio/ginetaddress.h: gchar * (*to_string) (GInetAddress *address); gio/ginetaddress.h:gchar * g_inet_address_to_string (GInetAddress *address); glib/gquark.h:const gchar * g_quark_to_string (GQuark quark) G_GNUC_CONST;
Renamed the function to to_string() now and pushed to master.