GNOME Bugzilla – Bug 602456
Syncronous fsync() in idle_save_application_usage()
Last modified: 2009-11-24 18:23:55 UTC
Every 5 seconds idle_save_application_usage() calls g_file_replace(); the output stream created by this function does a fsync() when closed. Doing a fsync during heavy IO can stall the entire GUI for several seconds; this makes the shell not very usable during something like a updating a bunch of packages. - We should drop the interval down from 5 seconds to maybe 5 minutes; it doesn't matter if your last few seconds of app usage get lost. - We should make sure that the fsync() gets called in a different thread. The simple thing to do is to write the output to a temporary memory buffer then use g_file_replace_contents_async(). See http://mail.gnome.org/archives/gtk-devel-list/2009-March/msg00082.html - there's apparently a flag to suppress the behavior fsync(), but I dion't think we want to use it, because while we don't care if a few seconds of data gets lost, it's bad if the file ends up empty, so using the flag would be basically saying that we know better than GIO about whether the fsync() is necessary or not.
Created attachment 148153 [details] [review] [ShellAppUsage] Bump save timeout to 5 minutes, close output asynchronously The synchronous close causes us to block in fsync() which has extremely poor interactivity implications on ext3. Also, the 5 second timeout was an accidental commit from debugging, 5 minutes is fine.
Created attachment 148403 [details] [review] Bump save timeout to 5 minutes, close output asynchronously I tested that, yes, this does make the fsync() get called in another thread. If you don't care about errors, you don't need to actually pass a callback at all though. Updated patch attached.
Review of attachment 148403 [details] [review]: Looks fine, except for minor niggles. ::: src/shell-app-usage.c @@ +67,2 @@ /* How often we save internally app data, in seconds */ +#define SAVE_APPS_TIMEOUT_SECONDS 5*60 Expressions should be parenthesized, and standard operator spacing: (5 * 60) @@ +715,3 @@ out: if (!error) + g_output_stream_close_async (G_OUTPUT_STREAM(data_output), 0, NULL, NULL, NULL); To be picky, if we are changing this line, should add the missing space in 'G_OUTPUT_STREAM(data_output)'
pushed with those changes Attachment 148403 [details] pushed as e2ac769 - Bump save timeout to 5 minutes, close output asynchronously