Re: Instance private data

[Owen Taylor <otaylor redhat com>]

On Mon, 2003-02-17 at 15:50, Tim Janik wrote:
> On 14 Feb 2003, Owen Taylor wrote:
> 
> > On Thu, 2003-02-13 at 19:14, Tim Janik wrote:
> 
> > > /* using g_class as argument is more convenient and conveys class_init()
> > >  * time like g_type_class_peek_parent()
> > >  */
> > > void g_type_class_add_private (gpointer g_class,
> > >                                guint16  priv_size);
> > >
> > > /* return ((guint8*) instance) + instance_size + priv_base(priv_type), where
> > >  * priv_base for priv_type is the sum of priv_size from its ancestors
> > >  */
> > > gpointer g_type_instance_get_private (GTypeInstance *instance,
> > >                                       GType          priv_type);
> > >
> > > /* macro to be used the same way as G_TYPE_INSTANCE_GET_CLASS() */
> > > #define G_TYPE_INSTANCE_GET_PRIVATE(obj,type,ctype) ((ctype*) g_type_instance_get_private ((obj), type))
> > >
> > > results in example usage:
> > > gtk_window_class_init (GtkWindowClass *class)
> > > {
> > >   g_type_class_add_private (class, sizeof (GtkWindowPrivate));
> > > }
> > > #define GTK_WINDOW_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE (obj, GTK_TYPE_WINDOW, GtkWindowPrivate))
> 
> 
> > The main reason I didn't go with something like this initially was
> > efficiency. With cast checking turned off, my scheme basically comes
> > down to a couple of dereferences:
> >
> >  object->class->private_base + private_offset
> >
> > Only slightly more expensive than object->priv. While
> > g_type_instance_get_private() is going to be a lot more
> > expensive - it involves a function call, and several
> > invocations of lookup_type_node_I, each with a high probability
> > branch prediction miss. (*)
> >
> > Does it matter? Perhaps in the overall scheme of GObject
> > performance not. And where Mark's scheme can be used,
> > it doesn't matter at all.
> 
> i don't think branch prediction misses have any significant impact
> for this function. after all, most people will want to do
> GtkWindowPrivate *priv = GTK_WINDOW_GET_PRIVATE (obj);
> /* use priv */
> anyway. and even if not, g_type_instance_get_private() involves
> nothing more than two array dereferences and a couple additions,
> we don't even need a read lock here (see below).

And a function call and two possibly missed branch-predictions :-). 
They are a lot more expensive than the array dereferences - I did some
experiments and G_TYPE_IS_OBJECT (some_type) is approximately twice as
slow when the branch in type_node_lookup_I is mispredicted. 

(This is darn hard to benchmark because of smart branch prediction
in modern CPUs ... artificial benchmarks tend to have very predictable
branch patterns.)

> > But the potential order-of-magnitude performance difference
> > was my concern.
> 
> the overhead of g_type_instance_get_private() is negligible compared
> to what ordinary accessors would do.

I'm not sure what you mean by ordinary accessors. The overhead
of g_type_instance_get_private() will be roughly comparable
to cast check, so:

 int
 my_foo_get_int (MyFoo *foo)
 {
   g_return_val_if_fail (MY_IS_FOO (foo));

   return foo->some_int;
 }

Becomes approximately twice as slow when you do:

 int
 my_foo_get_int (MyFoo *foo)
 {
   MyFooPrivate *priv;

   g_return_val_if_fail (MY_IS_FOO (foo));

   priv = MY_FOO_GET_PRIVATE (foo);

   return priv->some_int;
 }

Still, I like the increased simplicity here a lot, so let's go 
with this approach.

> > Random Q: Why does g_type_class_peek_private() need a read lock?
> 
> assuming you meant g_type_class_peek_parent(), it should simply
> have a read lock because it's accessing concurrently used data
> (mechanic reasoning).
> 
> most of TypeData (TypeNode.data) remains constant as long as a
> class or instance (implies class) of a type exists. so in situations
> where existance of a class or instance can safely be assumed,
> we don't in practice need a read lock to access constant portions
> of TypeData.
> 
> for g_type_class_peek_parent(), parent->data->class.class is constant
> as long as the derived class exists, and for g_type_instance_get_private(),
> node->data->instance.priv_{size|base} will be constant as long as
> instances exist. so we can actually turn the current read locks
> into comments describing why they don't need to be acquired.

Yeah, that's what I thought. Filed as:

 http://bugzilla.gnome.org/show_bug.cgi?id=106433

Regards,
                                               Owen