GNOME.org
 

[glib/wip/linux: 3/6] Stop dithering over GPrivate

commit 975f31e490c84f909076017f7342fac25c92ef92
Author: Ryan Lortie <desrt desrt ca>
Date:   Fri Sep 30 14:22:04 2011 -0400

    Stop dithering over GPrivate
    
    Take out the half-private g_private_init() stuff and replace it with a
    G_PRIVATE_INIT macro that allows specifying a GDestroyNotify.

 glib/gmessages.c     |    7 -----
 glib/gmutex-posix.c  |   61 ++++++++++++++++++++++++++++++------------
 glib/gmutex.h        |    7 +++++
 glib/gmutexprivate.h |   23 ----------------
 glib/gslice.c        |    4 +--
 glib/gthread-win32.c |   72 +++++++++++++++++++++++++++++++------------------
 glib/gthread.c       |   10 +++----
 7 files changed, 101 insertions(+), 83 deletions(-)
---
diff --git a/glib/gmessages.c b/glib/gmessages.c
index f34bdb2..acd55c7 100644
--- a/glib/gmessages.c
+++ b/glib/gmessages.c
@@ -67,7 +67,6 @@
 #include "gprintfint.h"
 #include "gtestutils.h"
 #include "gthread.h"
-#include "gmutexprivate.h"
 #include "gstrfuncs.h"
 #include "gstring.h"
 
@@ -108,7 +107,6 @@ static GLogLevelFlags g_log_always_fatal = G_LOG_FATAL_MASK;
 static GPrintFunc     glib_print_func = NULL;
 static GPrintFunc     glib_printerr_func = NULL;
 static GPrivate       g_log_depth;
-static gboolean       g_log_depth_initialised;
 static GLogLevelFlags g_log_msg_prefix = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_DEBUG;
 static GLogFunc       default_log_func = g_log_default_handler;
 static gpointer       default_log_data = NULL;
@@ -527,11 +525,6 @@ g_logv (const gchar   *log_domain,
 
 	  /* check recursion and lookup handler */
 	  g_mutex_lock (&g_messages_lock);
-          if (!g_log_depth_initialised)
-            {
-              g_private_init (&g_log_depth, NULL);
-              g_log_depth_initialised = TRUE;
-            }
           depth = GPOINTER_TO_UINT (g_private_get (&g_log_depth));
 	  domain = g_log_find_domain_L (log_domain ? log_domain : "");
 	  if (depth)
diff --git a/glib/gmutex-posix.c b/glib/gmutex-posix.c
index b4bcd76..1ced38f 100644
--- a/glib/gmutex-posix.c
+++ b/glib/gmutex-posix.c
@@ -21,7 +21,7 @@
 
 #include "config.h"
 
-#include "gmutexprivate.h"
+#include "gmutex.h"
 
 #include "gatomic.h"
 
@@ -732,13 +732,49 @@ g_cond_timedwait (GCond  *cond,
 }
 
 /* {{{1 GPrivate */
+static pthread_key_t *
+g_private_impl_new (GDestroyNotify notify)
+{
+  pthread_key_t *key;
+  gint status;
 
-void
-g_private_init (GPrivate       *key,
-                GDestroyNotify  notify)
+  key = malloc (sizeof (pthread_key_t));
+  if G_UNLIKELY (key == NULL)
+    g_mutex_abort (errno, "malloc");
+  status = pthread_key_create (key, notify);
+  if G_UNLIKELY (status != 0)
+    g_mutex_abort (status, "pthread_key_create");
+
+  return key;
+}
+
+static void
+g_private_impl_free (pthread_key_t *key)
+{
+  gint status;
+
+  status = pthread_key_delete (*key);
+  if G_UNLIKELY (status != 0)
+    g_mutex_abort (status, "pthread_key_delete");
+  free (key);
+}
+
+static pthread_key_t *
+g_private_get_impl (GPrivate *key)
 {
-  pthread_key_create (&key->key, notify);
-  key->ready = TRUE;
+  pthread_key_t *impl = 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;
 }
 
 /**
@@ -763,11 +799,8 @@ g_private_init (GPrivate       *key,
 gpointer
 g_private_get (GPrivate *key)
 {
-  if (!key->ready)
-    return key->single_value;
-
   /* quote POSIX: No errors are returned from pthread_getspecific(). */
-  return pthread_getspecific (key->key);
+  return pthread_getspecific (*g_private_get_impl (key));
 }
 
 /**
@@ -787,13 +820,7 @@ g_private_set (GPrivate *key,
 {
   gint status;
 
-  if (!key->ready)
-    {
-      key->single_value = value;
-      return;
-    }
-
-  if G_UNLIKELY ((status = pthread_setspecific (key->key, value)) != 0)
+  if G_UNLIKELY ((status = pthread_setspecific (*g_private_get_impl (key), value)) != 0)
     g_mutex_abort (status, "pthread_setspecific");
 }
 /* {{{1 Epilogue */
diff --git a/glib/gmutex.h b/glib/gmutex.h
index 6e4a7dc..6d8232a 100644
--- a/glib/gmutex.h
+++ b/glib/gmutex.h
@@ -89,6 +89,13 @@ struct _GRecMutex
   gpointer impl;
 };
 
+#define G_PRIVATE_INIT(notify) { NULL, (notify) }
+struct _GPrivate
+{
+  gpointer       p;
+  GDestroyNotify notify;
+};
+
 void            g_mutex_init                    (GMutex         *mutex);
 void            g_mutex_clear                   (GMutex         *mutex);
 
diff --git a/glib/gslice.c b/glib/gslice.c
index 3d6de7e..11a297c 100644
--- a/glib/gslice.c
+++ b/glib/gslice.c
@@ -50,7 +50,6 @@
 #include "gutils.h"
 #include "gtestutils.h"
 #include "gthread.h"
-#include "gmutexprivate.h"
 #include "glib_trace.h"
 #include "glib-ctor.h"
 
@@ -203,7 +202,7 @@ static int      smc_notify_free   (void   *pointer,
                                    size_t  size);
 
 /* --- variables --- */
-static GPrivate    private_thread_memory;
+static GPrivate    private_thread_memory = G_PRIVATE_INIT (private_thread_memory_cleanup);
 static gsize       sys_page_size = 0;
 static Allocator   allocator[1] = { { 0, }, };
 static SliceConfig slice_config = {
@@ -360,7 +359,6 @@ GLIB_CTOR (g_slice_init_nomessage)
   /* at this point, g_mem_gc_friendly() should be initialized, this
    * should have been accomplished by the above g_malloc/g_new calls
    */
-  g_private_init (&private_thread_memory, private_thread_memory_cleanup);
 }
 
 static inline guint
diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c
index 2517c4b..cf8d587 100644
--- a/glib/gthread-win32.c
+++ b/glib/gthread-win32.c
@@ -366,48 +366,67 @@ struct _GPrivateDestructor
 };
 
 static GPrivateDestructor * volatile g_private_destructors;
+static CRITICAL_SECTION g_private_lock;
 
-void
-g_private_init (GPrivate       *key,
-                GDestroyNotify  notify)
+static DWORD
+g_private_get_impl (GPrivate *key)
 {
-  GPrivateDestructor *destructor;
+  DWORD impl = (DWORD) key->p;
 
-  key->index = TlsAlloc ();
+  if G_UNLIKELY (impl == 0)
+    {
+      EnterCriticalSection (&g_private_lock);
+      impl = (DWORD) key->p;
+      if (impl == 0)
+        {
+          GPrivateDestructor *destructor;
 
-  destructor = malloc (sizeof (GPrivateDestructor));
-  if G_UNLIKELY (destructor == NULL)
-    g_thread_abort (errno, "malloc");
-  destructor->index = key->index;
-  destructor->notify = notify;
+          impl = TlsAlloc ();
 
-  do
-    destructor->next = g_private_destructors;
-  while (InterlockedCompareExchangePointer (&g_private_destructors, destructor->next, destructor) != destructor->next);
+          if (impl == TLS_OUT_OF_INDEXES)
+            g_thread_abort (0, "TlsAlloc");
+
+          if (key->notify != NULL)
+            {
+              destructor = malloc (sizeof (GPrivateDestructor));
+              if G_UNLIKELY (destructor == NULL)
+                g_thread_abort (errno, "malloc");
+              destructor->index = impl;
+              destructor->notify = key->notify;
+              destructor->next = g_private_destructors;
+
+              /* We need to do an atomic store due to the unlocked
+               * access to the destructor list from the thread exit
+               * function.
+               *
+               * It can double as a sanity check...
+               */
+              if (InterlockedCompareExchangePointer (&g_private_destructors, destructor,
+                                                     destructor->next) != destructor->next)
+                g_thread_abort (0, "g_private_get_impl(1)");
+            }
+
+          /* Ditto, due to the unlocked access on the fast path */
+          if (InterlockedCompareExchangePointer (&key->p, impl, NULL) != NULL)
+            g_thread_abort (0, "g_private_get_impl(2)");
+        }
+      LeaveCriticalSection (&g_private_lock);
+    }
 
-  key->ready = TRUE;
+  return impl;
 }
 
 gpointer
 g_private_get (GPrivate *key)
 {
-  if (!key->ready)
-    return key->single_value;
-
-  return TlsGetValue (key->index);
+  return TlsGetValue (g_private_get_impl (key));
 }
 
 void
 g_private_set (GPrivate *key,
                gpointer  value)
 {
-  if (!key->ready)
-    {
-      key->single_value = value;
-      return;
-    }
-
-  TlsSetValue (key->index, value);
+  TlsSetValue (g_private_get_impl (key), value);
 }
 
 /* {{{1 GThread */
@@ -425,7 +444,6 @@ g_private_set (GPrivate *key,
 #define G_MUTEX_SIZE (sizeof (gpointer))
 
 static DWORD g_thread_self_tls;
-static DWORD g_private_tls;
 
 typedef BOOL (__stdcall *GTryEnterCriticalSectionFunc) (CRITICAL_SECTION *);
 
@@ -1051,7 +1069,7 @@ g_thread_DllMain (void)
     }
 
   win32_check_for_error (TLS_OUT_OF_INDEXES != (g_thread_self_tls = TlsAlloc ()));
-  win32_check_for_error (TLS_OUT_OF_INDEXES != (g_private_tls = TlsAlloc ()));
+  InitializeCriticalSection (&g_private_lock);
 }
 
 /* vim:set foldmethod=marker: */
diff --git a/glib/gthread.c b/glib/gthread.c
index 49b7f3a..6f51627 100644
--- a/glib/gthread.c
+++ b/glib/gthread.c
@@ -43,7 +43,6 @@
 #include "gthread.h"
 #include "gthreadprivate.h"
 #include "deprecated/gthread.h"
-#include "gmutexprivate.h"
 #include "gslice.h"
 #include "gmain.h"
 
@@ -659,7 +658,8 @@ GSystemThread    zero_thread; /* This is initialized to all zero */
 GMutex           g_once_mutex = G_MUTEX_INIT;
 
 static GCond     g_once_cond = G_COND_INIT;
-static GPrivate  g_thread_specific_private;
+static void g_thread_cleanup (gpointer data);
+static GPrivate  g_thread_specific_private = G_PRIVATE_INIT (g_thread_cleanup);
 static GRealThread *g_thread_all_threads = NULL;
 static GSList   *g_thread_free_indices = NULL;
 static GSList*   g_once_init_list = NULL;
@@ -694,8 +694,6 @@ G_LOCK_DEFINE_STATIC (g_thread);
  * having to link with the thread libraries.</para></note>
  */
 
-static void g_thread_cleanup (gpointer data);
-
 void
 g_thread_init_glib (void)
 {
@@ -714,7 +712,6 @@ g_thread_init_glib (void)
 
   /* setup the basic threading system */
   g_threads_got_initialized = TRUE;
-  g_private_init (&g_thread_specific_private, g_thread_cleanup);
   g_private_set (&g_thread_specific_private, main_thread);
   g_system_thread_self (&main_thread->system_thread);
 
@@ -1630,10 +1627,11 @@ g_cond_free (GCond *cond)
 GPrivate *
 g_private_new (GDestroyNotify notify)
 {
+  GPrivate tmp = G_PRIVATE_INIT (notify);
   GPrivate *key;
 
   key = g_slice_new (GPrivate);
-  g_private_init (key, notify);
+  *key = tmp;
 
   return key;
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]