GNOME Bugzilla – Bug 616997
gtk_recent_manager_add_item() is slow
Last modified: 2012-09-03 17:31:32 UTC
Adding items to a GtkRecentManager is very slow. On both GEdit and Geany [1] it is a heavy bottleneck when opening a lot of files. GEdit takes about 90 seconds to open 300 empty files, but simply removing the call to gtk_recent_manager_add_full() makes it only take about 25 seconds. Geany, likewise, takes about 120 seconds to load the same 300 empty files with its gtk_recent_manager_add() call and about 11 seconds without. A little go with GDB seems to show that it is because GtkRecentManager saves its state at nearly each call to _add(), calls g_file_set_contents() which fsync()s -- and then I/O and slowness.
Created attachment 159745 [details] [review] A proposed fix based on deferring updates A proposed patch that defers the signal emission. It perhaps needs tweaking the deferring timeout duration/method if this solution is chosen.
Created attachment 159794 [details] [review] Update of the proposed fix based on deferring updates Same as previous patch but uses a GTimeVal in place of a time_t; allowing better precision. Also reduce the default deferring timeout to 100ms.
Created attachment 173017 [details] [review] The patch in attachment 159794 [details] [review] is way too complicated: a simple guard against a timeout source would be enough to coalesce emission of the ::changed signal. The patch I'm attaching is slightly different, though: it also uses a monotonic counter to force an emission in case we receive many ::changed emission requests within the timeout interval - more than one every millisecond for 250 milliseconds, to be precise - to avoid leaving the RecentManager in a dirty state for too many requests.
Created attachment 173020 [details] [review] recent-manager: Coalesce multiple changes Since the ::changed implementation of GtkRecentManager implies a synchronous write operation, when we receive multiple requests to emit a ::changed signal we might end up blocking. This change coalesces multiple ::changed emission requests using the following sequence: • the first request will install a timeout in 250 ms, which will emit the ::changed signal • each further request while the timeout has not been emitted will increase a counter ‣ if the counter reaches 250 before the timeout has been emitted, then the RecentManager will remove the timeout source and force a signal emission and reset the counter This sequence should guarantee that frequent ::changed emission requests are coalesced, and also guarantee that we don't let them dangle for too long.
Attachment 173020 [details] pushed as ce5a29b - recent-manager: Coalesce multiple changes
(In reply to comment #3) > The patch in attachment 159794 [details] [review] is way too complicated: a simple guard against a > timeout source would be enough to coalesce emission of the ::changed signal. Right, I should have thought of directly using a TimeoutSource rather than rebuilding the thing. (In reply to comment #5) > Attachment 173020 [details] pushed as ce5a29b - recent-manager: Coalesce multiple changes Great, thank you! I've just made 2 little test to actually check again whether this works to fix the reason why I reported the original issue, and it does it well :) I tested with the current 2-24 branch, first without and then with the patch (ce5a29b) cherry-picked: GEdit 2.30.4 takes ~37 seconds to open 200 empty files without the patch, and only ~8s with it. For Geany 0.20-svn, it varies from ~50s (without) to ~8s (with). It would be great to have it in 2.24 if it doesn't seem to introduce bugs 'til then. Would it be possible? -- I ask since this bug is now tagged 2.90.x... Regards, Colomban
*** Bug 668747 has been marked as a duplicate of this bug. ***
Why is this bug still open; is it just because the fix wasn't put in 2.24?
Yes, it got reopened in comment 6.
Backported the fix to gtk-2-24, since this has been giving no issues in the GTK3 branch for a long time now.