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  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
• 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
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...
*** 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.