Merge lp:~kamstrup/dee/optimized-getters into lp:dee

[Mikkel Kamstrup Erlandsen]

Optimize the getters in DeeSequenceModel by shaving off a g_variant_ref() compared to the default impl in DeeSerializableModel. Benchmarks indicate a ~13% speedup when reading strings.

This branch also adds a benchmark for reading a string column out of a seqmodel.

It is now possible to pass arguments to tests/test-benchmark that specify prefixes that the benchmark names must match in order to be run. Without args all benchmarks are run.

[Michal Hruby (mhr3)]

The code looks clean, and I can confirm ~15% speedup in the benchmark. Rock on!

review: Approve

=== modified file 'dee/dee-sequence-model.c'
--- dee/dee-sequence-model.c	2011-12-16 13:01:10 +0000
+++ dee/dee-sequence-model.c	2012-01-02 10:51:24 +0000
@@ -125,6 +125,38 @@
 static DeeModelIter* dee_sequence_model_get_iter_at_row (DeeModel     *self,
                                                          guint          row);
 
+static gboolean       dee_sequence_model_get_bool       (DeeModel    *self,
+                                                         DeeModelIter *iter,
+                                                         guint         column);
+
+static guchar         dee_sequence_model_get_uchar      (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static gint32         dee_sequence_model_get_int32     (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static guint32        dee_sequence_model_get_uint32    (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static gint64         dee_sequence_model_get_int64      (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static guint64        dee_sequence_model_get_uint64     (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static gdouble        dee_sequence_model_get_double     (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
+static const gchar*   dee_sequence_model_get_string     (DeeModel     *self,
+                                                         DeeModelIter *iter,
+                                                         guint          column);
+
 static DeeModelIter* dee_sequence_model_next            (DeeModel     *self,
                                                          DeeModelIter *iter);
 
@@ -255,6 +287,14 @@
   iface->get_first_iter       = dee_sequence_model_get_first_iter;
   iface->get_last_iter        = dee_sequence_model_get_last_iter;
   iface->get_iter_at_row      = dee_sequence_model_get_iter_at_row;
+  iface->get_bool             = dee_sequence_model_get_bool;
+  iface->get_uchar            = dee_sequence_model_get_uchar;
+  iface->get_int32            = dee_sequence_model_get_int32;
+  iface->get_uint32           = dee_sequence_model_get_uint32;
+  iface->get_int64            = dee_sequence_model_get_int64;
+  iface->get_uint64           = dee_sequence_model_get_uint64;
+  iface->get_double           = dee_sequence_model_get_double;
+  iface->get_string           = dee_sequence_model_get_string;
   iface->next                 = dee_sequence_model_next;
   iface->prev                 = dee_sequence_model_prev;
   iface->is_first             = dee_sequence_model_is_first;
@@ -484,13 +524,12 @@
 }
 
 static GVariant*
-dee_sequence_model_get_value (DeeModel     *self,
-                              DeeModelIter *iter,
-                              guint         column)
+dee_sequence_model_peek_value (DeeModel     *self,
+                               DeeModelIter *iter,
+                               guint         column)
 {
   DeeSequenceModel *_self = (DeeSequenceModel *)self;
   GPtrArray        *row;
-  GVariant         *col;
 
   g_return_val_if_fail (DEE_IS_SEQUENCE_MODEL (_self), NULL);
   g_return_val_if_fail (iter != NULL, NULL);
@@ -516,16 +555,25 @@
         return NULL;
       }
   
-  col = g_ptr_array_index (row, column);
-  if (G_UNLIKELY (col == NULL))
+  return g_ptr_array_index (row, column);
+}
+
+static GVariant*
+dee_sequence_model_get_value (DeeModel     *self,
+                              DeeModelIter *iter,
+                              guint         column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);
+
+  if (G_UNLIKELY (val == NULL))
     {
-      g_critical ("Internal error: Column %i in DeeSequenceModel@%p"
+      g_critical ("Unable to get value. Column %i in DeeSequenceModel@%p"
                   " holds a NULL value in row %u",
                   column, self, dee_model_get_position (self, iter));
       return NULL;
     }
-  
-  return g_variant_ref (col);
+
+  return g_variant_ref (val);
 }
 
 static DeeModelIter*
@@ -559,6 +607,150 @@
                                                       row);
 }
 
+static gboolean
+dee_sequence_model_get_bool (DeeModel    *self,
+                             DeeModelIter *iter,
+                             guint         column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get boolean. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return FALSE;
+    }
+
+  return g_variant_get_boolean (val);
+}
+
+static guchar
+dee_sequence_model_get_uchar (DeeModel     *self,
+                              DeeModelIter *iter,
+                              guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get byte. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return '\0';
+    }
+
+  return g_variant_get_byte (val);
+}
+
+static gint32
+dee_sequence_model_get_int32 (DeeModel     *self,
+                              DeeModelIter *iter,
+                              guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get int32. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return 0;
+    }
+
+  return g_variant_get_int32 (val);
+}
+
+static guint32
+dee_sequence_model_get_uint32 (DeeModel     *self,
+                               DeeModelIter *iter,
+                               guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get uint32. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return 0;
+    }
+
+  return g_variant_get_uint32 (val);
+}
+
+static gint64
+dee_sequence_model_get_int64 (DeeModel     *self,
+                              DeeModelIter *iter,
+                              guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get int64. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return G_GINT64_CONSTANT (0);
+    }
+
+  return g_variant_get_int64 (val);
+}
+
+static guint64
+dee_sequence_model_get_uint64 (DeeModel     *self,
+                               DeeModelIter *iter,
+                               guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get uint64. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return G_GUINT64_CONSTANT (0);
+    }
+
+  return g_variant_get_uint64 (val);
+}
+
+static gdouble
+dee_sequence_model_get_double (DeeModel     *self,
+                               DeeModelIter *iter,
+                               guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get double. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return 0;
+    }
+
+  return g_variant_get_double (val);
+}
+
+static const gchar*
+dee_sequence_model_get_string     (DeeModel     *self,
+                                   DeeModelIter *iter,
+                                   guint          column)
+{
+  GVariant *val = dee_sequence_model_peek_value (self, iter, column);;
+
+  if (G_UNLIKELY (val == NULL))
+    {
+      g_critical ("Unable to get string. Column %i in DeeSequenceModel@%p"
+                  " holds a NULL value in row %u",
+                  column, self, dee_model_get_position (self, iter));
+      return NULL;
+    }
+
+  return g_variant_get_string (val, NULL);
+}
+
 static DeeModelIter*
 dee_sequence_model_next (DeeModel     *self,
                          DeeModelIter *iter)

=== modified file 'tests/test-benchmark.c'
--- tests/test-benchmark.c	2011-12-15 12:38:28 +0000
+++ tests/test-benchmark.c	2012-01-02 10:51:24 +0000
@@ -117,14 +117,28 @@
 }
 
 static void
-run_benchmarks (void)
+run_benchmarks (gchar **prefixes)
 {
   GList  *iter;
+  gchar  **prefix;
   
   for (iter = benchmarks; iter; iter = iter->next)
     {
       Benchmark *bench = (Benchmark*) iter->data;
-      run_benchmark (bench);
+      
+      if (prefixes == NULL)
+        run_benchmark (bench);
+      else
+        {
+          for (prefix = prefixes; *prefix; prefix++)
+            {
+              if (g_str_has_prefix (bench->name, *prefix))
+                {
+                  run_benchmark (bench);
+                  return;
+                }
+            }
+        }
     }
 }
 
@@ -173,6 +187,26 @@
 }
 
 static void
+bench_seqmodel_read_string_setup (Benchmark *bench)
+{
+  DeeModel *model;
+  guint     limit = 2500, i;
+
+  model = dee_sequence_model_new ();
+  dee_model_set_schema (model, "s", "s", "s", "u", "b", NULL);
+
+  for (i = 0; i < limit; i++)
+    {
+      gchar *random_string = g_strdup_printf ("%"G_GINT32_FORMAT,
+                                              g_test_rand_int());
+      dee_model_prepend (model, random_string, "Hello world", "!", 42, TRUE);
+      g_free (random_string);
+    }
+
+  bench->state = model;
+}
+
+static void
 bench_model_append_run (Benchmark *bench)
 {
   DeeModel *model;
@@ -211,6 +245,25 @@
 }
 
 static void
+bench_model_read_string_run (Benchmark *bench)
+{
+  DeeModel     *model;
+  DeeModelIter *iter, *end;
+
+  g_assert (DEE_IS_MODEL (bench->state));
+
+  model = DEE_MODEL (bench->state);
+
+
+  for (iter = dee_model_get_first_iter (model),
+       end = dee_model_get_last_iter (model);
+       iter != end; iter = dee_model_next (model, iter))
+    {
+      dee_model_get_string (model, iter, 0);
+    }
+}
+
+static void
 bench_gobject_teardown (Benchmark *bench)
 {
   GObject *obj;
@@ -233,6 +286,13 @@
                                 100,
                                 NULL };
 
+Benchmark seqmodel_read_string = { "SequenceModel.read_string",
+                                   bench_seqmodel_read_string_setup,
+                                   bench_model_read_string_run,
+                                   bench_gobject_teardown,
+                                   1000,
+                                   NULL };
+
 Benchmark filtermodel_collate = { "FilterModel.collate",
                                   bench_filter_model_collator_setup,
                                   bench_model_prepend_run,
@@ -247,18 +307,32 @@
                                         100,
                                         NULL };
 
+/* Arguments are interpreted as prefixes that benchmark names must match
+ * in order to be run */
 gint
 main (gint argc, gchar *argv[])
 {
   g_type_init ();
   g_thread_init (NULL);
   g_test_init (&argc, &argv, NULL);
+  
+  /* Extract NULL terminated array of prefixes from arguments */
+  int i;
+  gchar **prefixes = g_new0 (gchar*, argc);
+  for (i = 1; i < argc; i++)
+    {
+      prefixes[i-1] = argv[i];
+    }
 
   add_benchmark (&seqmodel_append);
   add_benchmark (&seqmodel_prepend);
+  add_benchmark (&seqmodel_read_string);
   add_benchmark (&filtermodel_collate);
   add_benchmark (&filtermodel_collate_desc);
 
-  run_benchmarks ();
+  run_benchmarks (argc > 1 ? prefixes : NULL);
+  
+  g_free (prefixes);
+  
   return 0;
 }