From 0b083e3d8cfb572a4d863d327c753a6763540aa8 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Tue, 24 Sep 2024 15:03:14 -0700 Subject: [PATCH] glib/gvariant: Avoid many GBytes allocation Previously, all GVariants would allocate a GBytes for the buffered contents. This presents a challenge for small GVariant type created during the building process of GVariantBuilder as that results in an allocation for the GVariant, GBytes, and the byte buffer. Recent changes for GBytes may reduce those 3 allocations to 2, but even that is quite substantial overhead for a 32-bit integer. This changeset switches GVariant to use g_new/g_free allocators instead of g_slice_new/free. When benchmarked alone, this presented no measurable difference in overhead with the standard glibc allocator. With that change in place, allocations may then become variable in size to contain small allocations at the end of the GVariant reducing things to a single allocation (plus the GVariantTypeInfo reference). The size of GVariant is already 1 cacheline @ 64-bytes on x86_64. This uses that to guarantee our alignment of data maintains the 8-bytes guarantee of GVariant, and also extends it to match malloc(). On 32-bit systems, we are similarly aligned but reduce the amount we will inline to 32 bytes so we have a total of 1 cacheline. This is all asserted at compile-time to enforce the guarantee. In the end, this changeset reduces the wallclock time of building many GVariant in a loop using GVariantBuilder by 10% beyond the 10% already gained through GBytes doing the same thing. --- glib/gvariant-core.c | 93 ++++++++++++++++++++++++++++++++++++++------ glib/gvariant-core.h | 9 +++++ glib/gvariant.c | 5 ++- 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index ffe8d2a9c..a8de6689c 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -75,8 +75,15 @@ struct _GVariant gint state; gatomicrefcount ref_count; gsize depth; + + guint8 suffix[]; }; +/* Ensure our suffix data aligns to largest guaranteed offset + * within GVariant, of 8 bytes. + */ +G_STATIC_ASSERT (G_STRUCT_OFFSET (GVariant, suffix) % 8 == 0); + /* struct GVariant: * * There are two primary forms of GVariant instances: "serialized form" @@ -543,21 +550,31 @@ g_variant_ensure_serialised (GVariant *value) * @type: the type of the new instance * @serialised: if the instance will be in serialised form * @trusted: if the instance will be trusted + * @suffix_size: amount of extra bytes to add to allocation * * Allocates a #GVariant instance and does some common work (such as * looking up and filling in the type info), setting the state field, * and setting the ref_count to 1. * + * Use @suffix_size when you want to store data inside of the GVariant + * without having to add an additional GBytes allocation. + * * Returns: a new #GVariant with a floating reference */ static GVariant * g_variant_alloc (const GVariantType *type, gboolean serialised, - gboolean trusted) + gboolean trusted, + gsize suffix_size) { + G_GNUC_UNUSED gboolean size_check; GVariant *value; + gsize size; - value = g_slice_new (GVariant); + size_check = g_size_checked_add (&size, sizeof *value, suffix_size); + g_assert (size_check); + + value = g_malloc (size); value->type_info = g_variant_type_info_get (type); value->state = (serialised ? STATE_SERIALISED : 0) | (trusted ? STATE_TRUSTED : 0) | @@ -599,6 +616,54 @@ g_variant_new_from_bytes (const GVariantType *type, /* -- internal -- */ +/* < internal > + * g_variant_new_preallocated_trusted: + * @data: data to copy + * @size: the size of data + * + * Creates a new #GVariant for simple types such as int32, double, or + * bytes. + * + * Instead of allocating a GBytes, the data will be stored at the tail of + * the GVariant structures allocation. This can save considerable malloc + * overhead. + * + * The data is always aligned to the maximum alignment GVariant provides + * which is 8 bytes and therefore does not need to verify alignment based + * on the the @type provided. + * + * This should only be used for creating GVariant with trusted data. + * + * Returns: a new #GVariant with a floating reference + */ +GVariant * +g_variant_new_preallocated_trusted (const GVariantType *type, + gconstpointer data, + gsize size) +{ + GVariant *value; + gsize expected_size; + guint alignment; + + value = g_variant_alloc (type, TRUE, TRUE, size); + + g_variant_type_info_query (value->type_info, &alignment, &expected_size); + + g_assert (expected_size == 0 || size == expected_size); + + value->contents.serialised.ordered_offsets_up_to = G_MAXSIZE; + value->contents.serialised.checked_offsets_up_to = G_MAXSIZE; + value->contents.serialised.bytes = NULL; + value->contents.serialised.data = value->suffix; + value->size = size; + + memcpy (value->suffix, data, size); + + TRACE(GLIB_VARIANT_FROM_BUFFER(value, value->type_info, value->ref_count, value->state)); + + return value; +} + /* < internal > * g_variant_new_take_bytes: * @bytes: (transfer full): a #GBytes @@ -620,7 +685,7 @@ g_variant_new_take_bytes (const GVariantType *type, GBytes *owned_bytes = NULL; GVariantSerialised serialised; - value = g_variant_alloc (type, TRUE, trusted); + value = g_variant_alloc (type, TRUE, trusted, 0); g_variant_type_info_query (value->type_info, &alignment, &size); @@ -728,7 +793,7 @@ g_variant_new_from_children (const GVariantType *type, { GVariant *value; - value = g_variant_alloc (type, FALSE, trusted); + value = g_variant_alloc (type, FALSE, trusted, 0); value->contents.tree.children = children; value->contents.tree.n_children = n_children; TRACE(GLIB_VARIANT_FROM_CHILDREN(value, value->type_info, value->ref_count, value->state)); @@ -823,7 +888,7 @@ g_variant_unref (GVariant *value) g_variant_release_children (value); memset (value, 0, sizeof (GVariant)); - g_slice_free (GVariant, value); + g_free (value); } } @@ -1068,20 +1133,24 @@ g_variant_get_data (GVariant *value) * Returns: (transfer full): A new #GBytes representing the variant data * * Since: 2.36 - */ + */ GBytes * g_variant_get_data_as_bytes (GVariant *value) { const gchar *bytes_data; const gchar *data; - gsize bytes_size; + gsize bytes_size = 0; gsize size; g_variant_lock (value); g_variant_ensure_serialised (value); g_variant_unlock (value); - bytes_data = g_bytes_get_data (value->contents.serialised.bytes, &bytes_size); + if (value->contents.serialised.bytes != NULL) + bytes_data = g_bytes_get_data (value->contents.serialised.bytes, &bytes_size); + else + bytes_data = NULL; + data = value->contents.serialised.data; size = value->size; @@ -1091,11 +1160,13 @@ g_variant_get_data_as_bytes (GVariant *value) data = bytes_data; } - if (data == bytes_data && size == bytes_size) + if (bytes_data != NULL && data == bytes_data && size == bytes_size) return g_bytes_ref (value->contents.serialised.bytes); - else + else if (bytes_data != NULL) return g_bytes_new_from_bytes (value->contents.serialised.bytes, data - bytes_data, size); + else + return g_bytes_new (value->contents.serialised.data, size); } @@ -1226,7 +1297,7 @@ g_variant_get_child_value (GVariant *value, } /* create a new serialized instance out of it */ - child = g_slice_new (GVariant); + child = g_new (GVariant, 1); child->type_info = s_child.type_info; child->state = (value->state & STATE_TRUSTED) | STATE_SERIALISED; diff --git a/glib/gvariant-core.h b/glib/gvariant-core.h index 44388d83f..4d68b45a7 100644 --- a/glib/gvariant-core.h +++ b/glib/gvariant-core.h @@ -25,8 +25,17 @@ #include #include +#if GLIB_SIZEOF_VOID_P == 8 +# define G_VARIANT_MAX_PREALLOCATED 64 +#else +# define G_VARIANT_MAX_PREALLOCATED 32 +#endif + /* gvariant-core.c */ +GVariant * g_variant_new_preallocated_trusted (const GVariantType *type, + gconstpointer data, + gsize size); GVariant * g_variant_new_take_bytes (const GVariantType *type, GBytes *bytes, gboolean trusted); diff --git a/glib/gvariant.c b/glib/gvariant.c index a0832abee..51b609154 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -321,7 +321,10 @@ g_variant_new_from_trusted (const GVariantType *type, gconstpointer data, gsize size) { - return g_variant_new_take_bytes (type, g_bytes_new (data, size), TRUE); + if (size <= G_VARIANT_MAX_PREALLOCATED) + return g_variant_new_preallocated_trusted (type, data, size); + else + return g_variant_new_take_bytes (type, g_bytes_new (data, size), TRUE); } /**