After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751169 - Improve search performance
Improve search performance
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
Federico Mena Quintero
Depends on:
Blocks:
 
 
Reported: 2015-06-18 16:48 UTC by Matthias Clasen
Modified: 2015-06-18 19:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add batched insertion to file system model (3.63 KB, patch)
2015-06-18 16:50 UTC, Matthias Clasen
committed Details | Review
use batched insertion (2.41 KB, patch)
2015-06-18 16:51 UTC, Matthias Clasen
committed Details | Review

Description Matthias Clasen 2015-06-18 16:48:57 UTC
The file system model code is doing batched insertion to avoid excessive overhead due to resorting and whatnot, when populating from directories. But search results are inserted one-by-one, with a sorting of the model after each one, despite the fact that we do receive them in batches.
Comment 1 Matthias Clasen 2015-06-18 16:50:29 UTC
Created attachment 305614 [details] [review]
add batched insertion to file system model
Comment 2 Matthias Clasen 2015-06-18 16:51:02 UTC
Created attachment 305615 [details] [review]
use batched insertion
Comment 3 Emmanuele Bassi (:ebassi) 2015-06-18 17:01:27 UTC
Review of attachment 305615 [details] [review]:

Looks good.

::: gtk/gtkfilechooserwidget.c
@@ +6129,2 @@
   GFile *file;
+  const char *uri;

Both file and uri declarations could go into the for() loop.

@@ +6133,3 @@
+  for (l = hits; l; l = l->next)
+    {
+      uri = (const gchar *)l->data;

Minor pet peeve of mine: the cast is not really needed.
Comment 4 Emmanuele Bassi (:ebassi) 2015-06-18 17:03:47 UTC
Review of attachment 305614 [details] [review]:

Looks nice.

::: gtk/gtkfilesystemmodel.c
@@ +2139,3 @@
+
+  gtk_file_system_model_query_done (object, res, data);
+                                      gpointer      data)

I was caught a bit off guard from the thaw_updates() being called after the first query finished; the main loop has to re-enter, in order for this to be called, so effectively the model is thawed after all the queries have started. Maybe a comment would be helpful, but it's not strictly necessary.
Comment 5 Matthias Clasen 2015-06-18 19:25:37 UTC
freeze/thaw_updates are counting, so the model should be thawed after the last of the batched updates has completed. At least, thats the intention of the code.