GNOME.org
 

Re: Instance private data

Hi Owen,
	This is really nice - very useful indeed :-)

On Thu, 2002-12-26 at 11:05, Owen Taylor wrote:

<snip/>

> Notes:
> 
>  a) It's not clear to me whether the standard macro name should be
>     GTK_WINDOW_PRIVATE() or GTK_WINDOW_GET_PRIVATE(); the first
>     is shorter, but it might look like you could then cast
>     back to GTK_WINDOW().

	I'd prefer GET_PRIVATE() ...

>  b) The split where we have g_type_add_private() but 
>     G_OBJECT_GET_PRIVATE() is because gtype.c neds to be the party
>     allocating the private structures, but G_OBJECT_GET_PRIVATE()
>     is defined as:
> 
>     #define G_OBJECT_GET_PRIVATE(private_type, object, offset)      \
>      ((private_type *)(((guchar *) (object)) +                      \
>                        G_OBJECT_GET_CLASS (object)->private_base +  \
>                        (offset)))
> 
>     -- It uses a free slot in GObjectClass to store the size of the
>     instance. We didn't put any padding into GTypeClass. I don't
>     think this split is very confusing, so it doesn't bother me
>     much [and to get pedantic, you can even have classed but not 
>     instantiatable types, so it might not make sense in GTypeClass]
> 
>     Since the instance size is in the GTypeNode, another approach
>     would be to use a function call to retrieve this size.

	I think its fine the way you've done it.

>  c) A small downside to the approach I've taken here is that 
>     you get a static variable and thus a relocation for each
>     type. (We already use one for the signal array, one for
>     the type info, one for the type ID, so this isn't anything
>     new.) An alternate approach would be to store the offset
>     in the class structure for the individual type (e.g.
>     GtkWidgetClass). But would increase complexity a bit,
>     and use up some of the padding we reserved; so I don't
>     expect it is worthwhile.

	Object's that already have a seperately allocated private structure and
have a 'priv' pointer in their instance structures, for compatibility
reasons, will need to keep this 'priv' pointer and it can be used to
avoid this relocation for these types - we just need some way of
retrieving the private structure's offset in instance_init(). Bonobo and
Eel do this extensively, for example.

	I've attached a patch that implements g_type_get_private_offset(). The
only worry I can see about this is that it might encourage people to
start poking in the private structures - but I don't think we've had any
cases of people doing that with the 'priv' pointers ....

	Oh, the patch adds some tests of this to testgobject was well, which
may be useful even if you don't want get_private_offset().

>  d) As long as people are required to call g_type_add_private() in their
>     class_init(), it should work fine for dynamically loaded
>     types. My patch doesn't try to catch calling g_type_add_private()
>     at the wrong time, but I really don't imagine that people will
>     try to call it elsewhere.

	Hmm ... my initial foolish instinct was to do it in get_type() - so
maybe a check might be worthwhile ...

> Anyways - I think this addition makes a lot of sense -- the
> text above is longer than the code added to GObject, and it
> improves both efficiency and simplicity for storing private
> fields. I'd be interested in getting people's reactions.

	Yes, sounds sensible in the extreme to me :-)

Cheers,
Mark.

Index: gobject.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gobject.c,v
retrieving revision 1.53
diff -u -p -r1.53 gobject.c
--- gobject.c	5 Jul 2002 07:55:22 -0000	1.53
+++ gobject.c	1 Jan 2003 23:03:40 -0000
@@ -192,11 +192,15 @@ static void
 g_object_base_class_init (GObjectClass *class)
 {
   GObjectClass *pclass = g_type_class_peek_parent (class);
+  GTypeQuery query;
 
   /* reset instance specific fields and methods that don't get inherited */
   class->construct_properties = pclass ? g_slist_copy (pclass->construct_properties) : NULL;
   class->get_property = NULL;
   class->set_property = NULL;
+
+  g_type_query (G_TYPE_FROM_CLASS (class), &query);
+  class->private_base = query.instance_size;
 }
 
 static void
Index: gobject.h
===================================================================
RCS file: /cvs/gnome/glib/gobject/gobject.h,v
retrieving revision 1.26
diff -u -p -r1.26 gobject.h
--- gobject.h	21 Mar 2002 00:34:04 -0000	1.26
+++ gobject.h	1 Jan 2003 23:03:41 -0000
@@ -44,6 +44,10 @@ G_BEGIN_DECLS
 #define G_OBJECT_CLASS_NAME(class)  (g_type_name (G_OBJECT_CLASS_TYPE (class)))
 #define G_VALUE_HOLDS_OBJECT(value) (G_TYPE_CHECK_VALUE_TYPE ((value), G_TYPE_OBJECT))
 
+#define G_OBJECT_GET_PRIVATE(private_type, object, offset)	\
+ ((private_type *)(((guchar *) (object)) +			\
+	           G_OBJECT_GET_CLASS (object)->private_base +	\
+	           (offset)))
 
 /* --- typedefs & structures --- */
 typedef struct _GObject                  GObject;
@@ -98,8 +102,11 @@ struct  _GObjectClass
   /* signals */
   void	     (*notify)			(GObject	*object,
 					 GParamSpec	*pspec);
+
+  gsize      private_base;
+  
   /* padding */
-  gpointer	pdummy[8];
+  gpointer	pdummy[7];
 };
 struct _GObjectConstructParam
 {
Index: gtype.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/gtype.c,v
retrieving revision 1.54
diff -u -p -r1.54 gtype.c
--- gtype.c	22 Nov 2002 03:03:15 -0000	1.54
+++ gtype.c	1 Jan 2003 23:03:48 -0000
@@ -229,6 +229,7 @@ struct _InstanceData
   gconstpointer      class_data;
   gpointer           class;
   guint16            instance_size;
+  guint16            private_size;
   guint16            n_preallocs;
   GInstanceInitFunc  instance_init;
   GMemChunk        *mem_chunk;
@@ -909,6 +910,13 @@ type_data_make_W (TypeNode              
       data->instance.class_data = info->class_data;
       data->instance.class = NULL;
       data->instance.instance_size = info->instance_size;
+      if (NODE_PARENT_TYPE (node))
+	{
+	  TypeNode *pnode = lookup_type_node_I (NODE_PARENT_TYPE (node));
+	  data->instance.private_size = pnode->data->instance.private_size;
+	}
+      else
+	data->instance.private_size = 0;
 #ifdef	DISABLE_MEM_POOLS
       data->instance.n_preallocs = 0;
 #else	/* !DISABLE_MEM_POOLS */
@@ -1381,14 +1389,14 @@ g_type_create_instance (GType type)
       if (!node->data->instance.mem_chunk)
 	node->data->instance.mem_chunk = g_mem_chunk_new (NODE_NAME (node),
 							  node->data->instance.instance_size,
-							  (node->data->instance.instance_size *
+							  ((node->data->instance.instance_size + node->data->instance.private_size) *
 							   node->data->instance.n_preallocs),
 							  G_ALLOC_AND_FREE);
       instance = g_chunk_new0 (GTypeInstance, node->data->instance.mem_chunk);
       G_WRITE_UNLOCK (&type_rw_lock);
     }
   else
-    instance = g_malloc0 (node->data->instance.instance_size);	/* fine without read lock */
+    instance = g_malloc0 (node->data->instance.instance_size + node->data->instance.private_size);	/* fine without read lock */
   for (i = node->n_supers; i > 0; i--)
     {
       TypeNode *pnode;
@@ -1434,7 +1442,7 @@ g_type_free_instance (GTypeInstance *ins
   
   instance->g_class = NULL;
 #ifdef G_ENABLE_DEBUG  
-  memset (instance, 0xaa, node->data->instance.instance_size);	/* debugging hack */
+  memset (instance, 0xaa, node->data->instance.instance_size + node->data->instance.private_size);	/* debugging hack */
 #endif  
   if (node->data->instance.n_preallocs)
     {
@@ -3035,4 +3043,115 @@ void 
 g_type_init (void)
 {
   g_type_init_with_debug_flags (0);
+}
+
+/**
+ * g_type_add_private:
+ * @instance_type: an instantiatable type
+ * @private_size: size of private structure.
+ * 
+ * Registers a private structure for a instantiatable type;
+ * when an object is allocated, the private structures for
+ * the type and and all of its parent types are allocated
+ * sequentially in the same memory block as the public
+ * structures. This function should be called in the
+ * type's class_init() function. The return value can be
+ * used subsequently to retrieve the private structure.
+ * 
+ * Return value: the offset to add to the object location,
+ *   along with the size of the public instance, to get
+ *   the private structure. See the G_OBJECT_GET_PRIVATE()
+ *   macro for the normal way that this offset is used.
+ **/
+gsize
+g_type_add_private (GType instance_type,
+		    gsize private_size)
+{
+  TypeNode *node = lookup_type_node_I (instance_type);
+  gsize result;
+
+  if (!node || !node->is_instantiatable)
+    {
+      g_warning ("cannot add private field to non-instantiatable type '%s'",
+		 type_descriptive_name_I (instance_type));
+      return 0;
+    }
+
+  G_WRITE_LOCK (&type_rw_lock);
+
+  if (node->data && node->data->common.ref_count > 0)
+    {
+      result = node->data->instance.private_size;
+      
+      /* The 2*sizeof(size_t) alignment here is borrowed from
+       * GNU libc, so it should be good most everywhere.
+       * It is more conservative than is needed on some 64-bit
+       * platforms, but ia64 does require a 16-bit alignment.
+       * The SIMD extensions for x86 and ppc32 would want a
+       * larger alignment than this, but we don't need to
+       * do better than malloc.
+       */
+#define STRUCT_ALIGNMENT (2 * sizeof (gsize))
+      result = (result + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT;
+#undef STRUCT_ALIGNMENT      
+
+      node->data->instance.private_size = result + private_size;
+    }
+  else
+    {
+      g_warning ("g_type_add_private() called on unreferenced type '%s'\n"
+		 "should be called in class_init() function.",
+		 type_descriptive_name_I (instance_type));
+      result = 0;
+    }
+  
+  G_WRITE_UNLOCK (&type_rw_lock);
+
+  return result;
+}
+
+/**
+ * g_type_get_private_offset:
+ * @instance_type: an instantiatable type
+ * 
+ * Retrieves the offset of the type's private structure
+ * from the end of an instance of that type's structure.
+ * This is the same as the return value from
+ * g_type_add_private().
+ * 
+ * Return value: the offset to add to the object location,
+ *   along with the size of the public instance, to get
+ *   the private structure. See the G_OBJECT_GET_PRIVATE()
+ *   macro for the normal way that this offset is used.
+ **/
+gsize
+g_type_get_private_offset (GType instance_type)
+{
+  TypeNode *node = lookup_type_node_I (instance_type);
+  gsize result;
+
+  if (!node || !node->is_instantiatable)
+    {
+      g_warning ("cannot retrieve private offset for non-instantiatable type '%s'",
+		 type_descriptive_name_I (instance_type));
+      return 0;
+    }
+
+  node = lookup_type_node_I (NODE_PARENT_TYPE (node));
+
+  g_assert (node->data && node->data->common.ref_count);
+
+  G_READ_LOCK (&type_rw_lock);
+
+  result = node->data->instance.private_size;
+
+  G_READ_UNLOCK (&type_rw_lock);
+    
+  /* Keep in sync with g_type_add_private()
+   */ 
+#define STRUCT_ALIGNMENT (2 * sizeof (gsize))
+  result = (result + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT;
+#undef STRUCT_ALIGNMENT      
+
+  return result;
 }
Index: gtype.h
===================================================================
RCS file: /cvs/gnome/glib/gobject/gtype.h,v
retrieving revision 1.46
diff -u -p -r1.46 gtype.h
--- gtype.h	15 Oct 2002 21:16:18 -0000	1.46
+++ gtype.h	1 Jan 2003 23:03:50 -0000
@@ -297,6 +297,9 @@ void  g_type_interface_add_prerequisite 
 					 GType			     prerequisite_type);
 GType *g_type_interface_prerequisites   (GType                       interface_type,
 					 guint                       *n_prerequisites);
+gsize g_type_add_private                (GType                       instance_type,
+					 gsize                       private_size);
+gsize g_type_get_private_offset         (GType                       instance_type);
 
 
 /* --- protected (for fundamental type implementations) --- */
Index: testgobject.c
===================================================================
RCS file: /cvs/gnome/glib/gobject/testgobject.c,v
retrieving revision 1.6
diff -u -p -r1.6 testgobject.c
--- testgobject.c	12 Oct 2002 20:04:58 -0000	1.6
+++ testgobject.c	1 Jan 2003 23:03:50 -0000
@@ -126,8 +126,10 @@ iface_print_string (TestIface   *tiobj,
 #define TEST_IS_OBJECT(object)      (G_TYPE_CHECK_INSTANCE_TYPE ((object), TEST_TYPE_OBJECT))
 #define TEST_IS_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), TEST_TYPE_OBJECT))
 #define TEST_OBJECT_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), TEST_TYPE_OBJECT, TestObjectClass))
-typedef struct _TestObject      TestObject;
-typedef struct _TestObjectClass TestObjectClass;
+#define TEST_OBJECT_GET_PRIVATE(o)  (G_OBJECT_GET_PRIVATE (TestObjectPrivate, (o), test_object_private_offset))
+typedef struct _TestObject        TestObject;
+typedef struct _TestObjectClass   TestObjectClass;
+typedef struct _TestObjectPrivate TestObjectPrivate;
 struct _TestObject
 {
   GObject parent_instance;
@@ -140,6 +142,12 @@ struct _TestObjectClass
 			 TestIface  *iface_object,
 			 gpointer    tdata);
 };
+struct _TestObjectPrivate
+{
+  int     dummy1;
+  gdouble dummy2;
+};
+static gsize test_object_private_offset = 0;
 static void	test_object_class_init	(TestObjectClass	*class);
 static void	test_object_init	(TestObject		*tobject);
 static gboolean	test_signal_accumulator	(GSignalInvocationHint	*ihint,
@@ -190,10 +198,17 @@ test_object_class_init (TestObjectClass 
 		test_signal_accumulator, NULL,
 		g_cclosure_marshal_STRING__OBJECT_POINTER,
 		G_TYPE_STRING, 2, TEST_TYPE_IFACE, G_TYPE_POINTER);
+
+  test_object_private_offset = g_type_add_private (G_OBJECT_CLASS_TYPE (class), sizeof (TestObjectPrivate));
 }
 static void
 test_object_init (TestObject *tobject)
 {
+  TestObjectPrivate *priv;
+
+  priv = TEST_OBJECT_GET_PRIVATE (tobject);
+
+  g_assert (g_type_get_private_offset (G_OBJECT_TYPE (tobject)) == test_object_private_offset);
 }
 static gboolean
 test_signal_accumulator (GSignalInvocationHint *ihint,
@@ -274,8 +289,17 @@ derived_object_test_iface_init (gpointer
 #define DERIVED_IS_OBJECT(object)      (G_TYPE_CHECK_INSTANCE_TYPE ((object), DERIVED_TYPE_OBJECT))
 #define DERIVED_IS_OBJECT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), DERIVED_TYPE_OBJECT))
 #define DERIVED_OBJECT_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), DERIVED_TYPE_OBJECT, DerivedObjectClass))
-typedef struct _TestObject      DerivedObject;
-typedef struct _TestObjectClass DerivedObjectClass;
+#define DERIVED_OBJECT_GET_PRIVATE(o)  (G_OBJECT_GET_PRIVATE (DerivedObjectPrivate, (o), derived_object_private_offset))
+typedef struct _TestObject           DerivedObject;
+typedef struct _TestObjectClass      DerivedObjectClass;
+typedef struct _DerivedObjectPrivate DerivedObjectPrivate;
+struct _DerivedObjectPrivate
+{
+  char dummy;
+};
+static gsize derived_object_private_offset = 0;
+static void derived_object_class_init (DerivedObjectClass *class);
+static void derived_object_init       (DerivedObject      *dobject);
 GType
 derived_object_get_type (void)
 {
@@ -288,12 +312,12 @@ derived_object_get_type (void)
 	sizeof (DerivedObjectClass),
 	NULL,           /* base_init */
 	NULL,           /* base_finalize */
-	NULL,		/* class_init */
+	(GClassInitFunc) derived_object_class_init,
 	NULL,           /* class_finalize */
 	NULL,           /* class_data */
 	sizeof (DerivedObject),
 	5,              /* n_preallocs */
-	NULL,		/* instance_init */
+	(GInstanceInitFunc) derived_object_init,
       };
       GInterfaceInfo iface_info = { derived_object_test_iface_init, NULL, GUINT_TO_POINTER (87) };
 
@@ -303,7 +327,20 @@ derived_object_get_type (void)
 
   return derived_object_type;
 }
+static void
+derived_object_class_init (DerivedObjectClass *class)
+{
+  derived_object_private_offset = g_type_add_private (G_OBJECT_CLASS_TYPE (class), sizeof (DerivedObjectPrivate));
+}
+static void
+derived_object_init (DerivedObject *dobject)
+{
+  DerivedObjectPrivate *priv;
+
+  priv = DERIVED_OBJECT_GET_PRIVATE (dobject);
 
+  g_assert (g_type_get_private_offset (G_OBJECT_TYPE (dobject)) == derived_object_private_offset);
+}
 
 /* --- main --- */
 int
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]