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 587089 - lookup_attribute() takes too much CPU
lookup_attribute() takes too much CPU
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2009-06-26 20:07 UTC by Benjamin Otte (Company)
Modified: 2009-06-30 07:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
split attribute hash initialization into its own function (1.55 KB, patch)
2009-06-29 14:02 UTC, Benjamin Otte (Company)
none Details | Review
split attribute hash initialization into its own function (1.55 KB, patch)
2009-06-29 14:04 UTC, Benjamin Otte (Company)
none Details | Review
split lookup_attribute() into two functions (2.65 KB, patch)
2009-06-29 14:04 UTC, Benjamin Otte (Company)
none Details | Review
add private header with attribute ids (9.79 KB, patch)
2009-06-29 14:04 UTC, Benjamin Otte (Company)
none Details | Review
export and use _g_file_attribute_matcher_matches_id() (13.93 KB, patch)
2009-06-29 14:04 UTC, Benjamin Otte (Company)
none Details | Review
add g_file_attribute_set_*_by_id() and use them (27.09 KB, patch)
2009-06-29 14:04 UTC, Benjamin Otte (Company)
none Details | Review

Description Benjamin Otte (Company) 2009-06-26 20:07:18 UTC
When enumerating local files in the file chooser, almost 1/3rd of the CPU is taken by lookup_attribute() calls, with equal time taken by g_file_attribute_matcher_matches() and g_file_info_create_value_by_name().

As this is internal to gio, it seems like a worthwhile optimization to me to add hard-coded integer IDs for all the content types gio has a #define for and use those ids in the local file enumeration instead of looking them up all the time.

If that sounds like a good idea, I'll prepare a patch.

(Note: This bug is not about adding public API to use those IDs, but only about speeding up glocalfileinfo.c's g_local_file_info_get() function)
Comment 1 Matthias Clasen 2009-06-27 02:21:25 UTC
It sounds reasonable to me
Comment 2 Alexander Larsson 2009-06-29 07:28:50 UTC
All attributes are stored as ints internally. Maybe we can just expose lookup_attribute() as a public GQuark-style thing. Of course, this does not imply we can't have #defines for some predefined set of attributes too.
Comment 3 Benjamin Otte (Company) 2009-06-29 14:02:30 UTC
Created attachment 137560 [details] [review]
split attribute hash initialization into its own function
Comment 4 Benjamin Otte (Company) 2009-06-29 14:04:05 UTC
Created attachment 137561 [details] [review]
split attribute hash initialization into its own function
Comment 5 Benjamin Otte (Company) 2009-06-29 14:04:09 UTC
Created attachment 137562 [details] [review]
split lookup_attribute() into two functions
Comment 6 Benjamin Otte (Company) 2009-06-29 14:04:16 UTC
Created attachment 137563 [details] [review]
add private header with attribute ids

attribute ids are generated when the attribute hash is initialized. This
way we can guarantee that the ids match every time.
Comment 7 Benjamin Otte (Company) 2009-06-29 14:04:19 UTC
Created attachment 137564 [details] [review]
export and use _g_file_attribute_matcher_matches_id()
Comment 8 Benjamin Otte (Company) 2009-06-29 14:04:22 UTC
Created attachment 137565 [details] [review]
add g_file_attribute_set_*_by_id() and use them

This patch and the previous ones fixes the performance issues noted in
Bug 587089 – lookup_attribute() takes too much CPU
It increases performance for querying attributes by ~15% in my tests.
Comment 9 Benjamin Otte (Company) 2009-06-29 14:08:10 UTC
Here's a bunch of patches that implements this.
I put it into a private header - gfileinfo-priv.h - because I'm not sure exporting this implementation detail is a good idea.

Ok to commit?
Comment 10 Alexander Larsson 2009-06-30 07:29:49 UTC
Looks good, please commit.
Comment 11 Benjamin Otte (Company) 2009-06-30 07:32:28 UTC
pushed to master