gobject: rework GObjectNotifyQueue to not use GSList

GSList is almost in all use cases a bad choice. It's bad for locality
and requires a heap allocation per entry.

Instead, use an array, and grow the buffer exponentially via realloc().

Now, that we use g_datalist_id_update_atomic(), it is also easy to
update the pointer. Hence, the GObjectNotifyQueue struct does not point
to an array of pspecs. Instead the entire GObjectNotifyQueue itself gets
reallocated, thus saving one heap allocation for the separate head
structure.
This commit is contained in:
Thomas Haller 2024-01-29 10:55:41 +01:00
parent 2ff1ca9f68
commit 0b3c3089f6

View File

@ -213,14 +213,14 @@ static void object_interface_check_properties (gpointer check_d
gpointer g_iface); gpointer g_iface);
/* --- typedefs --- */ /* --- typedefs --- */
typedef struct _GObjectNotifyQueue GObjectNotifyQueue;
struct _GObjectNotifyQueue typedef struct
{ {
GSList *pspecs;
guint16 n_pspecs;
guint16 freeze_count; guint16 freeze_count;
}; guint16 len;
guint16 alloc;
GParamSpec *pspecs[];
} GObjectNotifyQueue;
/* --- variables --- */ /* --- variables --- */
static GQuark quark_closure_array = 0; static GQuark quark_closure_array = 0;
@ -656,13 +656,11 @@ object_bit_unlock (GObject *object, guint lock_bit)
} }
/* --- functions --- */ /* --- functions --- */
static void
g_object_notify_queue_free (gpointer data)
{
GObjectNotifyQueue *nqueue = data;
g_slist_free (nqueue->pspecs); G_ALWAYS_INLINE static inline gsize
g_free_sized (nqueue, sizeof (GObjectNotifyQueue)); g_object_notify_queue_alloc_size (gsize alloc)
{
return G_STRUCT_OFFSET (GObjectNotifyQueue, pspecs) + (alloc * sizeof (GParamSpec *));
} }
static GObjectNotifyQueue * static GObjectNotifyQueue *
@ -670,10 +668,11 @@ g_object_notify_queue_new_frozen (void)
{ {
GObjectNotifyQueue *nqueue; GObjectNotifyQueue *nqueue;
nqueue = g_new (GObjectNotifyQueue, 1); nqueue = g_malloc (g_object_notify_queue_alloc_size (4));
*nqueue = (GObjectNotifyQueue){
.freeze_count = 1, nqueue->freeze_count = 1;
}; nqueue->alloc = 4;
nqueue->len = 0;
return nqueue; return nqueue;
} }
@ -691,11 +690,11 @@ g_object_notify_queue_freeze_cb (gpointer *data,
/* The nqueue doesn't exist yet. We create it, and freeze thus 1 time. */ /* The nqueue doesn't exist yet. We create it, and freeze thus 1 time. */
nqueue = g_object_notify_queue_new_frozen (); nqueue = g_object_notify_queue_new_frozen ();
*data = nqueue; *data = nqueue;
*destroy_notify = g_object_notify_queue_free; *destroy_notify = g_free;
} }
else else
{ {
if (nqueue->freeze_count >= 65535) if (nqueue->freeze_count == G_MAXUINT16)
{ {
g_critical ("Free queue for %s (%p) is larger than 65535," g_critical ("Free queue for %s (%p) is larger than 65535,"
" called g_object_freeze_notify() too often." " called g_object_freeze_notify() too often."
@ -723,15 +722,9 @@ g_object_notify_queue_thaw_cb (gpointer *data,
GDestroyNotify *destroy_notify, GDestroyNotify *destroy_notify,
gpointer user_data) gpointer user_data)
{ {
GObject *object = ((gpointer *) user_data)[0]; GObject *object = user_data;
GObjectNotifyQueue *nqueue0 = ((gpointer *) user_data)[1];
GObjectNotifyQueue *nqueue = *data; GObjectNotifyQueue *nqueue = *data;
#if G_ENABLE_DEBUG
g_assert (!nqueue0 || nqueue0 == nqueue);
#endif
(void) nqueue0;
if (G_UNLIKELY (!nqueue || nqueue->freeze_count == 0)) if (G_UNLIKELY (!nqueue || nqueue->freeze_count == 0))
{ {
g_critical ("%s: property-changed notification for %s(%p) is not frozen", g_critical ("%s: property-changed notification for %s(%p) is not frozen",
@ -753,74 +746,54 @@ g_object_notify_queue_thaw (GObject *object,
GObjectNotifyQueue *nqueue, GObjectNotifyQueue *nqueue,
gboolean take_ref) gboolean take_ref)
{ {
GParamSpec *pspecs_stack[16];
GParamSpec **pspecs_heap = NULL;
GParamSpec **pspecs = pspecs_stack;
guint n_pspecs;
GSList *slist;
nqueue = _g_datalist_id_update_atomic (&object->qdata, nqueue = _g_datalist_id_update_atomic (&object->qdata,
quark_notify_queue, quark_notify_queue,
g_object_notify_queue_thaw_cb, g_object_notify_queue_thaw_cb,
((gpointer[]){ object, nqueue })); object);
if (!nqueue) if (!nqueue)
return; return;
if (nqueue->n_pspecs == 0) if (nqueue->len > 0)
goto out;
if (nqueue->n_pspecs > G_N_ELEMENTS (pspecs_stack))
{ {
pspecs_heap = g_new (GParamSpec *, nqueue->n_pspecs); guint16 i;
pspecs = pspecs_heap; guint16 j;
}
n_pspecs = 0; /* Reverse the list. This is the order that we historically had. */
for (slist = nqueue->pspecs; slist; slist = slist->next) for (i = 0, j = nqueue->len - 1u; i < j; i++, j--)
pspecs[n_pspecs++] = slist->data; {
#if G_ENABLE_DEBUG GParamSpec *tmp;
g_assert (n_pspecs == nqueue->n_pspecs);
#endif tmp = nqueue->pspecs[i];
nqueue->pspecs[i] = nqueue->pspecs[j];
nqueue->pspecs[j] = tmp;
}
if (take_ref) if (take_ref)
g_object_ref (object); g_object_ref (object);
G_OBJECT_GET_CLASS (object)->dispatch_properties_changed (object, n_pspecs, pspecs); G_OBJECT_GET_CLASS (object)->dispatch_properties_changed (object, nqueue->len, nqueue->pspecs);
if (take_ref) if (take_ref)
g_object_unref (object); g_object_unref (object);
if (pspecs_heap)
g_free (pspecs_heap);
out:
g_object_notify_queue_free (nqueue);
} }
typedef struct g_free (nqueue);
{ }
GObject *object;
GObjectNotifyQueue *nqueue;
GParamSpec *pspec;
gboolean in_init;
} NotifyQueueAddData;
static gpointer static gpointer
g_object_notify_queue_add_cb (gpointer *data, g_object_notify_queue_add_cb (gpointer *data,
GDestroyNotify *destroy_notify, GDestroyNotify *destroy_notify,
gpointer user_data) gpointer user_data)
{ {
NotifyQueueAddData *nqdata = user_data; GParamSpec *pspec = ((gpointer *) user_data)[0];
gboolean in_init = GPOINTER_TO_INT (((gpointer *) user_data)[1]);
GObjectNotifyQueue *nqueue = *data; GObjectNotifyQueue *nqueue = *data;
guint16 i;
#if G_ENABLE_DEBUG
g_assert (!nqdata->nqueue || nqdata->nqueue == nqueue);
#endif
if (!nqueue) if (!nqueue)
{ {
if (!nqdata->in_init) if (!in_init)
{ {
/* We are not in-init and are currently not frozen. There is nothing /* We are not in-init and are currently not frozen. There is nothing
* to do. We return FALSE to the caller, which then will dispatch * to do. We return FALSE to the caller, which then will dispatch
@ -838,17 +811,36 @@ g_object_notify_queue_add_cb (gpointer *data,
* freeze an additional time. */ * freeze an additional time. */
nqueue = g_object_notify_queue_new_frozen (); nqueue = g_object_notify_queue_new_frozen ();
*data = nqueue; *data = nqueue;
*destroy_notify = g_object_notify_queue_free; *destroy_notify = g_free;
} }
else
g_assert (nqueue->n_pspecs < 65535);
if (!g_slist_find (nqueue->pspecs, nqdata->pspec))
{ {
nqueue->pspecs = g_slist_prepend (nqueue->pspecs, nqdata->pspec); for (i = 0; i < nqueue->len; i++)
nqueue->n_pspecs++; {
if (nqueue->pspecs[i] == pspec)
goto out;
} }
if (G_UNLIKELY (nqueue->len == nqueue->alloc))
{
guint32 alloc;
alloc = ((guint32) nqueue->alloc) * 2u;
if (alloc >= G_MAXUINT16)
{
g_assert (nqueue->len < G_MAXUINT16);
alloc = G_MAXUINT16;
}
nqueue = g_realloc (nqueue, g_object_notify_queue_alloc_size (alloc));
nqueue->alloc = alloc;
*data = nqueue;
}
}
nqueue->pspecs[nqueue->len++] = pspec;
out:
return GINT_TO_POINTER (TRUE); return GINT_TO_POINTER (TRUE);
} }
@ -863,12 +855,7 @@ g_object_notify_queue_add (GObject *object,
result = _g_datalist_id_update_atomic (&object->qdata, result = _g_datalist_id_update_atomic (&object->qdata,
quark_notify_queue, quark_notify_queue,
g_object_notify_queue_add_cb, g_object_notify_queue_add_cb,
&((NotifyQueueAddData){ ((gpointer[]){ pspec, GINT_TO_POINTER (!!in_init) }));
.object = object,
.nqueue = nqueue,
.pspec = pspec,
.in_init = in_init,
}));
return GPOINTER_TO_INT (result); return GPOINTER_TO_INT (result);
} }