gdataset: reorder epilogue code path in g_datalist_id_update_atomic()

The most common case is that the callback does not change the
data/destroy pointers. Reorder the "if" checks to favor those cases.

g_datalist_id_update_atomic() makes sense to track some data structure,
and atomically update it. But in that case, we usually update the values
inside the structure, and don't reset the pointer to the structure
itself. That means, if old data was present, then new_data is likley
unchanged.

With this change, we instead check whether the values are unchanged and
avoid resetting the (same) value. The idea is in the common case to
avoid writing the same value to memory to avoid false sharing or
invalidating CPU caches.
This commit is contained in:
Thomas Haller 2024-08-13 21:18:02 +02:00
parent a4a20a101b
commit a1e9284f47

View File

@ -1097,7 +1097,6 @@ g_datalist_id_update_atomic (GData **datalist,
gpointer result; gpointer result;
GDestroyNotify new_destroy; GDestroyNotify new_destroy;
guint32 idx; guint32 idx;
gboolean to_unlock = TRUE;
d = g_datalist_lock_and_get (datalist); d = g_datalist_lock_and_get (datalist);
@ -1116,49 +1115,59 @@ g_datalist_id_update_atomic (GData **datalist,
result = callback (&new_data, &new_destroy, user_data); result = callback (&new_data, &new_destroy, user_data);
if (data && !new_data) if (G_LIKELY (data))
{ {
GData *d_to_free; if (G_LIKELY (data->data == new_data && data->destroy == new_destroy))
/* Remove. The callback indicates to drop the entry.
*
* The old data->data was stolen by callback(). */
datalist_remove (d, idx);
if (datalist_shrink (&d, &d_to_free))
{ {
g_datalist_unlock_and_set (datalist, d); /* No change. */
if (G_UNLIKELY (d_to_free)) }
g_free (d_to_free); else if (!new_data)
to_unlock = FALSE; {
GData *d_to_free;
/* Remove. The callback indicates to drop the entry.
*
* The old data->data was stolen by callback(). */
datalist_remove (d, idx);
if (datalist_shrink (&d, &d_to_free))
{
g_datalist_unlock_and_set (datalist, d);
if (G_UNLIKELY (d_to_free))
g_free (d_to_free);
goto return_without_unlock;
}
}
else
{
/* Update. The callback may have provided new pointers to an existing
* entry.
*
* The old data was stolen by callback(). We only update the pointers and
* are done. */
data->data = new_data;
data->destroy = new_destroy;
} }
}
else if (data)
{
/* Update. The callback may have provided new pointers to an existing
* entry.
*
* The old data was stolen by callback(). We only update the pointers and
* are done. */
data->data = new_data;
data->destroy = new_destroy;
}
else if (!data && !new_data)
{
/* Absent. No change. The entry didn't exist and still does not. */
} }
else else
{ {
/* Add. Add a new entry that didn't exist previously. */ if (G_LIKELY (!new_data))
if (datalist_append (&d, key_id, new_data, new_destroy))
{ {
g_datalist_unlock_and_set (datalist, d); /* No change. The entry didn't exist and still does not. */
to_unlock = FALSE; }
else
{
/* Add. Add a new entry that didn't exist previously. */
if (datalist_append (&d, key_id, new_data, new_destroy))
{
g_datalist_unlock_and_set (datalist, d);
goto return_without_unlock;
}
} }
} }
if (to_unlock) g_datalist_unlock (datalist);
g_datalist_unlock (datalist);
return_without_unlock:
return result; return result;
} }