GNOME Bugzilla – Bug 498564
accelerate .desktop file reading
Last modified: 2013-02-19 23:49:35 UTC
This should improve cold start time of eg. the application browser by 1/2 a second or so (according to the lies I encourage iogrind to tell ;-). Also; it's pretty simple :-) I'd really like to defer the 'TryExec' work - which rather ruins the seek pattern too until much later: any ideas wrt. that ? :-)
Created attachment 99406 [details] [review] patch
perf keyword on performance bugs/patches would be appreciated. Setting it
(I don't know that code well yet) Seems to make sense. Just wondering: maybe it's also better to load the recursive dirs after all files in a directory? Ie, delay cached_dir_add_subdir() after the loop. Don't know if it can have an impact on performance. Also, I guess on Linux, we can also use dirent.d_type to make sure it's really a directory before calling the function on the potential subdir. As for TryExec, it might make sense. My first idea to do this would be to have the items hidden (as if TryExec failed), and once we've loaded all the desktop files, do all the TryExec work. It just means keeping a list of those items somewhere for easy access, I guess.
I was thinking about this :-) I guess we have several options: first we could try to defer the work indefinitely - until someone calls the get_try_exec method (or whatever) - AFAIR this is fairly nicely hidden. Secondly - we could defer it until we've read all the .desktop files in a given directory: then simply stat all pending file-names (sorted by path) - which should substantially reduce the overhead [ and be simpler I guess ]. I guess I'd recommend the latter: simply build a sorted list of these things, and run all the stats in eg. /usr/bin together in one block later: would most likely get most of the win rather simply [ also, there are not that many TryExecs overall so memory cost is low ]. Wrt. the d_type check - sure, we can do that for Linux; but since the file will be 'next' anyway (and we're reading the whole directory), I don't see that being a particularly huge win: clearly if there were lots of non-.desktop, non-directory entries it might be, but AFAICS that's not the common case. Thanks.
New patch that splits out and sorts the TryExec stats, it further reduces the simulated I/O startup time - though, still not by as much as I would like (sadly). But perhaps my .desktop directory is rather fragmented. It seems the inode hint is not as wonderful as it could be for ordering in this case. Anyhow - it's clearly a nice win, for little extra complexity:
Created attachment 99432 [details] [review] patch.
(In reply to comment #6) > Created an attachment (id=99432) [edit] > patch. > Silly comment: +DesktopEntryScanState * +desktop_entry_scan_state_new (void) +{ + return g_new0 (DesktopEntryScanState, 1); +} + Should use g_slice_new here? (and free accordingly of course...)
.desktop file reading is now part of GDesktopAppInfo. Please contribute fixes there.