[glib/g-property] gproperty: Updates after review

[Emmanuele Bassi <ebassi src gnome org>]

commit a63249051e6c20ca3a49dcdb3873285f0e04f939
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Wed May 18 08:04:24 2011 +0100

    gproperty: Updates after review
    
    First round of updates past Matthias' review:
    
      - reduce the size of the Property structure by using guint16 for
        the offsets instead of a full gssize;
    
      - upon its first invocation, store the default value set using
        g_property_set_default_value() inside a separate GValue, to
        avoid creating a hash table for the common case of a property
        with no overrides;
    
      - use direct hashing for the overridden defaults hash table, as
        the type names we use as keys are interned;
    
      - compute the lock quark on installation, if the property is
        atomic and it's using the default locking mechanism; the
        default locking uses the quark instead of the string to
        retrieve the bitlock.

 gobject/gproperty.c |  204 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 128 insertions(+), 76 deletions(-)
---
diff --git a/gobject/gproperty.c b/gobject/gproperty.c
index a17e0e5..dcc3942 100644
--- a/gobject/gproperty.c
+++ b/gobject/gproperty.c
@@ -441,17 +441,22 @@ struct _GProperty
   GType gtype;
   GType class_gtype;
 
-  gssize type_size;
-  gssize priv_offset;
-  gssize field_offset;
+  guint16 type_size;
+  guint16 priv_offset;
+  guint16 field_offset;
 
-  GHashTable *default_values;
+  GValue *default_value;
+
+  GHashTable *overridden_defaults;
 
   GType prerequisite;
 
   GPropertyLockFunc lock_func;
   GPropertyUnlockFunc unlock_func;
 
+  /* used by the default locking */
+  GQuark lock_quark;
+
   guint is_installed : 1;
 };
 
@@ -735,6 +740,37 @@ g_##g_t##_property_get_value (GProperty *property, \
     } \
 }
 
+static void
+g_property_default_lock (GProperty *property,
+                         gpointer   gobject)
+{
+  gpointer bit_lock_p;
+
+  bit_lock_p = g_object_get_qdata (gobject, property->lock_quark);
+  if (bit_lock_p == NULL)
+    {
+      bit_lock_p = g_new0 (gint, 1);
+      g_object_set_qdata (gobject, property->lock_quark, bit_lock_p);
+    }
+
+  g_bit_lock (bit_lock_p, 0);
+}
+
+static void
+g_property_default_unlock (GProperty *property,
+                           gpointer   gobject)
+{
+  gpointer bit_lock_p;
+
+  bit_lock_p = g_object_get_qdata (gobject, property->lock_quark);
+  if (G_UNLIKELY (bit_lock_p == NULL))
+    return;
+
+  g_object_set_qdata (gobject, property->lock_quark, NULL);
+  g_bit_unlock (bit_lock_p, 0);
+  g_free (bit_lock_p);
+}
+
 static inline void
 property_lock_internal (GProperty *property,
                         gpointer   gobject)
@@ -744,6 +780,8 @@ property_lock_internal (GProperty *property,
 
   if (property->lock_func != NULL)
     property->lock_func (property, gobject);
+  else
+    g_property_default_lock (property, gobject);
 }
 
 static inline void
@@ -755,6 +793,8 @@ property_unlock_internal (GProperty *property,
 
   if (property->unlock_func != NULL)
     property->unlock_func (property, gobject);
+  else
+    g_property_default_unlock (property, gobject);
 }
 
 static inline gpointer
@@ -2831,6 +2871,19 @@ _g_property_set_installed (GProperty *property,
   else
     property->priv_offset = -1;
 
+  /* if the property is using the default locking, pre-compute the
+   * quark for the lock
+   */
+  if ((property->flags & G_PROPERTY_ATOMIC) != 0 &&
+      property->lock_func == NULL)
+    {
+      gchar *lock_n = g_strconcat ("__g_property_lock_",
+                                   G_PARAM_SPEC (property)->name,
+                                   NULL);
+      property->lock_quark = g_quark_from_string (lock_n);
+      g_free (lock_n);
+    }
+
   property->class_gtype = class_gtype;
   property->is_installed = TRUE;
 }
@@ -3448,13 +3501,38 @@ g_property_set_default_value (GProperty    *property,
 
   g_return_if_fail (g_value_type_transformable (G_VALUE_TYPE (default_value), gtype));
 
-  if (property->default_values == NULL)
-    property->default_values = g_hash_table_new_full (g_str_hash, g_str_equal,
-                                                      g_free,
-                                                      g_free);
+  /* if this is the first time g_property_set_default() has been called
+   * on this property, then we special-case it, and we use a single
+   * GValue, to avoid creating an hash table for the common case of a
+   * property with a single default.
+   */
+  if (property->default_value == NULL)
+    {
+      property->default_value = g_new0 (GValue, 1);
+      g_value_init (property->default_value, gtype);
+      if (!g_value_transform (default_value, property->default_value))
+        {
+          g_critical (G_STRLOC ": unable to set the default value for "
+                      "property '%s': the type %s of the value is not "
+                      "compatible with the type of the %s property",
+                      G_PARAM_SPEC (property)->name,
+                      g_type_name (G_VALUE_TYPE (default_value)),
+                      g_type_name (gtype));
+
+          g_value_unset (property->default_value);
+          g_free (property->default_value);
+        }
+
+      return;
+    }
+
+  if (property->overridden_defaults == NULL)
+    property->overridden_defaults = g_hash_table_new_full (NULL, NULL,
+                                                           g_free,
+                                                           g_free);
 
   gtype_name = g_type_name (G_TYPE_FROM_CLASS (gobject_class));
-  if (g_hash_table_lookup (property->default_values, gtype_name) != NULL)
+  if (g_hash_table_lookup (property->overridden_defaults, gtype_name) != NULL)
     {
       g_critical (G_STRLOC ": a default value of property '%s' for "
                   "type '%s' has already been defined.",
@@ -3479,9 +3557,7 @@ g_property_set_default_value (GProperty    *property,
       return;
     }
 
-  g_hash_table_insert (property->default_values,
-                       g_strdup (gtype_name),
-                       value);
+  g_hash_table_insert (property->overridden_defaults, (gpointer) gtype_name, value);
 }
 
 /**
@@ -3507,11 +3583,25 @@ g_property_get_default_value_for_type (GProperty *property,
   g_return_if_fail (property->gtype != G_TYPE_INVALID);
   g_return_if_fail (g_type_name (gtype) != 0);
 
-  if (property->default_values != NULL)
+  if (property->default_value != NULL)
+    {
+      if (!g_value_transform (property->default_value, value))
+        {
+          g_critical (G_STRLOC ": Unable to copy the default value "
+                      "of property '%s' from type '%s' to type '%s'",
+                      G_PARAM_SPEC (property)->name,
+                      g_type_name (property->gtype),
+                      g_type_name (G_VALUE_TYPE (value)));
+        }
+
+      return;
+    }
+
+  if (property->overridden_defaults != NULL)
     {
       GValue *default_value;
 
-      default_value = g_hash_table_lookup (property->default_values,
+      default_value = g_hash_table_lookup (property->overridden_defaults,
                                            g_type_name (gtype));
 
       if (default_value != NULL)
@@ -3619,7 +3709,7 @@ g_property_get_default (GProperty *property,
                         ...)
 {
   GValue value = { 0, };
-  GType p_type;
+  GType gtype, p_type;
   gchar *error;
   va_list var_args;
 
@@ -3627,6 +3717,8 @@ g_property_get_default (GProperty *property,
   g_return_if_fail (G_IS_OBJECT (gobject));
   g_return_if_fail (property->gtype != G_TYPE_INVALID);
 
+  gtype = G_OBJECT_TYPE (gobject);
+
   if (property->prerequisite != G_TYPE_INVALID)
     p_type = property->prerequisite;
   else
@@ -3634,15 +3726,20 @@ g_property_get_default (GProperty *property,
 
   g_value_init (&value, p_type);
 
-  if (property->default_values != NULL)
+  if (property->default_value != NULL)
+    {
+      g_value_copy (property->default_value, &value);
+      goto lcopy;
+    }
+
+  if (property->overridden_defaults != NULL)
     {
-      GType gtype = G_OBJECT_TYPE (gobject);
       GValue *default_value = NULL;
 
       /* we need to recurse through the inheritance chain... */
       while (gtype != G_TYPE_INVALID && default_value == NULL)
         {
-          default_value = g_hash_table_lookup (property->default_values,
+          default_value = g_hash_table_lookup (property->overridden_defaults,
                                                g_type_name (gtype));
 
           gtype = g_type_parent (gtype);
@@ -3659,7 +3756,7 @@ g_property_get_default (GProperty *property,
           ifaces = g_type_interfaces (gtype, &n_ifaces);
           while (n_ifaces-- && default_value == NULL)
             {
-              default_value = g_hash_table_lookup (property->default_values,
+              default_value = g_hash_table_lookup (property->overridden_defaults,
                                                    g_type_name (ifaces[n_ifaces]));
             }
 
@@ -3677,7 +3774,7 @@ g_property_get_default (GProperty *property,
         g_value_copy (default_value, &value);
     }
 
-
+lcopy:
   va_start (var_args, gobject);
 
   G_VALUE_LCOPY (&value, var_args, 0, &error);
@@ -5170,51 +5267,6 @@ g_property_unlock (GProperty *property,
   property_unlock_internal (property, gobject);
 }
 
-static void
-g_property_default_lock (GProperty *property,
-                         gpointer   gobject)
-{
-  gchar *lock_name = g_strconcat ("__g_property_lock_",
-                                  G_PARAM_SPEC (property)->name,
-                                  NULL);
-  gint *bit_lock_p;
-
-  bit_lock_p = g_object_get_data (gobject, lock_name);
-  if (bit_lock_p == NULL)
-    {
-      bit_lock_p = g_new0 (gint, 1);
-
-      g_object_set_data (gobject, lock_name, bit_lock_p);
-    }
-
-  g_free (lock_name);
-
-  g_bit_lock (bit_lock_p, 0);
-}
-
-static void
-g_property_default_unlock (GProperty *property,
-                           gpointer   gobject)
-{
-  gchar *lock_name = g_strconcat ("__g_property_lock_",
-                                  G_PARAM_SPEC (property)->name,
-                                  NULL);
-  gpointer bit_lock_p;
-
-  bit_lock_p = g_object_get_data (gobject, lock_name);
-  if (bit_lock_p == NULL)
-    {
-      g_free (lock_name);
-      return;
-    }
-
-  g_object_set_data (gobject, lock_name, NULL);
-  g_free (lock_name);
-
-  g_bit_unlock (bit_lock_p, 0);
-  g_free (bit_lock_p);
-}
-
 /**
  * g_property_set_lock_functions:
  * @property: a #GProperty
@@ -5237,13 +5289,7 @@ g_property_set_lock_functions (GProperty           *property,
   g_return_if_fail (!property->is_installed);
 
   if (lock_func == NULL)
-    {
-      g_return_if_fail (unlock_func == NULL);
-      lock_func = g_property_default_lock;
-    }
-
-  if (unlock_func == NULL)
-    unlock_func = g_property_default_unlock;
+    g_return_if_fail (unlock_func == NULL);
 
   property->lock_func = lock_func;
   property->unlock_func = unlock_func;
@@ -5255,8 +5301,14 @@ property_finalize (GParamSpec *pspec)
   GProperty *property = G_PROPERTY (pspec);
   GParamSpecClass *parent_class = g_type_class_peek (g_type_parent (G_TYPE_PROPERTY));
 
-  if (property->default_values != NULL)
-    g_hash_table_destroy (property->default_values);
+  if (property->default_value != NULL)
+    {
+      g_value_unset (property->default_value);
+      g_free (property->default_value);
+    }
+
+  if (property->overridden_defaults != NULL)
+    g_hash_table_destroy (property->overridden_defaults);
 
   parent_class->finalize (pspec);
 }
@@ -5320,8 +5372,8 @@ property_init (GParamSpec *pspec)
   property->field_offset = -1;
   property->priv_offset = -1;
 
-  property->lock_func = g_property_default_lock;
-  property->unlock_func = g_property_default_unlock;
+  property->lock_func = NULL;
+  property->unlock_func = NULL;
 }
 
 GType