From 357e611825470431ba26aac11a5020c4158e27dc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Aug 2024 21:18:02 +0200
Subject: [PATCH] gdataset: optimize return 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.
Also, avoid setting the data to the same value.
---
 glib/gdataset.c | 75 +++++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/glib/gdataset.c b/glib/gdataset.c
index 42864f3d0..eacebed32 100644
--- a/glib/gdataset.c
+++ b/glib/gdataset.c
@@ -1095,7 +1095,6 @@ g_datalist_id_update_atomic (GData **datalist,
   gpointer result;
   GDestroyNotify new_destroy;
   guint32 idx;
-  gboolean to_unlock = TRUE;
 
   if (G_UNLIKELY (already_locked))
     {
@@ -1121,49 +1120,59 @@ g_datalist_id_update_atomic (GData **datalist,
 
   result = callback (&new_data, &new_destroy, user_data);
 
-  if (data && !new_data)
+  if (data)
     {
-      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))
+      if (G_LIKELY (data->data == new_data && data->destroy == new_destroy))
         {
-          g_datalist_unlock_and_set (datalist, d);
-          if (d_to_free)
-            g_free (d_to_free);
-          to_unlock = FALSE;
+          /* No change. */
+        }
+      else if (!new_data)
+        {
+          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 (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
     {
-      /* Add. Add a new entry that didn't exist previously. */
-      if (datalist_append (&d, key_id, new_data, new_destroy))
+      if (G_LIKELY (!new_data))
         {
-          g_datalist_unlock_and_set (datalist, d);
-          to_unlock = FALSE;
+          /* No change. The entry didn't exist and still does not. */
+        }
+      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;
 }