Merging gobject-performance-2 branch



I've made a pass over the gobject-performance-2 branch, reviewing stuff
and generally getting a feel for it.

I commited some fixes for issues in my changes that I found during
review. For changes not by me I found the following:

The reorganization of g_type_class_ref makes this part useless:
  if (node->data->class.class) /* class was initialized during parent class initialization? */
    INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node));
as this check is now the first thing in this check:
  if (!node->data->class.class) /* class uninitialized */

It should be safe to just remove this check imho.


g_type_class_unref() and g_type_class_unref_uncached() check node->data
without a lock. This is volatile and not read/written atomically. It
should look at NODE_REFCOUNT (node) instead.



g_type_class_peek(), g_type_class_peek_static(),
g_type_default_interface_peek() and g_type_value_table_peek() all
check NODE_REFCOUNT and if > 0 assume they can dereference node->data.
But theoretically they can't as some other thread may unref the type
after the refcount check. 

However, the *_peek style of functions generally require some level of
guarantee that any refcount will have longer lifetime, as otherwise the
return value of the functions would risk being invalid by the time they
are returned.

So, dereferencing node->data here means we risk a NULL dereference where
we previously only risked returning invalid data. I think this is OK,
but we should have a comment about this.

In fact, there used to be a comment like this in
g_type_value_table_peek():
-  /* speed up common code path, we're not 100% safe here,
-   * but we should only get called with referenced types anyway
-   */
We should have something similar in all these functions.


Now, how do we get this branch merged? A whole lot of people have
reviewed this and everyone seems to think it looks good. The speedups
and scalability changes in this branch are very important for threaded
apps.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
       alexl redhat com            alexander larsson gmail com 
He's an impetuous skateboarding astronaut for the 21st century. She's an 
enchanted goth lawyer from a family of eight older brothers. They fight crime! 



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