From 766663dafea9695446574e3b2a7a47cb4e686aee Mon Sep 17 00:00:00 2001 From: Allison Karlitskaya Date: Fri, 21 Jul 2023 17:05:26 +0100 Subject: [PATCH] GPrivate: eliminate a malloc for pthread_key_t on most platforms We don't know how big a pthread_key_t is, so we malloc() a big enough chunk of memory for it and store a pointer into the GPrivate struct. It turns out, on Linux, pthread_key_t is just an int, so we could much easier just store it directly into the struct. https://bugzilla.gnome.org/show_bug.cgi?id=737445 Fixes: #931 --- glib/gthread-posix.c | 95 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 86 insertions(+), 9 deletions(-) diff --git a/glib/gthread-posix.c b/glib/gthread-posix.c index 30164432e..ad4c006fd 100644 --- a/glib/gthread-posix.c +++ b/glib/gthread-posix.c @@ -1047,22 +1047,99 @@ g_private_impl_free (pthread_key_t *key) free (key); } -static inline pthread_key_t -g_private_get_impl (GPrivate *key) +static gpointer +g_private_impl_new_direct (GDestroyNotify notify) { - pthread_key_t *impl = g_atomic_pointer_get (&key->p); + gpointer impl = (void *) (gssize) -1; + pthread_key_t key; + gint status; - if G_UNLIKELY (impl == NULL) + status = pthread_key_create (&key, notify); + if G_UNLIKELY (status != 0) + g_thread_abort (status, "pthread_key_create"); + + memcpy (&impl, &key, sizeof (pthread_key_t)); + + /* pthread_key_create could theoretically put a NULL value into key. + * If that happens, waste the result and create a new one, since we + * use NULL to mean "not yet allocated". + * + * This will only happen once per program run. + * + * We completely avoid this problem for the case where pthread_key_t + * is smaller than void* (for example, on 64 bit Linux) by putting + * some high bits in the value of 'impl' to start with. Since we only + * overwrite part of the pointer, we will never end up with NULL. + */ + if (sizeof (pthread_key_t) == sizeof (gpointer)) { - impl = g_private_impl_new (key->notify); - if (!g_atomic_pointer_compare_and_exchange (&key->p, NULL, impl)) + if G_UNLIKELY (impl == NULL) { - g_private_impl_free (impl); - impl = key->p; + status = pthread_key_create (&key, notify); + if G_UNLIKELY (status != 0) + g_thread_abort (status, "pthread_key_create"); + + memcpy (&impl, &key, sizeof (pthread_key_t)); + + if G_UNLIKELY (impl == NULL) + g_thread_abort (status, "pthread_key_create (gave NULL result twice)"); } } - return *impl; + return impl; +} + +static void +g_private_impl_free_direct (gpointer impl) +{ + pthread_key_t tmp; + gint status; + + memcpy (&tmp, &impl, sizeof (pthread_key_t)); + + status = pthread_key_delete (tmp); + if G_UNLIKELY (status != 0) + g_thread_abort (status, "pthread_key_delete"); +} + +static inline pthread_key_t +g_private_get_impl (GPrivate *key) +{ + if (sizeof (pthread_key_t) > sizeof (gpointer)) + { + pthread_key_t *impl = g_atomic_pointer_get (&key->p); + + if G_UNLIKELY (impl == NULL) + { + impl = g_private_impl_new (key->notify); + if (!g_atomic_pointer_compare_and_exchange (&key->p, NULL, impl)) + { + g_private_impl_free (impl); + impl = key->p; + } + } + + return *impl; + } + else + { + gpointer impl = g_atomic_pointer_get (&key->p); + pthread_key_t tmp; + + if G_UNLIKELY (impl == NULL) + { + impl = g_private_impl_new_direct (key->notify); + if (!g_atomic_pointer_compare_and_exchange (&key->p, NULL, impl)) + { + g_private_impl_free_direct (impl); + impl = key->p; + } + } + + memcpy (&tmp, &impl, sizeof (pthread_key_t)); + + return tmp; + } } /**