Merge lp:~mhr3/dee/gi-modeliter into lp:dee

[Michal Hruby]

Makes DeeModelIter usable from introspected languages by registering it as a boxed GType.

[lp:~mhr3/dee/gi-modeliter]

340. By Michal Hruby on 2012-01-26

Make DeeModelIter usable from introspected languages

[Mikkel Kamstrup Erlandsen (kamstrup)]

16 + /* FIXME: this implementation will work fine with DeeSequenceModel, but what
17 + * about others? */

This is not an issue. It's implicit in the DeeModel interface that iters are stable (that is you can keep them around and pass like pointers).

What I am mostly concerned about for this branch is performance. The signals will now do lookup in the Boxed type tree, so it might be a slow-down.

Before I can approve can you:

a) post benchmarks from before/after
b) add some tests for the boxing logic

review: Needs Fixing

[Mikkel Kamstrup Erlandsen (kamstrup)]

Ok, I can the benchmarks myself. Looks like the boxing is a non-issue:

TRUNK:
=== SequenceModel.append ===
Runs : 100
Total runtime : 2.262057s
Avg. runtime : 0.022621s
Std. deviation : 0.001089s
Accuracy : 95.185140% [good]

=== SequenceModel.prepend ===
Runs : 100
Total runtime : 2.128361s
Avg. runtime : 0.021284s
Std. deviation : 0.000753s
Accuracy : 96.459987% [good]

THIS BRANCH:
=== SequenceModel.append ===
Runs : 100
Total runtime : 2.286914s
Avg. runtime : 0.022869s
Std. deviation : 0.001062s
Accuracy : 95.357487% [good]

=== SequenceModel.prepend ===
Runs : 100
Total runtime : 2.169105s
Avg. runtime : 0.021691s
Std. deviation : 0.000880s
Accuracy : 95.941740% [good]

[lp:~mhr3/dee/gi-modeliter]

341. By Michal Hruby on 2012-01-26

Add tests

[Michal Hruby (mhr3)]

Bump (tests were added).

[Mikkel Kamstrup Erlandsen (kamstrup)]

Perfect! Having signals and iters work over GI is just awesome :-)

review: Approve

=== modified file 'src/dee-model.c'
--- src/dee-model.c	2012-01-10 07:54:04 +0000
+++ src/dee-model.c	2012-01-26 19:12:25 +0000
@@ -141,6 +141,38 @@
                                                    GVariant **out_row_members,
                                                    va_list   *args);
 
+/* 
+ * We provide here a couple of DeeModelIter functions, so that they're usable
+ * from introspected languages.
+ */
+
+static gpointer
+dee_model_iter_copy (gpointer boxed)
+{
+  /* FIXME: this implementation will work fine with DeeSequenceModel, but what
+   * about others? */
+  return boxed;
+}
+
+static void
+dee_model_iter_free (gpointer boxed)
+{
+}
+
+GType dee_model_iter_get_type (void)
+{
+  static GType dee_model_iter_type = 0;
+
+  if (dee_model_iter_type == 0)
+  {
+    dee_model_iter_type = g_boxed_type_register_static ("DeeModelIter",
+                                                        dee_model_iter_copy,
+                                                        dee_model_iter_free);
+  }
+
+  return dee_model_iter_type;
+}
+
 static void
 dee_model_default_init (DeeModelInterface *klass)
 {
@@ -157,9 +189,9 @@
                   G_SIGNAL_RUN_LAST,
                   G_STRUCT_OFFSET (DeeModelIface,row_added),
                   NULL, NULL,
-                  g_cclosure_marshal_VOID__POINTER,
+                  g_cclosure_marshal_VOID__BOXED,
                   G_TYPE_NONE, 1,
-                  G_TYPE_POINTER);
+                  DEE_TYPE_MODEL_ITER);
   /**
    * DeeModel::row-removed:
    * @self: the #DeeModel on which the signal is emitted
@@ -174,9 +206,9 @@
                   G_SIGNAL_RUN_LAST,
                   G_STRUCT_OFFSET (DeeModelIface,row_removed),
                   NULL, NULL,
-                  g_cclosure_marshal_VOID__POINTER,
+                  g_cclosure_marshal_VOID__BOXED,
                   G_TYPE_NONE, 1,
-                  G_TYPE_POINTER);
+                  DEE_TYPE_MODEL_ITER);
   /**
    * DeeModel::row-changed:
    * @self: the #DeeModel on which the signal is emitted
@@ -190,9 +222,9 @@
                   G_SIGNAL_RUN_LAST,
                   G_STRUCT_OFFSET (DeeModelIface,row_changed),
                   NULL, NULL,
-                  g_cclosure_marshal_VOID__POINTER,
+                  g_cclosure_marshal_VOID__BOXED,
                   G_TYPE_NONE, 1,
-                  G_TYPE_POINTER);
+                  DEE_TYPE_MODEL_ITER);
 }
 
 /**

=== modified file 'src/dee-model.h'
--- src/dee-model.h	2012-01-10 07:54:04 +0000
+++ src/dee-model.h	2012-01-26 19:12:25 +0000
@@ -30,6 +30,8 @@
 
 G_BEGIN_DECLS
 
+#define DEE_TYPE_MODEL_ITER (dee_model_iter_get_type ())
+
 #define DEE_TYPE_MODEL (dee_model_get_type ())
 
 #define DEE_MODEL(obj) \
@@ -225,6 +227,8 @@
   void     (*_dee_model_8) (void);
 };
 
+GType           dee_model_iter_get_type         (void);
+
 /**
  * dee_model_get_type:
  *

=== modified file 'tests/test-model-rows.c'
--- tests/test-model-rows.c	2012-01-09 11:17:10 +0000
+++ tests/test-model-rows.c	2012-01-26 19:12:25 +0000
@@ -48,13 +48,22 @@
 static void test_illegal_access  (RowsFixture *fix, gconstpointer data);
 static void test_sorted          (RowsFixture *fix, gconstpointer data);
 
+static void test_model_iter_copy (RowsFixture *fix, gconstpointer data);
+static void test_model_iter_free (RowsFixture *fix, gconstpointer data);
+
 void
 test_model_rows_create_suite (void)
 {
+#define ITER_DOMAIN "/ModelIter/Boxing"
 #define SEQ_DOMAIN "/Model/Sequence/Rows"
 #define PROXY_DOMAIN "/Model/Proxy/Rows"
 #define TXN_DOMAIN "/Model/Transaction/Rows"
 
+  g_test_add (ITER_DOMAIN"/Copy", RowsFixture, 0,
+              seq_rows_setup, test_model_iter_copy, seq_rows_teardown);
+  g_test_add (ITER_DOMAIN"/Free", RowsFixture, 0,
+              seq_rows_setup, test_model_iter_free, seq_rows_teardown);
+
   g_test_add (SEQ_DOMAIN"/Allocation", RowsFixture, 0,
               seq_rows_setup, test_rows_allocation, seq_rows_teardown);
   g_test_add (PROXY_DOMAIN"/Allocation", RowsFixture, 0,
@@ -176,6 +185,33 @@
   fix->model = NULL;
 }
 
+static void test_model_iter_copy (RowsFixture *fix, gconstpointer data)
+{
+  DeeModelIter *iter, *copied;
+ 
+  iter = dee_model_append (fix->model, 10, "Rooney");
+
+  copied = g_boxed_copy (DEE_TYPE_MODEL_ITER, iter);
+
+  g_assert (iter == copied);
+}
+
+static void test_model_iter_free (RowsFixture *fix, gconstpointer data)
+{
+  DeeModelIter *iter;
+  gint i;
+
+  iter = dee_model_append (fix->model, 10, "Rooney");
+
+  g_boxed_free (DEE_TYPE_MODEL_ITER, iter);
+
+  /* And since it's supposed to be a no-op we can do this */
+  for (i = 0; i < 100; i++)
+    g_boxed_free (DEE_TYPE_MODEL_ITER, iter);
+
+  /* Didn't crash? Good! */
+}
+
 static void
 test_rows_allocation (RowsFixture *fix, gconstpointer data)
 {