GBytes: substantial internal rework

We have a wide variety of different sources of data for GBytes.

Instead of having all possibilities inside of a single structure type,
add a 'type' field and a couple of subtypes.

This also forces us to clean up our access to the ->data pointer from
all over the code which may become a problem in the future if we want to
lazy-map memfd GBytes instances by keeping the data pointer as NULL
until we are ready to use it.

We also introduce a new type of GBytes: 'inline'.  This allows us to
make a single allocation instead of two in the g_bytes_new() case.

glib/gbytes.c

@@ -66,13 +66,72 @@
 
 struct _GBytes
 {
-  gconstpointer data;  /* may be NULL iff (size == 0) */
+  gsize size;
-  gsize size;  /* may be 0 */
   gint  ref_count;
-  GDestroyNotify free_func;
+  gint  type;
-  gpointer user_data;
 };
 
+typedef struct
+{
+  GBytes bytes;
+#if GLIB_SIZEOF_SIZE_T == 4
+  guint pad;
+#endif
+
+  guchar data[1];
+} GBytesInline;
+
+/* important: the ->data field of GBytesInline should always be 'nicely
+ * aligned'.
+ */
+G_STATIC_ASSERT (G_STRUCT_OFFSET (GBytesInline, data) % (2 * sizeof (gpointer)) == 0);
+G_STATIC_ASSERT (G_STRUCT_OFFSET (GBytesInline, data) % 8 == 0);
+
+
+typedef struct
+{
+  GBytes   bytes;
+
+  gpointer data;
+} GBytesData;
+
+typedef struct
+{
+  GBytesData     data_bytes;
+
+  GDestroyNotify notify;
+  gpointer       user_data;
+} GBytesNotify;
+
+#define G_BYTES_TYPE_INLINE        (-1)
+#define G_BYTES_TYPE_STATIC        (-2)
+#define G_BYTES_TYPE_FREE          (-3)
+#define G_BYTES_TYPE_NOTIFY        (-4)
+
+/* All bytes are either inline or subtypes of GBytesData */
+#define G_BYTES_IS_INLINE(bytes)   ((bytes)->type == G_BYTES_TYPE_INLINE)
+#define G_BYTES_IS_DATA(bytes)     (!G_BYTES_IS_INLINE(bytes))
+
+/* More specific subtypes of GBytesData */
+#define G_BYTES_IS_STATIC(bytes)   ((bytes)->type == G_BYTES_TYPE_STATIC)
+#define G_BYTES_IS_FREE(bytes)     ((bytes)->type == G_BYTES_TYPE_FREE)
+#define G_BYTES_IS_NOTIFY(bytes)   ((bytes)->type == G_BYTES_TYPE_NOTIFY)
+
+static gpointer
+g_bytes_allocate (guint struct_size,
+                  guint type,
+                  gsize data_size)
+{
+  GBytes *bytes;
+
+  bytes = g_slice_alloc (struct_size);
+  bytes->size = data_size;
+  bytes->ref_count = 1;
+  bytes->type = type;
+
+  return bytes;
+}
+
 /**
  * g_bytes_new:
  * @data: (transfer none) (array length=size) (element-type guint8) (allow-none):
@@ -91,9 +150,14 @@ GBytes *
 g_bytes_new (gconstpointer data,
              gsize         size)
 {
+  GBytesInline *bytes;
+
   g_return_val_if_fail (data != NULL || size == 0, NULL);
 
-  return g_bytes_new_take (g_memdup (data, size), size);
+  bytes = g_bytes_allocate (G_STRUCT_OFFSET (GBytesInline, data[size]), G_BYTES_TYPE_INLINE, size);
+  memcpy (bytes->data, data, size);
+
+  return (GBytes *) bytes;
 }
 
 /**
@@ -123,9 +187,13 @@ GBytes *
 g_bytes_new_take (gpointer data,
                   gsize    size)
 {
-  return g_bytes_new_with_free_func (data, size, g_free, data);
+  GBytesData *bytes;
-}
 
+  bytes = g_bytes_allocate (sizeof (GBytesNotify), G_BYTES_TYPE_FREE, size);
+  bytes->data = data;
+
+  return (GBytes *) bytes;
+}
 
 /**
  * g_bytes_new_static: (skip)
@@ -146,7 +214,14 @@ GBytes *
 g_bytes_new_static (gconstpointer data,
                     gsize         size)
 {
-  return g_bytes_new_with_free_func (data, size, NULL, NULL);
+  GBytesData *bytes;
+
+  g_return_val_if_fail (data != NULL || size == 0, NULL);
+
+  bytes = g_bytes_allocate (sizeof (GBytesData), G_BYTES_TYPE_STATIC, size);
+  bytes->data = (gpointer) data;
+
+  return (GBytes *) bytes;
 }
 
 /**
@@ -176,16 +251,15 @@ g_bytes_new_with_free_func (gconstpointer  data,
                             GDestroyNotify free_func,
                             gpointer       user_data)
 {
-  GBytes *bytes;
+  GBytesNotify *bytes;
 
-  g_return_val_if_fail (data != NULL || size == 0, NULL);
+  if (!free_func)
+    return g_bytes_new_static (data, size);
 
-  bytes = g_slice_new (GBytes);
+  bytes = g_bytes_allocate (sizeof (GBytesNotify), G_BYTES_TYPE_NOTIFY, size);
-  bytes->data = data;
+  bytes->data_bytes.data = (gpointer) data;
-  bytes->size = size;
+  bytes->notify = free_func;
-  bytes->free_func = free_func;
   bytes->user_data = user_data;
-  bytes->ref_count = 1;
 
   return (GBytes *) bytes;
 }
@@ -216,7 +290,7 @@ g_bytes_new_from_bytes (GBytes  *bytes,
   g_return_val_if_fail (offset <= bytes->size, NULL);
   g_return_val_if_fail (offset + length <= bytes->size, NULL);
 
-  return g_bytes_new_with_free_func ((gchar *)bytes->data + offset, length,
+  return g_bytes_new_with_free_func ((gchar *) g_bytes_get_data (bytes, NULL) + offset, length,
                                      (GDestroyNotify)g_bytes_unref, g_bytes_ref (bytes));
 }
 
@@ -243,9 +317,24 @@ g_bytes_get_data (GBytes *bytes,
                   gsize  *size)
 {
   g_return_val_if_fail (bytes != NULL, NULL);
+
   if (size)
     *size = bytes->size;
-  return bytes->data;
+
+  if (G_BYTES_IS_DATA (bytes))
+    {
+      GBytesData *data_bytes = (GBytesData *) bytes;
+
+      return data_bytes->data;
+    }
+  else if (G_BYTES_IS_INLINE (bytes))
+    {
+      GBytesInline *inline_bytes = (GBytesInline *) bytes;
+
+      return inline_bytes->data;
+    }
+  else
+    g_assert_not_reached ();
 }
 
 /**
@@ -305,9 +394,42 @@ g_bytes_unref (GBytes *bytes)
 
   if (g_atomic_int_dec_and_test (&bytes->ref_count))
     {
-      if (bytes->free_func != NULL)
+      switch (bytes->type)
-        bytes->free_func (bytes->user_data);
+        {
-      g_slice_free (GBytes, bytes);
+        case G_BYTES_TYPE_STATIC:
+          /* data does not need to be freed */
+          g_slice_free (GBytesData, (GBytesData *) bytes);
+          break;
+
+        case G_BYTES_TYPE_INLINE:
+          /* data will be freed along with struct */
+          g_slice_free1 (G_STRUCT_OFFSET (GBytesInline, data[bytes->size]), bytes);
+          break;
+
+        case G_BYTES_TYPE_FREE:
+          {
+            GBytesData *data_bytes = (GBytesData *) bytes;
+
+            g_free (data_bytes->data);
+
+            g_slice_free (GBytesData, data_bytes);
+            break;
+          }
+
+        case G_BYTES_TYPE_NOTIFY:
+          {
+            GBytesNotify *notify_bytes = (GBytesNotify *) bytes;
+
+            /* We don't create GBytesNotify if callback was NULL */
+            (* notify_bytes->notify) (notify_bytes->user_data);
+
+            g_slice_free (GBytesNotify, notify_bytes);
+            break;
+          }
+
+        default:
+          g_assert_not_reached ();
+        }
     }
 }
 
@@ -330,14 +452,22 @@ gboolean
 g_bytes_equal (gconstpointer bytes1,
                gconstpointer bytes2)
 {
-  const GBytes *b1 = bytes1;
+  gconstpointer d1, d2;
-  const GBytes *b2 = bytes2;
+  gsize s1, s2;
 
   g_return_val_if_fail (bytes1 != NULL, FALSE);
   g_return_val_if_fail (bytes2 != NULL, FALSE);
 
-  return b1->size == b2->size &&
+  d1 = g_bytes_get_data ((GBytes *) bytes1, &s1);
-         memcmp (b1->data, b2->data, b1->size) == 0;
+  d2 = g_bytes_get_data ((GBytes *) bytes2, &s2);
+
+  if (s1 != s2)
+    return FALSE;
+
+  if (d1 == d2)
+    return TRUE;
+
+  return memcmp (d1, d2, s1) == 0;
 }
 
 /**
@@ -356,14 +486,18 @@ g_bytes_equal (gconstpointer bytes1,
 guint
 g_bytes_hash (gconstpointer bytes)
 {
-  const GBytes *a = bytes;
+  const guchar *data;
-  const signed char *p, *e;
+  const guchar *end;
+  gsize size;
   guint32 h = 5381;
 
   g_return_val_if_fail (bytes != NULL, 0);
 
-  for (p = (signed char *)a->data, e = (signed char *)a->data + a->size; p != e; p++)
+  data = g_bytes_get_data ((GBytes *) bytes, &size);
-    h = (h << 5) + h + *p;
+  end = data + size;
+
+  while (data != end)
+    h = (h << 5) + h + *(data++);
 
   return h;
 }
@@ -386,42 +520,23 @@ gint
 g_bytes_compare (gconstpointer bytes1,
                  gconstpointer bytes2)
 {
-  const GBytes *b1 = bytes1;
+  gconstpointer d1, d2;
-  const GBytes *b2 = bytes2;
+  gsize s1, s2;
   gint ret;
 
   g_return_val_if_fail (bytes1 != NULL, 0);
   g_return_val_if_fail (bytes2 != NULL, 0);
 
-  ret = memcmp (b1->data, b2->data, MIN (b1->size, b2->size));
+  d1 = g_bytes_get_data ((GBytes *) bytes1, &s1);
-  if (ret == 0 && b1->size != b2->size)
+  d2 = g_bytes_get_data ((GBytes *) bytes2, &s2);
-      ret = b1->size < b2->size ? -1 : 1;
+
+  ret = memcmp (d1, d2, MIN (s1, s2));
+  if (ret == 0 && s1 != s2)
+    ret = s1 < s2 ? -1 : 1;
+
   return ret;
 }
 
-static gpointer
-try_steal_and_unref (GBytes         *bytes,
-                     GDestroyNotify  free_func,
-                     gsize          *size)
-{
-  gpointer result;
-
-  if (bytes->free_func != free_func || bytes->data == NULL)
-    return NULL;
-
-  /* Are we the only reference? */
-  if (g_atomic_int_get (&bytes->ref_count) == 1)
-    {
-      *size = bytes->size;
-      result = (gpointer)bytes->data;
-      g_slice_free (GBytes, bytes);
-      return result;
-    }
-
-  return NULL;
-}
-
-
 /**
  * g_bytes_unref_to_data:
  * @bytes: (transfer full): a #GBytes
@@ -449,20 +564,26 @@ g_bytes_unref_to_data (GBytes *bytes,
   g_return_val_if_fail (bytes != NULL, NULL);
   g_return_val_if_fail (size != NULL, NULL);
 
+
   /*
    * Optimal path: if this is was the last reference, then we can return
    * the data from this GBytes without copying.
    */
-
+  if (G_BYTES_IS_FREE(bytes) && g_atomic_int_get (&bytes->ref_count) == 1)
-  result = try_steal_and_unref (bytes, g_free, size);
-  if (result == NULL)
     {
-      /*
+      GBytesData *data_bytes = (GBytesData *) bytes;
-       * Copy: Non g_malloc (or compatible) allocator, or static memory,
+
-       * so we have to copy, and then unref.
+      result = data_bytes->data;
-       */
-      result = g_memdup (bytes->data, bytes->size);
       *size = bytes->size;
+
+      g_slice_free (GBytesData, data_bytes);
+    }
+  else
+    {
+      gconstpointer data;
+
+      data = g_bytes_get_data (bytes, size);
+      result = g_memdup (data, *size);
       g_bytes_unref (bytes);
     }