Merge branch 'gobject-speedups4' into 'main'

Speed up property lookup

See merge request GNOME/glib!2678

gobject/gobject.c

@@ -485,6 +485,8 @@ g_object_base_class_init (GObjectClass *class)
   class->n_construct_properties = g_slist_length (class->construct_properties);
   class->get_property = NULL;
   class->set_property = NULL;
+  class->pspecs = NULL;
+  class->n_pspecs = 0;
 }
 
 static void
@@ -704,6 +706,75 @@ g_object_class_install_property (GObjectClass *class,
                                               pspec);
 }
 
+typedef struct {
+  const char *name;
+  GParamSpec *pspec;
+} PspecEntry;
+
+static int
+compare_pspec_entry (const void *a,
+                     const void *b)
+{
+  const PspecEntry *ae = a;
+  const PspecEntry *be = b;
+
+  return ae->name < be->name ? -1 : (ae->name > be->name ? 1 : 0);
+}
+
+/* This uses pointer comparisons with @property_name, so
+ * will only work with string literals. */
+static inline GParamSpec *
+find_pspec (GObjectClass *class,
+            const char   *property_name)
+{
+  const PspecEntry *pspecs = (const PspecEntry *)class->pspecs;
+  gsize n_pspecs = class->n_pspecs;
+
+  g_assert (n_pspecs <= G_MAXSSIZE);
+
+  /* The limit for choosing between linear and binary search is
+   * fairly arbitrary.
+   *
+   * Both searches use pointer comparisons against @property_name.
+   * If this function is called with a non-static @property_name,
+   * it will fall through to the g_param_spec_pool_lookup() case.
+   * That’s OK; this is an opportunistic optimisation which relies
+   * on the fact that *most* (but not all) property lookups use
+   * static property names.
+   */
+  if (n_pspecs < 10)
+    {
+      for (gsize i = 0; i < n_pspecs; i++)
+        {
+          if (pspecs[i].name == property_name)
+            return pspecs[i].pspec;
+        }
+    }
+  else
+    {
+      gssize lower = 0;
+      gssize upper = (int)class->n_pspecs - 1;
+      gssize mid;
+
+      while (lower <= upper)
+        {
+          mid = (lower + upper) / 2;
+
+          if (property_name < pspecs[mid].name)
+            upper = mid - 1;
+          else if (property_name > pspecs[mid].name)
+            lower = mid + 1;
+          else
+            return pspecs[mid].pspec;
+        }
+    }
+
+  return g_param_spec_pool_lookup (pspec_pool,
+                                   property_name,
+                                   ((GTypeClass *)class)->g_type,
+                                   TRUE);
+}
+
 /**
  * g_object_class_install_properties:
  * @oclass: a #GObjectClass
@@ -810,6 +881,32 @@ g_object_class_install_properties (GObjectClass  *oclass,
           break;
         }
     }
+
+  /* Save a copy of the pspec array inside the class struct. This
+   * makes it faster to look up pspecs for the class in future when
+   * acting on those properties.
+   *
+   * If a pspec is not in this cache array, calling code will fall
+   * back to using g_param_spec_pool_lookup(), so a pspec not being
+   * in this array is a (potential) performance problem but not a
+   * correctness problem. */
+  if (oclass->pspecs == NULL)
+    {
+      PspecEntry *entries;
+
+      entries = g_new (PspecEntry, n_pspecs - 1);
+
+      for (i = 1; i < n_pspecs; i++)
+        {
+          entries[i - 1].name = pspecs[i]->name;
+          entries[i - 1].pspec = pspecs[i];
+        }
+
+      qsort (entries, n_pspecs - 1, sizeof (PspecEntry), compare_pspec_entry);
+
+      oclass->pspecs = entries;
+      oclass->n_pspecs = n_pspecs - 1;
+    }
 }
 
 /**
@@ -857,6 +954,14 @@ g_object_interface_install_property (gpointer      g_iface,
   (void) install_property_internal (iface_class->g_type, 0, pspec);
 }
 
+/* Inlined version of g_param_spec_get_redirect_target(), for speed */
+static inline void
+param_spec_follow_override (GParamSpec **pspec)
+{
+  if (((GTypeInstance *) (*pspec))->g_class->g_type == G_TYPE_PARAM_OVERRIDE)
+    *pspec = ((GParamSpecOverride *) (*pspec))->overridden;
+}
+
 /**
  * g_object_class_find_property:
  * @oclass: a #GObjectClass
@@ -872,26 +977,17 @@ g_object_class_find_property (GObjectClass *class,
 			      const gchar  *property_name)
 {
   GParamSpec *pspec;
-  GParamSpec *redirect;
 
   g_return_val_if_fail (G_IS_OBJECT_CLASS (class), NULL);
   g_return_val_if_fail (property_name != NULL, NULL);
 
-  pspec = g_param_spec_pool_lookup (pspec_pool,
+  pspec = find_pspec (class, property_name);
-				    property_name,
+
-				    G_OBJECT_CLASS_TYPE (class),
-				    TRUE);
   if (pspec)
-    {
+    param_spec_follow_override (&pspec);
-      redirect = g_param_spec_get_redirect_target (pspec);
+
-      if (redirect)
-	return redirect;
-      else
   return pspec;
 }
-  else
-    return NULL;
-}
 
 /**
  * g_object_interface_find_property:
@@ -1344,14 +1440,6 @@ g_object_freeze_notify (GObject *object)
   g_object_unref (object);
 }
 
-/* Inlined version of g_param_spec_get_redirect_target(), for speed */
-static inline void
-param_spec_follow_override (GParamSpec **pspec)
-{
-  if (((GTypeInstance *) (*pspec))->g_class->g_type == G_TYPE_PARAM_OVERRIDE)
-    *pspec = ((GParamSpecOverride *) (*pspec))->overridden;
-}
-
 static inline void
 g_object_notify_by_spec_internal (GObject    *object,
                                   GParamSpec *pspec)
@@ -2217,8 +2305,8 @@ g_object_new_with_properties (GType          object_type,
       params = g_newa (GObjectConstructParam, n_properties);
       for (i = 0; i < n_properties; i++)
         {
-          GParamSpec *pspec;
+          GParamSpec *pspec = find_pspec (class, names[i]);
-          pspec = g_param_spec_pool_lookup (pspec_pool, names[i], object_type, TRUE);
+
           if (!g_object_new_is_valid_property (object_type, pspec, names[i], params, count))
             continue;
           params[count].pspec = pspec;
@@ -2283,9 +2371,8 @@ g_object_newv (GType       object_type,
 
       for (i = 0; i < n_parameters; i++)
         {
-          GParamSpec *pspec;
+          GParamSpec *pspec = find_pspec (class, parameters[i].name);
 
-          pspec = g_param_spec_pool_lookup (pspec_pool, parameters[i].name, object_type, TRUE);
           if (!g_object_new_is_valid_property (object_type, pspec, parameters[i].name, cparams, j))
             continue;
 
@@ -2356,9 +2443,7 @@ g_object_new_valist (GType        object_type,
       do
         {
           gchar *error = NULL;
-          GParamSpec *pspec;
+          GParamSpec *pspec = find_pspec (class, name);
-
-          pspec = g_param_spec_pool_lookup (pspec_pool, name, object_type, TRUE);
 
           if (!g_object_new_is_valid_property (object_type, pspec, name, params, n_params))
             break;
@@ -2524,7 +2609,7 @@ g_object_setv (GObject       *object,
   guint i;
   GObjectNotifyQueue *nqueue = NULL;
   GParamSpec *pspec;
-  GType obj_type;
+  GObjectClass *class;
 
   g_return_if_fail (G_IS_OBJECT (object));
 
@@ -2532,14 +2617,15 @@ g_object_setv (GObject       *object,
     return;
 
   g_object_ref (object);
-  obj_type = G_OBJECT_TYPE (object);
+
+  class = G_OBJECT_GET_CLASS (object);
 
   if (_g_object_has_notify_handler (object))
     nqueue = g_object_notify_queue_freeze (object, FALSE);
 
   for (i = 0; i < n_properties; i++)
     {
-      pspec = g_param_spec_pool_lookup (pspec_pool, names[i], obj_type, TRUE);
+      pspec = find_pspec (class, names[i]);
 
       if (!g_object_set_is_valid_property (object, pspec, names[i]))
         break;
@@ -2569,6 +2655,7 @@ g_object_set_valist (GObject	 *object,
 {
   GObjectNotifyQueue *nqueue = NULL;
   const gchar *name;
+  GObjectClass *class;
   
   g_return_if_fail (G_IS_OBJECT (object));
 
@@ -2577,6 +2664,8 @@ g_object_set_valist (GObject	 *object,
   if (_g_object_has_notify_handler (object))
     nqueue = g_object_notify_queue_freeze (object, FALSE);
 
+  class = G_OBJECT_GET_CLASS (object);
+
   name = first_property_name;
   while (name)
     {
@@ -2585,10 +2674,7 @@ g_object_set_valist (GObject	 *object,
       gchar *error = NULL;
       GTypeValueTable *vtab;
       
-      pspec = g_param_spec_pool_lookup (pspec_pool,
+      pspec = find_pspec (class, name);
-					name,
-					G_OBJECT_TYPE (object),
-					TRUE);
 
       if (!g_object_set_is_valid_property (object, pspec, name))
         break;
@@ -2661,7 +2747,7 @@ g_object_getv (GObject      *object,
 {
   guint i;
   GParamSpec *pspec;
-  GType obj_type;
+  GObjectClass *class;
 
   g_return_if_fail (G_IS_OBJECT (object));
 
@@ -2670,12 +2756,14 @@ g_object_getv (GObject      *object,
 
   g_object_ref (object);
 
+  class = G_OBJECT_GET_CLASS (object);
+
   memset (values, 0, n_properties * sizeof (GValue));
 
-  obj_type = G_OBJECT_TYPE (object);
   for (i = 0; i < n_properties; i++)
     {
-      pspec = g_param_spec_pool_lookup (pspec_pool, names[i], obj_type, TRUE);
+      pspec = find_pspec (class, names[i]);
+
       if (!g_object_get_is_valid_property (object, pspec, names[i]))
         break;
       g_value_init (&values[i], pspec->value_type);
@@ -2705,11 +2793,14 @@ g_object_get_valist (GObject	 *object,
 		     va_list	  var_args)
 {
   const gchar *name;
+  GObjectClass *class;
   
   g_return_if_fail (G_IS_OBJECT (object));
   
   g_object_ref (object);
 
+  class = G_OBJECT_GET_CLASS (object);
+
   name = first_property_name;
 
   while (name)
@@ -2718,10 +2809,7 @@ g_object_get_valist (GObject	 *object,
       GParamSpec *pspec;
       gchar *error;
 
-      pspec = g_param_spec_pool_lookup (pspec_pool,
+      pspec = find_pspec (class, name);
-					name,
-					G_OBJECT_TYPE (object),
-					TRUE);
 
       if (!g_object_get_is_valid_property (object, pspec, name))
         break;
@@ -2881,10 +2969,7 @@ g_object_get_property (GObject	   *object,
   
   g_object_ref (object);
   
-  pspec = g_param_spec_pool_lookup (pspec_pool,
+  pspec = find_pspec (G_OBJECT_GET_CLASS (object), property_name);
-				    property_name,
-				    G_OBJECT_TYPE (object),
-				    TRUE);
 
   if (g_object_get_is_valid_property (object, pspec, property_name))
     {

gobject/gobject.h

@@ -370,8 +370,12 @@ struct  _GObjectClass
   gsize		flags;
 
   gsize         n_construct_properties;
+
+  gpointer pspecs;
+  gsize n_pspecs;
+
   /* padding */
-  gpointer	pdummy[5];
+  gpointer	pdummy[3];
 };
 
 /**

gobject/tests/properties.c

@@ -173,22 +173,29 @@ test_object_class_init (TestObjectClass *klass)
   properties[PROP_FOO] = g_param_spec_int ("foo", "Foo", "Foo",
                                            -1, G_MAXINT,
                                            0,
-                                           G_PARAM_READWRITE);
+                                           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
   properties[PROP_BAR] = g_param_spec_boolean ("bar", "Bar", "Bar",
                                                FALSE,
-                                               G_PARAM_READWRITE);
+                                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
   properties[PROP_BAZ] = g_param_spec_string ("baz", "Baz", "Baz",
                                               NULL,
                                               G_PARAM_READWRITE);
-  properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux",
-                                               NULL,
-                                               G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY);
 
   gobject_class->set_property = test_object_set_property;
   gobject_class->get_property = test_object_get_property;
   gobject_class->finalize = test_object_finalize;
 
-  g_object_class_install_properties (gobject_class, N_PROPERTIES, properties);
+  g_object_class_install_properties (gobject_class, N_PROPERTIES - 1, properties);
+
+  /* We intentionally install this property separately, to test
+   * that that works, and that property lookup works regardless
+   * how the property was installed.
+   */
+  properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux",
+                                               NULL,
+                                               G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY);
+
+  g_object_class_install_property (gobject_class, PROP_QUUX, properties[PROP_QUUX]);
 }
 
 static void
@@ -205,12 +212,130 @@ properties_install (void)
 {
   TestObject *obj = g_object_new (test_object_get_type (), NULL);
   GParamSpec *pspec;
+  char *name;
 
   g_assert (properties[PROP_FOO] != NULL);
 
   pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "foo");
   g_assert (properties[PROP_FOO] == pspec);
 
+  name = g_strdup ("bar");
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), name);
+  g_assert (properties[PROP_BAR] == pspec);
+  g_free (name);
+
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "baz");
+  g_assert (properties[PROP_BAZ] == pspec);
+
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "quux");
+  g_assert (properties[PROP_QUUX] == pspec);
+
+  g_object_unref (obj);
+}
+
+typedef struct {
+  GObject parent_instance;
+  int value[16];
+} ManyProps;
+
+typedef GObjectClass ManyPropsClass;
+
+static GParamSpec *props[16];
+
+GType many_props_get_type (void) G_GNUC_CONST;
+
+G_DEFINE_TYPE(ManyProps, many_props, G_TYPE_OBJECT)
+
+static void
+many_props_init (ManyProps *self)
+{
+}
+
+static void
+get_prop (GObject    *object,
+          guint       prop_id,
+          GValue     *value,
+          GParamSpec *pspec)
+{
+  ManyProps *mp = (ManyProps *) object;
+
+  if (prop_id > 0 && prop_id < 13)
+    g_value_set_int (value, mp->value[prop_id]);
+  else
+    G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+}
+
+static void
+set_prop (GObject      *object,
+          guint         prop_id,
+          const GValue *value,
+          GParamSpec   *pspec)
+{
+  ManyProps *mp = (ManyProps *) object;
+
+  if (prop_id > 0 && prop_id < 13)
+    mp->value[prop_id] = g_value_get_int (value);
+  else
+    G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+}
+
+static void
+many_props_class_init (ManyPropsClass *class)
+{
+  G_OBJECT_CLASS (class)->get_property = get_prop;
+  G_OBJECT_CLASS (class)->set_property = set_prop;
+
+  props[1] = g_param_spec_int ("one", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[2] = g_param_spec_int ("two", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[3] = g_param_spec_int ("three", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[4] = g_param_spec_int ("four", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[5] = g_param_spec_int ("five", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[6] = g_param_spec_int ("six", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[7] = g_param_spec_int ("seven", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[8] = g_param_spec_int ("eight", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[9] = g_param_spec_int ("nine", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[10] = g_param_spec_int ("ten", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[11] = g_param_spec_int ("eleven", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  props[12] = g_param_spec_int ("twelve", NULL, NULL,
+                               0, G_MAXINT, 0,
+                               G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+  g_object_class_install_properties (G_OBJECT_CLASS (class), 12, props);
+}
+
+static void
+properties_install_many (void)
+{
+  ManyProps *obj = g_object_new (many_props_get_type (), NULL);
+  GParamSpec *pspec;
+
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "one");
+  g_assert (props[1] == pspec);
+
+  pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "ten");
+  g_assert (props[10] == pspec);
+
   g_object_unref (obj);
 }
 
@@ -639,6 +764,7 @@ main (int argc, char *argv[])
   g_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/properties/install", properties_install);
+  g_test_add_func ("/properties/install-many", properties_install_many);
   g_test_add_func ("/properties/notify", properties_notify);
   g_test_add_func ("/properties/notify-queue", properties_notify_queue);
   g_test_add_func ("/properties/construct", properties_construct);