From a07a3351b315f8c43e29e64e3f95f17d635da3bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Jan 2024 10:28:26 +0100 Subject: [PATCH] gobject: drop OPTIONAL_BIT_LOCK_NOTIFY lock Now all accesses to quark_notify_queue are guarded by the GData lock. Several non-trivial operations are implemented via g_datalist_id_update_atomic(). The OPTIONAL_BIT_LOCK_NOTIFY lock is thus unnecessary and can be dropped. Note that with the move to g_datalist_id_update_atomic(), we now potentially do more work while holding the GData lock (e.g. some code paths allocation additional memory). But note that g_datalist_id_set_data() already has code paths where it must allocate memory to track the GDataElt. Also, most objects are not used in parallel, so holding the per-object (per-GData) lock longer does not affect them. Also, many operations also require a object_bit_lock(), so it seems very unlikely that you really could achieve higher parallelism by taking more locks (and minimizing the time to hold the GData lock). On the contrary, taking one lock less and doing all the work there is beneficial. --- gobject/gobject.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/gobject/gobject.c b/gobject/gobject.c index fe00f1078..15b667620 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -125,8 +125,7 @@ enum { * integers to GObjectPrivate. But increasing memory usage for more parallelism * (per-object!) is not worth it. */ #define OPTIONAL_BIT_LOCK_WEAK_REFS 1 -#define OPTIONAL_BIT_LOCK_NOTIFY 2 -#define OPTIONAL_BIT_LOCK_TOGGLE_REFS 3 +#define OPTIONAL_BIT_LOCK_TOGGLE_REFS 2 #if SIZEOF_INT == 4 && GLIB_SIZEOF_VOID_P >= 8 #define HAVE_OPTIONAL_FLAGS_IN_GOBJECT 1 @@ -713,15 +712,10 @@ g_object_notify_queue_freeze_cb (gpointer *data, static GObjectNotifyQueue * g_object_notify_queue_freeze (GObject *object) { - GObjectNotifyQueue *nqueue; - - object_bit_lock (object, OPTIONAL_BIT_LOCK_NOTIFY); - nqueue = _g_datalist_id_update_atomic (&object->qdata, - quark_notify_queue, - g_object_notify_queue_freeze_cb, - object); - object_bit_unlock (object, OPTIONAL_BIT_LOCK_NOTIFY); - return nqueue; + return _g_datalist_id_update_atomic (&object->qdata, + quark_notify_queue, + g_object_notify_queue_freeze_cb, + object); } static gpointer @@ -765,15 +759,11 @@ g_object_notify_queue_thaw (GObject *object, guint n_pspecs; GSList *slist; - object_bit_lock (object, OPTIONAL_BIT_LOCK_NOTIFY); - nqueue = _g_datalist_id_update_atomic (&object->qdata, quark_notify_queue, g_object_notify_queue_thaw_cb, ((gpointer[]){ object, nqueue })); - object_bit_unlock (object, OPTIONAL_BIT_LOCK_NOTIFY); - if (!nqueue) return; @@ -870,8 +860,6 @@ g_object_notify_queue_add (GObject *object, { gpointer result; - object_bit_lock (object, OPTIONAL_BIT_LOCK_NOTIFY); - result = _g_datalist_id_update_atomic (&object->qdata, quark_notify_queue, g_object_notify_queue_add_cb, @@ -882,8 +870,6 @@ g_object_notify_queue_add (GObject *object, .in_init = in_init, })); - object_bit_unlock (object, OPTIONAL_BIT_LOCK_NOTIFY); - return GPOINTER_TO_INT (result); }