[glib/g-property: 6/8] property: Pack the GProperty structure



commit 0f2d3f2e041a9064c7ef297c415369b55680feb4
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Sat May 21 09:41:41 2011 +0100

    property: Pack the GProperty structure
    
    Using pahole to check for structure packing, size and holes we can make
    GProperty go (on 32bit archs) from this:
    
    struct _GProperty {
            GParamSpec                 parent_instance;      /*     0    40 */
            GPropertyFlags             flags;                /*    40     4 */
            GType                      gtype;                /*    44     4 */
            GType                      class_gtype;          /*    48     4 */
            guint16                    type_size;            /*    52     2 */
            guint16                    priv_offset;          /*    54     2 */
            guint16                    field_offset;         /*    56     2 */
    
            /* XXX 2 bytes hole, try to pack */
    
            GValue *                   default_value;        /*    60     4 */
            /* --- cacheline 1 boundary (64 bytes) --- */
            GHashTable *               overridden_defaults;  /*    64     4 */
            GType                      prerequisite;         /*    68     4 */
            GPropertyLockFunc          lock_func;            /*    72     4 */
            GPropertyUnlockFunc        unlock_func;          /*    76     4 */
            GQuark                     lock_quark;           /*    80     4 */
            guint                      is_installed:1;       /*    84:31  4 */
    
            /* size: 88, cachelines: 2, members: 14 */
            /* sum members: 86, holes: 1, sum holes: 2 */
            /* bit_padding: 31 bits */
            /* last cacheline: 24 bytes */
    };
    
    to this:
    
    struct _GProperty {
            GParamSpec                 parent_instance;      /*     0    40 */
            guint                      flags:15;             /*    40:17  4 */
            guint                      is_installed:1;       /*    40:16  4 */
    
            /* Bitfield combined with next fields */
    
            guint16                    type_size;            /*    42     2 */
            guint16                    priv_offset;          /*    44     2 */
            guint16                    field_offset;         /*    46     2 */
            GType                      gtype;                /*    48     4 */
            GType                      prerequisite;         /*    52     4 */
            GValue *                   default_value;        /*    56     4 */
            GQuark                     lock_quark;           /*    60     4 */
            /* --- cacheline 1 boundary (64 bytes) --- */
            GPropertyLockFunc          lock_func;            /*    64     4 */
            GPropertyUnlockFunc        unlock_func;          /*    68     4 */
    
            /* size: 72, cachelines: 2, members: 12 */
            /* last cacheline: 8 bytes */
    };
    
    The flags have been packed with the is_installed bit.
    
    The class_gtype field has been removed; it was used to retrieve the
    default of a GProperty through the GParamSpec API for backward
    compatibility, but we can do that by using a guard value and some common
    code.
    
    The overridden_defaults hash table has been removed by using the GData
    inside GParamSpec, using the GType's quark instead of its name as the
    key.
    
    There's still room for improvement by moving the default value to the
    GData as well, using a generalized lock_quark as its key; that would
    allow us to remove the default_value field and save another pointer.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=648526

 gobject/gproperty.c |  238 ++++++++++++++++++++-------------------------------
 1 files changed, 94 insertions(+), 144 deletions(-)
---
diff --git a/gobject/gproperty.c b/gobject/gproperty.c
index 79bb54d..5693771 100644
--- a/gobject/gproperty.c
+++ b/gobject/gproperty.c
@@ -436,28 +436,22 @@ struct _GProperty
 {
   GParamSpec parent_instance;
 
-  GPropertyFlags flags;
-
-  GType gtype;
-  GType class_gtype;
+  guint flags        : 15;
+  guint is_installed : 1;
 
   guint16 type_size;
   guint16 priv_offset;
   guint16 field_offset;
 
-  GValue *default_value;
-
-  GHashTable *overridden_defaults;
-
+  GType gtype;
   GType prerequisite;
 
-  GPropertyLockFunc lock_func;
-  GPropertyUnlockFunc unlock_func;
-
   /* used by the default locking */
-  GQuark lock_quark;
+  GValue *default_value;
 
-  guint is_installed : 1;
+  GQuark lock_quark;
+  GPropertyLockFunc lock_func;
+  GPropertyUnlockFunc unlock_func;
 };
 
 /* defines for the integer sub-types we don't have */
@@ -480,16 +474,6 @@ typedef struct { \
   GProperty##G_t##Get getter; \
 } G##G_t##Property; \
 \
-static void \
-property_##g_t##_set_default (GParamSpec *pspec, \
-                              GValue     *value) \
-{ \
-  GProperty *property = (GProperty *) pspec; \
-\
-  if (property->is_installed) \
-    g_property_get_default_value_for_type (property, property->class_gtype, value); \
-} \
-\
 static gint \
 property_##g_t##_values_cmp (GParamSpec   *pspec, \
                              const GValue *value_a, \
@@ -525,7 +509,6 @@ property_##g_t##_class_init (GParamSpecClass *klass) \
 { \
   klass->value_type = G_T; \
 \
-  klass->value_set_default = property_##g_t##_set_default; \
   klass->value_validate = property_##g_t##_validate; \
   klass->values_cmp = property_##g_t##_values_cmp; \
 } \
@@ -589,7 +572,6 @@ g_##g_t##_property_new (const gchar         *name,      \
   prop->flags = flags; \
   prop->gtype = G_T; \
   prop->prerequisite = G_TYPE_INVALID; \
-  prop->class_gtype = G_TYPE_INVALID; \
 \
   prop->field_offset = offset; \
 \
@@ -828,6 +810,47 @@ property_flags_to_param_flags (GPropertyFlags flags)
   return retval;
 }
 
+/* forward declaration */
+static void property_set_default (GParamSpec *pspec,
+                                  GValue     *value);
+
+static const GValue *
+property_get_default_for_type (GProperty *property,
+                               GType      gtype)
+{
+  if (gtype == G_TYPE_INVALID)
+    {
+      return property->default_value;
+    }
+  else
+    {
+      GParamSpec *pspec = (GParamSpec *) property;
+
+      return g_param_spec_get_qdata (pspec, g_type_qname (gtype));
+    }
+}
+
+static void
+value_unset_and_free (gpointer data)
+{
+  GValue *value = data;
+
+  g_value_unset (value);
+  g_free (value);
+}
+
+static void
+property_set_default_for_type (GProperty *property,
+                               GType      gtype,
+                               GValue    *value)
+{
+  GParamSpec *pspec = (GParamSpec *) property;
+
+  g_param_spec_set_qdata_full (pspec, g_type_qname (gtype),
+                               value,
+                               value_unset_and_free);
+}
+
 /**
  * g_boolean_property_new:
  * @name: canonical name of the property
@@ -1095,16 +1118,6 @@ typedef struct {
   GPropertyEnumGet getter;
 } GEnumProperty;
 
-static void
-property_enum_set_default (GParamSpec *pspec,
-                           GValue     *value)
-{
-  GProperty *property = G_PROPERTY (pspec);
-
-  if (property->is_installed)
-    g_property_get_default_value_for_type (property, property->class_gtype, value);
-}
-
 static gboolean
 property_enum_validate (GParamSpec *pspec,
                         GValue     *value)
@@ -1114,7 +1127,7 @@ property_enum_validate (GParamSpec *pspec,
 
   if (property->e_class == NULL ||
       g_enum_get_value (property->e_class, value->data[0].v_long) == NULL)
-    property_enum_set_default (pspec, value);
+    property_set_default (pspec, value);
 
   return value->data[0].v_long != oval;
 }
@@ -1140,7 +1153,6 @@ property_enum_class_init (GParamSpecClass *klass)
 {
   klass->value_type = G_TYPE_FLAGS;
 
-  klass->value_set_default = property_enum_set_default;
   klass->value_validate = property_enum_validate;
 
   klass->finalize = property_enum_finalize;
@@ -1216,7 +1228,6 @@ g_enum_property_new (const gchar      *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_ENUM;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -1356,16 +1367,6 @@ typedef struct {
   GPropertyFlagsGet getter;
 } GFlagsProperty;
 
-static void
-property_flags_set_default (GParamSpec *pspec,
-                            GValue     *value)
-{
-  GProperty *property = G_PROPERTY (pspec);
-
-  if (property->is_installed)
-    g_property_get_default_value_for_type (property, property->class_gtype, value);
-}
-
 static gboolean
 property_flags_validate (GParamSpec *pspec,
                          GValue     *value)
@@ -1376,7 +1377,7 @@ property_flags_validate (GParamSpec *pspec,
   if (property->f_class != NULL)
     value->data[0].v_ulong &= property->f_class->mask;
   else
-    property_flags_set_default (pspec, value);
+    property_set_default (pspec, value);
 
   return value->data[0].v_ulong != oval;
 }
@@ -1402,7 +1403,6 @@ property_flags_class_init (GParamSpecClass *klass)
 {
   klass->value_type = G_TYPE_FLAGS;
 
-  klass->value_set_default = property_flags_set_default;
   klass->value_validate = property_flags_validate;
 
   klass->finalize = property_flags_finalize;
@@ -1478,7 +1478,6 @@ g_flags_property_new (const gchar       *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_FLAGS;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -1625,16 +1624,6 @@ typedef struct {
   GPropertyFloatGet getter;
 } GFloatProperty;
 
-static void
-property_float_set_default (GParamSpec *pspec,
-                            GValue     *value)
-{
-  GProperty *property = G_PROPERTY (pspec);
-
-  if (property->is_installed)
-    g_property_get_default_value_for_type (property, property->class_gtype, value);
-}
-
 static gboolean
 property_float_validate (GParamSpec *pspec,
                          GValue     *value)
@@ -1667,7 +1656,6 @@ property_float_class_init (GParamSpecClass *klass)
 {
   klass->value_type = G_TYPE_FLOAT;
 
-  klass->value_set_default = property_float_set_default;
   klass->value_validate = property_float_validate;
   klass->values_cmp = property_float_values_cmp;
 }
@@ -1743,7 +1731,6 @@ g_float_property_new (const gchar       *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_FLOAT;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -1910,16 +1897,6 @@ typedef struct {
   GPropertyDoubleGet getter;
 } GDoubleProperty;
 
-static void
-property_double_set_default (GParamSpec *pspec,
-                             GValue     *value)
-{
-  GProperty *property = G_PROPERTY (pspec);
-
-  if (property->is_installed)
-    g_property_get_default_value_for_type (property, property->class_gtype, value);
-}
-
 static gboolean
 property_double_validate (GParamSpec *pspec,
                           GValue     *value)
@@ -1952,7 +1929,6 @@ property_double_class_init (GParamSpecClass *klass)
 {
   klass->value_type = G_TYPE_DOUBLE;
 
-  klass->value_set_default = property_double_set_default;
   klass->value_validate = property_double_validate;
   klass->values_cmp = property_double_values_cmp;
 }
@@ -2028,7 +2004,6 @@ g_double_property_new (const gchar        *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_DOUBLE;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -2260,7 +2235,6 @@ g_string_property_new (const gchar        *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_STRING;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -2471,7 +2445,6 @@ g_boxed_property_new (const gchar       *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_BOXED;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -2681,7 +2654,6 @@ g_object_property_new (const gchar        *name,
   prop->flags = flags;
   prop->gtype = G_TYPE_OBJECT;
   prop->prerequisite = G_TYPE_INVALID;
-  prop->class_gtype = G_TYPE_INVALID;
 
   prop->field_offset = offset;
 
@@ -2884,7 +2856,6 @@ _g_property_set_installed (GProperty *property,
       g_free (lock_n);
     }
 
-  property->class_gtype = class_gtype;
   property->is_installed = TRUE;
 }
 
@@ -3560,9 +3531,8 @@ g_property_set_default_value (GProperty    *property,
                               gpointer      gobject_class,
                               const GValue *default_value)
 {
-  const gchar *gtype_name;
   GValue *value;
-  GType gtype;
+  GType gtype, class_gtype;
 
   g_return_if_fail (G_IS_PROPERTY (property));
   g_return_if_fail (G_IS_OBJECT_CLASS (gobject_class));
@@ -3601,18 +3571,13 @@ g_property_set_default_value (GProperty    *property,
       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->overridden_defaults, gtype_name) != NULL)
+  class_gtype = G_TYPE_FROM_CLASS (gobject_class);
+  if (property_get_default_for_type (property, class_gtype) == NULL)
     {
       g_critical (G_STRLOC ": a default value of property '%s' for "
                   "type '%s' has already been defined.",
                   G_PARAM_SPEC (property)->name,
-                  gtype_name);
+                  g_type_name (class_gtype));
       return;
     }
 
@@ -3632,7 +3597,7 @@ g_property_set_default_value (GProperty    *property,
       return;
     }
 
-  g_hash_table_insert (property->overridden_defaults, (gpointer) gtype_name, value);
+  property_set_default_for_type (property, class_gtype, value);
 }
 
 /**
@@ -3654,6 +3619,8 @@ g_property_get_default_value_for_type (GProperty *property,
                                        GType      gtype,
                                        GValue    *value)
 {
+  const GValue *default_value;
+
   g_return_if_fail (G_IS_PROPERTY (property));
   g_return_if_fail (property->gtype != G_TYPE_INVALID);
   g_return_if_fail (g_type_name (gtype) != 0);
@@ -3672,23 +3639,16 @@ g_property_get_default_value_for_type (GProperty *property,
       return;
     }
 
-  if (property->overridden_defaults != NULL)
+  default_value = property_get_default_for_type (property, gtype);
+  if (default_value != NULL)
     {
-      GValue *default_value;
-
-      default_value = g_hash_table_lookup (property->overridden_defaults,
-                                           g_type_name (gtype));
-
-      if (default_value != NULL)
+      if (!g_value_transform (default_value, value))
         {
-          if (!g_value_transform (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)));
-            }
+          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)));
         }
     }
 }
@@ -3787,6 +3747,7 @@ g_property_get_default (GProperty *property,
   GType gtype, p_type;
   gchar *error;
   va_list var_args;
+  const GValue *default_value = NULL;
 
   g_return_if_fail (G_IS_PROPERTY (property));
   g_return_if_fail (G_IS_OBJECT (gobject));
@@ -3807,47 +3768,37 @@ g_property_get_default (GProperty *property,
       goto lcopy;
     }
 
-  if (property->overridden_defaults != NULL)
+  /* we need to recurse through the inheritance chain... */
+  while (gtype != G_TYPE_INVALID && default_value == NULL)
     {
-      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->overridden_defaults,
-                                               g_type_name (gtype));
-
-          gtype = g_type_parent (gtype);
-        }
+      default_value = property_get_default_for_type (property, gtype);
+      gtype = g_type_parent (gtype);
+    }
 
-      /* ... and eventually check the implemented interfaces */
-      if (default_value == NULL)
-        {
-          GType *ifaces;
-          guint n_ifaces;
+  /* ... and eventually check the implemented interfaces */
+  if (default_value == NULL)
+    {
+      GType *ifaces;
+      guint n_ifaces;
 
-          gtype = G_OBJECT_TYPE (gobject);
+      gtype = G_OBJECT_TYPE (gobject);
 
-          ifaces = g_type_interfaces (gtype, &n_ifaces);
-          while (n_ifaces-- && default_value == NULL)
-            {
-              default_value = g_hash_table_lookup (property->overridden_defaults,
-                                                   g_type_name (ifaces[n_ifaces]));
-            }
+      ifaces = g_type_interfaces (gtype, &n_ifaces);
+      while (n_ifaces-- && default_value == NULL)
+        default_value = property_get_default_for_type (property, ifaces[n_ifaces]);
 
-          g_free (ifaces);
-        }
+      g_free (ifaces);
+    }
 
-      if (default_value == NULL)
-        {
-          g_critical (G_STRLOC ": No default value of property '%s' "
-                      "was found for type '%s'",
-                      G_PARAM_SPEC (property)->name,
-                      G_OBJECT_TYPE_NAME (gobject));
-        }
-      else
-        g_value_copy (default_value, &value);
+  if (default_value == NULL)
+    {
+      g_critical (G_STRLOC ": No default value of property '%s' "
+                  "was found for type '%s'",
+                  G_PARAM_SPEC (property)->name,
+                  G_OBJECT_TYPE_NAME (gobject));
     }
+  else
+    g_value_copy (default_value, &value);
 
 lcopy:
   va_start (var_args, gobject);
@@ -5382,9 +5333,6 @@ property_finalize (GParamSpec *pspec)
       g_free (property->default_value);
     }
 
-  if (property->overridden_defaults != NULL)
-    g_hash_table_destroy (property->overridden_defaults);
-
   parent_class->finalize (pspec);
 }
 
@@ -5393,9 +5341,11 @@ property_set_default (GParamSpec *pspec,
                       GValue     *value)
 {
   GProperty *property = G_PROPERTY (pspec);
+  const GValue *default_value;
 
-  if (property->is_installed)
-    g_property_get_default_value_for_type (property, property->class_gtype, value);
+  default_value = property_get_default_for_type (property, G_TYPE_INVALID);
+  if (default_value != NULL)
+    g_value_copy (default_value, value);
 }
 
 static gboolean



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]