[gtk+/filesystemmodel: 17/28] add _gtk_file_system_model_get_value() to speed up sorting

[Benjamin Otte <otte src gnome org>]

commit f8c59d67818c5f2b6d731e115be23bf9d2c39dad
Author: Benjamin Otte <otte gnome org>
Date:   Mon Jun 22 11:54:56 2009 +0200

    add _gtk_file_system_model_get_value() to speed up sorting
    
    "speed up" is kind of an understatement here, because sorting went from
    ~5 seconds to less than 0.5 seconds for my test case. There is 2
    significant problems with gtk_tree_model_get():
    1) The value is copied, which takes quite a bit of time for strings.
       ~25% of excessive time
    2) The tree model functions need to lookup the interface vfunc. And
       gtk_tree_model_get() doesn't do that only once, but multiple times
       (verifying column id, getting the actual value, ...)
       ~75% of excessive time
    
    This function works around both of these issues, but introduces an ugly
    extra function that should not be necessary, but I can't imagine how to
    get rid of it.

 gtk/gtkfilechooserdefault.c |   65 +++++++++++++++++++++++++++++++++---------
 gtk/gtkfilesystemmodel.c    |   62 +++++++++++++++++++++++++++++-----------
 gtk/gtkfilesystemmodel.h    |    3 ++
 3 files changed, 99 insertions(+), 31 deletions(-)
---
diff --git a/gtk/gtkfilechooserdefault.c b/gtk/gtkfilechooserdefault.c
index 441b04e..c45f4ee 100644
--- a/gtk/gtkfilechooserdefault.c
+++ b/gtk/gtkfilechooserdefault.c
@@ -6094,10 +6094,20 @@ install_list_model_filter (GtkFileChooserDefault *impl)
 
 #define COMPARE_DIRECTORIES										       \
   GtkFileChooserDefault *impl = user_data;								       \
+  GtkFileSystemModel *fs_model = model == (GtkTreeModel *) impl->browse_files_model ?                          \
+      impl->browse_files_model : NULL;                                                                         \
   gboolean dir_a, dir_b;										       \
 													       \
-  gtk_tree_model_get (model, a, MODEL_COL_IS_FOLDER, &dir_a, -1);                                              \
-  gtk_tree_model_get (model, b, MODEL_COL_IS_FOLDER, &dir_b, -1);                                              \
+  if (fs_model)                                                                                                \
+    {                                                                                                          \
+      dir_a = g_value_get_boolean (_gtk_file_system_model_get_value (fs_model, a, MODEL_COL_IS_FOLDER));       \
+      dir_b = g_value_get_boolean (_gtk_file_system_model_get_value (fs_model, b, MODEL_COL_IS_FOLDER));       \
+    }                                                                                                          \
+  else                                                                                                         \
+    {                                                                                                          \
+      gtk_tree_model_get (model, a, MODEL_COL_IS_FOLDER, &dir_a, -1);                                          \
+      gtk_tree_model_get (model, b, MODEL_COL_IS_FOLDER, &dir_b, -1);                                          \
+    }                                                                                                          \
 													       \
   if (dir_a != dir_b)											       \
     return impl->list_sort_ascending ? (dir_a ? -1 : 1) : (dir_a ? 1 : -1) /* Directories *always* go first */
@@ -6112,23 +6122,34 @@ name_sort_func (GtkTreeModel *model,
   COMPARE_DIRECTORIES;
   else
     {
-      gchar *key_a, *key_b;
+      const char *key_a, *key_b;
       gint result;
 
-      gtk_tree_model_get (model, a, MODEL_COL_NAME_COLLATED, &key_a, -1);
-      gtk_tree_model_get (model, b, MODEL_COL_NAME_COLLATED, &key_b, -1);
+      if (fs_model)
+        {
+          key_a = g_value_get_string (_gtk_file_system_model_get_value (fs_model, a, MODEL_COL_NAME_COLLATED));
+          key_b = g_value_get_string (_gtk_file_system_model_get_value (fs_model, b, MODEL_COL_NAME_COLLATED));
+        }
+      else
+        {
+          gtk_tree_model_get (model, a, MODEL_COL_NAME_COLLATED, &key_a, -1);
+          gtk_tree_model_get (model, b, MODEL_COL_NAME_COLLATED, &key_b, -1);
+        }
 
       if (key_a && key_b)
         result = strcmp (key_a, key_b);
       else if (key_a)
-        return 1;
+        result = 1;
       else if (key_b)
-        return -1;
+        result = -1;
       else
-        return 0;
+        result = 0;
 
-      g_free (key_a);
-      g_free (key_b);
+      if (!fs_model)
+        {
+          g_free ((char *) key_a);
+          g_free ((char *) key_b);
+        }
 
       return result;
     }
@@ -6146,8 +6167,16 @@ size_sort_func (GtkTreeModel *model,
     {
       gint64 size_a, size_b;
 
-      gtk_tree_model_get (model, a, MODEL_COL_SIZE, &size_a, -1);
-      gtk_tree_model_get (model, b, MODEL_COL_SIZE, &size_b, -1);
+      if (fs_model)
+        {
+          size_a = g_value_get_int64 (_gtk_file_system_model_get_value (fs_model, a, MODEL_COL_SIZE));
+          size_b = g_value_get_int64 (_gtk_file_system_model_get_value (fs_model, b, MODEL_COL_SIZE));
+        }
+      else
+        {
+          gtk_tree_model_get (model, a, MODEL_COL_SIZE, &size_a, -1);
+          gtk_tree_model_get (model, b, MODEL_COL_SIZE, &size_b, -1);
+        }
 
       return size_a > size_b ? -1 : (size_a == size_b ? 0 : 1);
     }
@@ -6165,8 +6194,16 @@ mtime_sort_func (GtkTreeModel *model,
     {
       glong ta, tb;
 
-      gtk_tree_model_get (model, a, MODEL_COL_MTIME, &ta, -1);
-      gtk_tree_model_get (model, b, MODEL_COL_MTIME, &tb, -1);
+      if (fs_model)
+        {
+          ta = g_value_get_long (_gtk_file_system_model_get_value (fs_model, a, MODEL_COL_MTIME));
+          tb = g_value_get_long (_gtk_file_system_model_get_value (fs_model, b, MODEL_COL_MTIME));
+        }
+      else
+        {
+          gtk_tree_model_get (model, a, MODEL_COL_MTIME, &ta, -1);
+          gtk_tree_model_get (model, b, MODEL_COL_MTIME, &tb, -1);
+        }
 
       return ta > tb ? -1 : (ta == tb ? 0 : 1);
     }
diff --git a/gtk/gtkfilesystemmodel.c b/gtk/gtkfilesystemmodel.c
index 34d8396..d4cc013 100644
--- a/gtk/gtkfilesystemmodel.c
+++ b/gtk/gtkfilesystemmodel.c
@@ -348,26 +348,14 @@ gtk_file_system_model_get_value (GtkTreeModel *tree_model,
 				 GValue       *value)
 {
   GtkFileSystemModel *model = GTK_FILE_SYSTEM_MODEL (tree_model);
-  FileModelNode *node;
+  const GValue *original;
   
   g_return_if_fail ((guint) column < model->n_columns);
   g_return_if_fail (ITER_IS_VALID (model, iter));
 
-  node = get_node (model, ITER_INDEX (iter));
-  
-  if (!G_VALUE_TYPE (&node->values[column]))
-    {
-      g_value_init (&node->values[column], model->column_types[column]);
-      model->get_func (model, 
-                       node->file, 
-                       node->info, 
-                       column, 
-                       &node->values[column],
-                       model->get_data);
-    }
-
-  g_value_init (value, model->column_types[column]);
-  g_value_copy (&node->values[column], value);
+  original = _gtk_file_system_model_get_value (model, iter, column);
+  g_value_init (value, G_VALUE_TYPE (original));
+  g_value_copy (original, value);
 }
 
 static gboolean
@@ -1166,7 +1154,7 @@ _gtk_file_system_model_get_info (GtkFileSystemModel *model,
  * @model: a #GtkFileSystemModel
  * @iter: a #GtkTreeIter pointing to a row of @model
  * 
- * Gets the path for a particular row in @model. 
+ * Gets the file for a particular row in @model. 
  *
  * Return value: the file. This object is owned by @model and
  *   or freed. If you want to save the path for later use,
@@ -1185,6 +1173,46 @@ _gtk_file_system_model_get_file (GtkFileSystemModel *model,
   return node->file;
 }
 
+/**
+ * _gtk_file_system_model_get_value:
+ * @model: a #GtkFileSystemModel
+ * @iter: a #GtkTreeIter pointing to a row of @model
+ * @column: the column to get the value for
+ *
+ * Gets the value associated with the given row @iter and @column. 
+ * This is a performance optimization for the calls 
+ * gtk_tree_model_get() or gtk_tree_model_get_value(), which copy 
+ * the value and spend a considerable amount of time in iterator 
+ * lookups. Both of which are slow.
+ *
+ * Returns: a pointer to the actual value as stored in @model.
+ **/
+const GValue *
+_gtk_file_system_model_get_value (GtkFileSystemModel *model,
+                                  GtkTreeIter *       iter,
+                                  int                 column)
+{
+  FileModelNode *node;
+
+  g_return_val_if_fail (GTK_IS_FILE_SYSTEM_MODEL (model), NULL);
+  g_return_val_if_fail (column >= 0 && (guint) column < model->n_columns, NULL);
+
+  node = get_node (model, ITER_INDEX (iter));
+    
+  if (!G_VALUE_TYPE (&node->values[column]))
+    {
+      g_value_init (&node->values[column], model->column_types[column]);
+      model->get_func (model, 
+      node->file, 
+      node->info, 
+      column, 
+      &node->values[column],
+      model->get_data);
+    }
+  
+  return &node->values[column];
+}
+
 gboolean
 _gtk_file_system_model_get_iter_for_file (GtkFileSystemModel *model,
 					  GtkTreeIter        *iter,
diff --git a/gtk/gtkfilesystemmodel.h b/gtk/gtkfilesystemmodel.h
index bf8e766..fc50721 100644
--- a/gtk/gtkfilesystemmodel.h
+++ b/gtk/gtkfilesystemmodel.h
@@ -54,6 +54,9 @@ gboolean            _gtk_file_system_model_get_iter_for_file(GtkFileSystemModel
 							     GFile *             file);
 GFile *             _gtk_file_system_model_get_file         (GtkFileSystemModel *model,
 							     GtkTreeIter        *iter);
+const GValue *      _gtk_file_system_model_get_value        (GtkFileSystemModel *model,
+                                                             GtkTreeIter *       iter,
+                                                             int                 column);
 void                _gtk_file_system_model_set_show_hidden  (GtkFileSystemModel *model,
 							     gboolean            show_hidden);
 void                _gtk_file_system_model_set_show_folders (GtkFileSystemModel *model,