Re: rfc: alleviate GType write locking on ref/unref

On Thu, 25 May 2006, Jindrich Makovicka wrote:


we are currently working on a multithreaded application using GStreamer,
and one of the most annoying performance issues we have is the lock
contention of the GType write lock during creation and deletion of an
instance. As the GStreamer buffers are GType instances, and most of them
are only temporary, created at the beginning of the pipeline and
destroyed when arriving at the end, it means these rw locks are under a
heavy load.

The attached patch attempts to relieve this lock pressure by allowing
atomic modifications of class ref_count only with a read lock. I am
however not sure if it doesn't fatally break things. Can someone with a
better knowledge of GType/GObject internals comment on it?

well, at least this portion introduces breakage:

+      if (g_atomic_int_dec_and_test(&node->data->common.ref_count))
+        {
+          /* we decremented 1->0 ; revert the counter for last_unref */

you can't toggle between 0 and non-0 refcounting in multi threaded
scenarios. in fact, any refcounting code that "temporarily" drops a
reference it's assuming to still be the owener off won't work in multi
threaded scenrios. e.g. think of a second thread deciding to initialize
a new class due to your invalid reference count.

+          g_atomic_int_inc(&node->data->common.ref_count);

attempt to go 0 -> 1.

+          G_READ_UNLOCK (&type_rw_lock);

no lock, ANYTHING can happen here, this is a multithreaded scenrio.

+          G_WRITE_LOCK (&type_rw_lock);
+          if (g_atomic_int_get(&node->data->common.ref_count) == 1)

and here you assume the ref_count is still 1? good luck.

+            {
+              if (!node->plugin)
+                {
+                  g_warning ("static type `%s' unreferenced too often",
+                             NODE_NAME (node));

bailing out with a stale write lock? using g_error() instead would
be a gentler way to abort.

+                  return;
+                }

there are a few other issues with the patch, though mostly minor.
but before giving it a final go, i'd like to see proof that it really
improves things. can you come up with a test program that shows proovable performance gains
from application of this patch?
glib/tests/refcount/ and glib/tests/gobject/ contain numerous examples of
multithreaded ref counting tests and test object/class implementations to
get you started.

Best regards,
Jindrich Makovicka


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