From 30dfead61b202e29d2069aa2c606e78f35c76b65 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Fri, 24 Jul 2026 15:51:57 +0200 Subject: [PATCH] gobject/gclosure: cache libffi plans for closures If we are using libffi to trampoline then there is opportunity to cache the ffi_call_plan introduced in newer libffi releases. A rather contrived test introduced in the previous commit includes a basic performance test that can compare between versions. For my system, the average call time dropped from 200ns/call to 145ns/call for cached GClosure. Since the previous version did no caching, the uncached GClosure use was also just slightly slower around 205ns/call. For the cached form (since it can reuse a global plan cache via GMutex) it drops to 180ns/call. So that is 27.5% savings and a 10% savings in runtime duration. Closes: #4002 Co-authored-by: Philip Withnall --- .gitlab-ci.yml | 10 +- .gitlab-ci/mingw.Dockerfile | 2 +- INSTALL.md | 3 +- docs/reference/glib/building.md | 3 + gobject/gclosure.c | 282 +++++++++++++++++++++++++++++++- gobject/gtype-private.h | 1 + subprojects/libffi.wrap | 19 ++- tools/glib.supp | 21 ++- 8 files changed, 317 insertions(+), 24 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c25796648..e79caee7a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -41,12 +41,12 @@ cache: - _ccache/ variables: - FEDORA_IMAGE: "registry.gitlab.gnome.org/gnome/glib/fedora:v41.5" + FEDORA_IMAGE: "registry.gitlab.gnome.org/gnome/glib/fedora:v41.6" COVERITY_IMAGE: "registry.gitlab.gnome.org/gnome/glib/coverity:v7" - DEBIAN_IMAGE: "registry.gitlab.gnome.org/gnome/glib/debian-stable:v25" - DEBIAN_I386_IMAGE: "registry.gitlab.gnome.org/gnome/glib/debian-stable-i386:v5" - ALPINE_IMAGE: "registry.gitlab.gnome.org/gnome/glib/alpine:v8" - MINGW_IMAGE: "registry.gitlab.gnome.org/gnome/glib/mingw:v41.3" + DEBIAN_IMAGE: "registry.gitlab.gnome.org/gnome/glib/debian-stable:v25.1" + DEBIAN_I386_IMAGE: "registry.gitlab.gnome.org/gnome/glib/debian-stable-i386:v5.1" + ALPINE_IMAGE: "registry.gitlab.gnome.org/gnome/glib/alpine:v8.1" + MINGW_IMAGE: "registry.gitlab.gnome.org/gnome/glib/mingw:v41.4" GOBJECT_INTROSPECTION_TAG: "1.80.1" MESON_TEST_TIMEOUT_MULTIPLIER: 4 G_MESSAGES_DEBUG: all diff --git a/.gitlab-ci/mingw.Dockerfile b/.gitlab-ci/mingw.Dockerfile index f1258d32c..64ed02a3e 100644 --- a/.gitlab-ci/mingw.Dockerfile +++ b/.gitlab-ci/mingw.Dockerfile @@ -1,4 +1,4 @@ -FROM registry.gitlab.gnome.org/gnome/glib/fedora:v41.3 +FROM registry.gitlab.gnome.org/gnome/glib/fedora:v41.6 USER root diff --git a/INSTALL.md b/INSTALL.md index 7d3b82821..980bd83e7 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -74,7 +74,8 @@ Some of the mimetype-related functionality in GIO requires the `update-mime-database` and `update-desktop-database` utilities, which are part of shared-mime-info and desktop-file-utils, respectively. -GObject uses libffi to implement generic marshalling functionality. +GObject uses libffi to implement generic marshalling functionality. With +libffi 3.7.0 or newer, GObject also caches call plans for generic marshalling. The Nitty-Gritty ================ diff --git a/docs/reference/glib/building.md b/docs/reference/glib/building.md index fce142ef0..55aadb65c 100644 --- a/docs/reference/glib/building.md +++ b/docs/reference/glib/building.md @@ -90,6 +90,9 @@ libraries. expression matching. The system version of PCRE is used, unless not available (which is the case on Android), in which case a fallback subproject is used. +- GObject uses libffi to implement generic marshalling functionality. With + libffi 3.7.0 or newer, GObject also caches call plans for generic marshalling. + - The optional extended attribute support in GIO requires the `getxattr()` family of functions that may be provided by the C library or by the standalone libattr library. To build GLib without extended attribute diff --git a/gobject/gclosure.c b/gobject/gclosure.c index c797a7c56..d65cb5e77 100644 --- a/gobject/gclosure.c +++ b/gobject/gclosure.c @@ -170,6 +170,230 @@ enum { POST_NOTIFY }; +#if defined(FFI_VERSION_NUMBER) && FFI_VERSION_NUMBER >= 30700 + +typedef struct +{ + ffi_cif cif; + ffi_call_plan *plan; + ffi_type **atypes; + ffi_type *rtype; + ffi_abi abi; + guint n_args; +} GClosureMarshalCallPlan; + +typedef struct +{ + GMutex lock; + GHashTable *plans; +} GClosureMarshalPlanCache; + +/* Like quarks, the cache and its entries live for the lifetime of the process. */ +static GClosureMarshalPlanCache marshal_plan_cache; + +static void +closure_marshal_call_plan_free (GClosureMarshalCallPlan *call_plan) +{ + g_clear_pointer (&call_plan->plan, ffi_call_plan_free); + g_clear_pointer (&call_plan->atypes, g_free); + g_free (call_plan); +} + +static GClosureMarshalCallPlan * +closure_marshal_call_plan_new (ffi_abi abi, + guint n_args, + ffi_type *rtype, + ffi_type **atypes) +{ + GClosureMarshalCallPlan *call_plan; + + g_assert (rtype != NULL); + g_assert (atypes != NULL); + + call_plan = g_new0 (GClosureMarshalCallPlan, 1); + call_plan->atypes = g_new0 (ffi_type *, n_args); + memcpy (call_plan->atypes, atypes, sizeof (*atypes) * n_args); + call_plan->rtype = rtype; + call_plan->abi = abi; + call_plan->n_args = n_args; + + if (ffi_prep_cif (&call_plan->cif, + call_plan->abi, + call_plan->n_args, + call_plan->rtype, + call_plan->atypes) != FFI_OK) + { + closure_marshal_call_plan_free (call_plan); + return NULL; + } + + if (!(call_plan->plan = ffi_call_plan_alloc (&call_plan->cif))) + { + closure_marshal_call_plan_free (call_plan); + return NULL; + } + + return call_plan; +} + +static gboolean +closure_marshal_call_plan_matches (const GClosureMarshalCallPlan *call_plan, + ffi_abi abi, + guint n_args, + ffi_type *rtype, + ffi_type **atypes) +{ + g_assert (call_plan != NULL); + g_assert (rtype != NULL); + g_assert (atypes != NULL); + + return call_plan->abi == abi && + call_plan->n_args == n_args && + call_plan->rtype == rtype && + memcmp (call_plan->atypes, atypes, sizeof (*atypes) * n_args) == 0; +} + +static guint +closure_marshal_call_plan_hash (gconstpointer data) +{ + const GClosureMarshalCallPlan *call_plan = data; + guint hash; + guint i; + + hash = (guint) call_plan->abi; + hash = (hash * 33) ^ call_plan->n_args; + hash = (hash * 33) ^ g_direct_hash (call_plan->rtype); + + for (i = 0; i < call_plan->n_args; i++) + hash = (hash * 33) ^ g_direct_hash (call_plan->atypes[i]); + + return hash; +} + +static gboolean +closure_marshal_call_plan_equal (gconstpointer data_a, + gconstpointer data_b) +{ + const GClosureMarshalCallPlan *call_plan_a = data_a; + const GClosureMarshalCallPlan *call_plan_b = data_b; + + return closure_marshal_call_plan_matches (call_plan_a, + call_plan_b->abi, + call_plan_b->n_args, + call_plan_b->rtype, + call_plan_b->atypes); +} + +static GClosureMarshalCallPlan * +closure_marshal_plan_cache_lookup_unlocked (GClosureMarshalPlanCache *cache, + ffi_abi abi, + guint n_args, + ffi_type *rtype, + ffi_type **atypes) +{ + GClosureMarshalCallPlan lookup = { 0, }; + + if (cache->plans == NULL) + return NULL; + + lookup.atypes = atypes; + lookup.rtype = rtype; + lookup.abi = abi; + lookup.n_args = n_args; + + return g_hash_table_lookup (cache->plans, &lookup); +} + +static GClosureMarshalCallPlan * +closure_marshal_plan_cache_get (ffi_abi abi, + guint n_args, + ffi_type *rtype, + ffi_type **atypes) +{ + GClosureMarshalCallPlan *call_plan; + GClosureMarshalCallPlan *candidate; + + g_mutex_lock (&marshal_plan_cache.lock); + call_plan = closure_marshal_plan_cache_lookup_unlocked (&marshal_plan_cache, + abi, + n_args, + rtype, + atypes); + g_mutex_unlock (&marshal_plan_cache.lock); + + if (call_plan != NULL) + return call_plan; + + if (!(candidate = closure_marshal_call_plan_new (abi, n_args, rtype, atypes))) + return NULL; + + g_mutex_lock (&marshal_plan_cache.lock); + + if (marshal_plan_cache.plans == NULL) + marshal_plan_cache.plans = + g_hash_table_new_full (closure_marshal_call_plan_hash, + closure_marshal_call_plan_equal, + (GDestroyNotify) closure_marshal_call_plan_free, + NULL); + + call_plan = closure_marshal_plan_cache_lookup_unlocked (&marshal_plan_cache, + abi, + n_args, + rtype, + atypes); + + if (call_plan == NULL) + { + call_plan = g_steal_pointer (&candidate); + g_hash_table_add (marshal_plan_cache.plans, call_plan); + } + + g_mutex_unlock (&marshal_plan_cache.lock); + + g_clear_pointer (&candidate, closure_marshal_call_plan_free); + + return call_plan; +} + +static gboolean +closure_marshal_call_plan_invoke (GClosure *closure, + ffi_abi abi, + guint n_args, + ffi_type *rtype, + ffi_type **atypes, + void (*callback) (void), + void *rvalue, + void **args) +{ + GRealClosure *real_closure; + GClosureMarshalCallPlan *call_plan; + + g_assert (closure != NULL); + g_assert (rtype != NULL); + g_assert (atypes != NULL); + g_assert (callback != NULL); + g_assert (args != NULL); + + real_closure = G_REAL_CLOSURE (closure); + call_plan = g_atomic_pointer_get (&real_closure->marshal_call_plan); + + if (call_plan == NULL || + !closure_marshal_call_plan_matches (call_plan, abi, n_args, rtype, atypes)) + { + if (!(call_plan = closure_marshal_plan_cache_get (abi, n_args, rtype, atypes))) + return FALSE; + + /* Concurrent invocations may cache different signatures; each uses its + * own local plan, and the next invocation checks the cached signature. */ + g_atomic_pointer_set (&real_closure->marshal_call_plan, call_plan); + } + + ffi_call_plan_invoke (call_plan->plan, callback, rvalue, args); + + return TRUE; +} + +#endif /* FFI_VERSION_NUMBER >= 30700 */ /* --- functions --- */ /** @@ -1696,10 +1920,30 @@ g_cclosure_marshal_generic (GClosure *closure, g_assert (n_args <= UINT_MAX); +#if defined(FFI_VERSION_NUMBER) && FFI_VERSION_NUMBER >= 30700 + if (!closure_marshal_call_plan_invoke (closure, + FFI_DEFAULT_ABI, + (guint) n_args, + rtype, + atypes, + FFI_FN (marshal_data ? marshal_data : cc->callback), + rvalue, + args)) + { + if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, (unsigned int) n_args, rtype, atypes) != FFI_OK) + return; + + ffi_call (&cif, + FFI_FN (marshal_data ? marshal_data : cc->callback), + rvalue, + args); + } +#else if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, (unsigned int) n_args, rtype, atypes) != FFI_OK) return; - ffi_call (&cif, marshal_data ? marshal_data : cc->callback, rvalue, args); + ffi_call (&cif, FFI_FN (marshal_data ? marshal_data : cc->callback), rvalue, args); +#endif if (return_gvalue && G_VALUE_TYPE (return_gvalue)) value_from_ffi_type (return_gvalue, rvalue); @@ -1748,6 +1992,7 @@ g_cclosure_marshal_generic_va (GClosure *closure, gint *enum_tmpval; gpointer *pointer_tmpval; gboolean tmpval_used = FALSE; + gboolean call_succeeded = FALSE; va_list args_copy; GValue *value_storage; @@ -1828,10 +2073,33 @@ g_cclosure_marshal_generic_va (GClosure *closure, g_assert (n_args <= UINT_MAX); - if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, (unsigned int) n_args, rtype, atypes) != FFI_OK) - return; - - ffi_call (&cif, marshal_data ? marshal_data : cc->callback, rvalue, args); +#if defined(FFI_VERSION_NUMBER) && FFI_VERSION_NUMBER >= 30700 + if (closure_marshal_call_plan_invoke (closure, + FFI_DEFAULT_ABI, + (guint) n_args, + rtype, + atypes, + FFI_FN (marshal_data ? marshal_data : cc->callback), + rvalue, + args)) + { + call_succeeded = TRUE; + } + else if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, (unsigned int) n_args, rtype, atypes) == FFI_OK) + { + ffi_call (&cif, + FFI_FN (marshal_data ? marshal_data : cc->callback), + rvalue, + args); + call_succeeded = TRUE; + } +#else + if (ffi_prep_cif (&cif, FFI_DEFAULT_ABI, (unsigned int) n_args, rtype, atypes) == FFI_OK) + { + ffi_call (&cif, FFI_FN (marshal_data ? marshal_data : cc->callback), rvalue, args); + call_succeeded = TRUE; + } +#endif /* Unbox non-primitive arguments */ for (i = 0; i < unsigned_n_params; i++) @@ -1855,7 +2123,7 @@ g_cclosure_marshal_generic_va (GClosure *closure, if (fundamental == G_TYPE_OBJECT && storage[i]._gpointer != NULL) g_object_unref (storage[i]._gpointer); } - - if (return_value && G_VALUE_TYPE (return_value)) + + if (call_succeeded && return_value && G_VALUE_TYPE (return_value)) value_from_ffi_type (return_value, rvalue); } diff --git a/gobject/gtype-private.h b/gobject/gtype-private.h index 53fb4af43..45b49d48b 100644 --- a/gobject/gtype-private.h +++ b/gobject/gtype-private.h @@ -58,6 +58,7 @@ struct _GRealClosure gpointer meta_marshal_data; GVaClosureMarshal va_meta_marshal; GVaClosureMarshal va_marshal; + gpointer marshal_call_plan; /* (atomic) */ GClosure closure; }; diff --git a/subprojects/libffi.wrap b/subprojects/libffi.wrap index 92976ba9d..f70cfb380 100644 --- a/subprojects/libffi.wrap +++ b/subprojects/libffi.wrap @@ -1,13 +1,14 @@ [wrap-file] -directory = libffi-3.5.2 -source_url = https://github.com/libffi/libffi/releases/download/v3.5.2/libffi-3.5.2.tar.gz -source_filename = libffi-3.5.2.tar.gz -source_hash = f3a3082a23b37c293a4fcd1053147b371f2ff91fa7ea1b2a52e335676bac82dc -patch_filename = libffi_3.5.2-1_patch.zip -patch_url = https://wrapdb.mesonbuild.com/v2/libffi_3.5.2-1/get_patch -patch_hash = 1ee3035d92e4df3541d0b2b8d192d2fa183b46e4a3d68c00d8f71ede354bee74 -source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/libffi_3.5.2-1/libffi-3.5.2.tar.gz -wrapdb_version = 3.5.2-1 +directory = libffi-3.7.1 +source_url = https://github.com/libffi/libffi/releases/download/v3.7.1/libffi-3.7.1.tar.gz +source_filename = libffi-3.7.1.tar.gz +source_hash = d5e9a6638ddbd2513ddb54518eb67e4bbe6fa707bcc01c10f6212f0a088d819d +source_fallback_url = https://wrapdb.mesonbuild.com/v2/libffi_3.7.1-1/get_source/libffi-3.7.1.tar.gz +patch_filename = libffi_3.7.1-1_patch.zip +patch_url = https://wrapdb.mesonbuild.com/v2/libffi_3.7.1-1/get_patch +patch_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/libffi_3.7.1-1/libffi_3.7.1-1_patch.zip +patch_hash = 7c5397bbfd026a5b9aac87e8aea58a6098329b4ee13243e3a9c8f49978418fac +wrapdb_version = 3.7.1-1 [provide] dependency_names = libffi diff --git a/tools/glib.supp b/tools/glib.supp index 7b4ae3c2a..9613b3c4e 100644 --- a/tools/glib.supp +++ b/tools/glib.supp @@ -84,6 +84,25 @@ fun:g_quark_init } +# Cached libffi call plans and their hash table live for the process lifetime. +{ + gobject-marshal-plan-cache-malloc + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:closure_marshal_plan_cache_get +} + +{ + gobject-marshal-plan-cache-calloc + Memcheck:Leak + match-leak-kinds:reachable + fun:calloc + ... + fun:closure_marshal_plan_cache_get +} + { gobject-init-malloc Memcheck:Leak @@ -1362,4 +1381,4 @@ fun:malloc ... fun:g_test_print_handler_full -} \ No newline at end of file +}