GNOME Bugzilla – Bug 500638
gkeyfile speedup ...
Last modified: 2007-12-03 18:59:17 UTC
It turns out that much of the time g_keyfile spends is spent in g_get_language_names - called per key. This patch caches that over the scope of the parsing, which substantially accelerates parsing.
Created attachment 99901 [details] [review] keyfile speedup
for future-proofing, we prolly want to NULL the cache after parsing in each of the 3 call-sites that end up in g_key_file_locale_is_interesting: the lifecycle of g_get_language_names is a little 'interesting' frankly. Failing that, it would (perhaps) be cleaner & sweeter to g_strdupv the lang names as we create the GKeyFile (?) - happy to massage this as you like & commit.
Hmm, g_get_language_names() already has a cache. Is that not effective ? Maybe we should rather fix that then ?
Well - yes, it has a cache - but in order to check whether the cache is stale it calls gutils.c (guess_category_value) - which (for me with only LANG set) checks 4 environment variables via g_getenv before it notices that in fact it's just the same lang as the last foo[en]="baa" line we parsed in the same file ;-) and then uses the 'cache' to avoid some allocation & a strsplit etc. ie. the cache works fine, but the mechanism for detecting dirtiness is the horribly slow thing to be doing per-line :-) And yes using the cache directly without dupping it would be nice, but that has odd lifecycle side-effects. HTH.
Created attachment 99912 [details] profile before - note getenv.
Created attachment 99913 [details] profile afterwards - not no getenv.
Hmm, yeah, this is unfortunate that we spend quite some overhead on this when the likeliness of the value ever changing is very low. Note that your patch _adds_ a g_get_language_names() call in the keep-translations case. Might be better to file the cache on-demand. As to duping or not, not duping is safe as long as you unset the cache after each parse (since it is per-thread), but just duping the damn thing is probably better for maintainability. There is a similar use of g_get_language_names() in gtkiconcache.c in gtk. Does that show up on the radar as well ? It might be instructive to find out if the case where the cached language list gets changed is seen at all in real life.
Do we even support live changes to env var? IMO guess_category_value() should simply keep the result in an static var. Use g_once_init() for bonus.
The live change support was added in bug 160271
2007-12-03 Matthias Clasen <mclasen@redhat.com> * glib/gkeyfile.c: Don't call g_get_language_names() per-key. (#500638, Michael Meeks)