honour g_mem_gc_friendly settings when freeing slices, make sure

Wed Jan 25 16:39:18 2006  Tim Janik  <timj@imendio.com>

        * glib/gslice.c: honour g_mem_gc_friendly settings when freeing
        slices, make sure g_mem_gc_friendly is properly initialized.

        * gmem.[hc]: ensure g_mem_gc_friendly is initialized from G_DEBUG upon
        the first allocation. applied some branching optimizations.

        * docs/macros.txt: reflected --enable-gc-friendly change and
        described ENABLE_GC_FRIENDLY_DEFAULT as well as G_DEBUG=gc-friendly.

        * configure.in: changed --enable-gc-friendly=yes to define
        ENABLE_GC_FRIENDLY_DEFAULT.

        * glib/garray.c: changed ENABLE_GC_FRIENDLY macro #ifdef-s to
        if (G_UNLIKELY (g_mem_gc_friendly)).

        * glib/gtree.c:
        * glib/ghash.c: removed ENABLE_GC_FRIENDLY code which is now taken
        care of by g_slice_free1().

        * tests/slice-test.c: fixed leaks, reported by Kjartan Maraas.

ChangeLog

@@ -1,3 +1,26 @@
+Wed Jan 25 16:39:18 2006  Tim Janik  <timj@imendio.com>
+
+	* glib/gslice.c: honour g_mem_gc_friendly settings when freeing
+	slices, make sure g_mem_gc_friendly is properly initialized.
+
+	* gmem.[hc]: ensure g_mem_gc_friendly is initialized from G_DEBUG upon
+	the first allocation. applied some branching optimizations.
+
+	* docs/macros.txt: reflected --enable-gc-friendly change and
+	described ENABLE_GC_FRIENDLY_DEFAULT as well as G_DEBUG=gc-friendly.
+
+	* configure.in: changed --enable-gc-friendly=yes to define
+	ENABLE_GC_FRIENDLY_DEFAULT.
+
+	* glib/garray.c: changed ENABLE_GC_FRIENDLY macro #ifdef-s to
+	if (G_UNLIKELY (g_mem_gc_friendly)).
+
+	* glib/gtree.c:
+	* glib/ghash.c: removed ENABLE_GC_FRIENDLY code which is now taken
+	care of by g_slice_free1().
+
+	* tests/slice-test.c: fixed leaks, reported by Kjartan Maraas.
+
 Tue Jan 24 17:49:36 2006  Tim Janik  <timj@imendio.com>
 
 	* glib/gslice.c: only use posix_memalign() if it's known to work,

ChangeLog.pre-2-10

@@ -1,3 +1,26 @@
+Wed Jan 25 16:39:18 2006  Tim Janik  <timj@imendio.com>
+
+	* glib/gslice.c: honour g_mem_gc_friendly settings when freeing
+	slices, make sure g_mem_gc_friendly is properly initialized.
+
+	* gmem.[hc]: ensure g_mem_gc_friendly is initialized from G_DEBUG upon
+	the first allocation. applied some branching optimizations.
+
+	* docs/macros.txt: reflected --enable-gc-friendly change and
+	described ENABLE_GC_FRIENDLY_DEFAULT as well as G_DEBUG=gc-friendly.
+
+	* configure.in: changed --enable-gc-friendly=yes to define
+	ENABLE_GC_FRIENDLY_DEFAULT.
+
+	* glib/garray.c: changed ENABLE_GC_FRIENDLY macro #ifdef-s to
+	if (G_UNLIKELY (g_mem_gc_friendly)).
+
+	* glib/gtree.c:
+	* glib/ghash.c: removed ENABLE_GC_FRIENDLY code which is now taken
+	care of by g_slice_free1().
+
+	* tests/slice-test.c: fixed leaks, reported by Kjartan Maraas.
+
 Tue Jan 24 17:49:36 2006  Tim Janik  <timj@imendio.com>
 
 	* glib/gslice.c: only use posix_memalign() if it's known to work,

ChangeLog.pre-2-12

@@ -1,3 +1,26 @@
+Wed Jan 25 16:39:18 2006  Tim Janik  <timj@imendio.com>
+
+	* glib/gslice.c: honour g_mem_gc_friendly settings when freeing
+	slices, make sure g_mem_gc_friendly is properly initialized.
+
+	* gmem.[hc]: ensure g_mem_gc_friendly is initialized from G_DEBUG upon
+	the first allocation. applied some branching optimizations.
+
+	* docs/macros.txt: reflected --enable-gc-friendly change and
+	described ENABLE_GC_FRIENDLY_DEFAULT as well as G_DEBUG=gc-friendly.
+
+	* configure.in: changed --enable-gc-friendly=yes to define
+	ENABLE_GC_FRIENDLY_DEFAULT.
+
+	* glib/garray.c: changed ENABLE_GC_FRIENDLY macro #ifdef-s to
+	if (G_UNLIKELY (g_mem_gc_friendly)).
+
+	* glib/gtree.c:
+	* glib/ghash.c: removed ENABLE_GC_FRIENDLY code which is now taken
+	care of by g_slice_free1().
+
+	* tests/slice-test.c: fixed leaks, reported by Kjartan Maraas.
+
 Tue Jan 24 17:49:36 2006  Tim Janik  <timj@imendio.com>
 
 	* glib/gslice.c: only use posix_memalign() if it's known to work,

configure.in

@@ -217,8 +217,7 @@ AC_DEFINE_UNQUOTED(G_COMPILED_WITH_DEBUGGING, ["${enable_debug}"],
 
 AC_MSG_CHECKING([whether to enable garbage collector friendliness])
 if test "x$enable_gc_friendly" = "xyes"; then
-  AC_DEFINE(ENABLE_GC_FRIENDLY, 1, [Whether to enable GC friendliness])
+  AC_DEFINE(ENABLE_GC_FRIENDLY_DEFAULT, 1, [Whether to enable GC friendliness by default])
-  AC_SUBST(ENABLE_GC_FRIENDLY)
   AC_MSG_RESULT([yes])
 else
   AC_MSG_RESULT([no])

docs/macros.txt

@@ -10,7 +10,7 @@ GLib's configure options and corresponding macros
 --enable-debug=yes	[default for development branches]
 	-DG_ENABLE_DEBUG -g
 --enable-gc-friendly=yes
-	#define ENABLE_GC_FRIENDLY 1
+	#define ENABLE_GC_FRIENDLY_DEFAULT 1
 --disable-mem-pools=yes
 	#define DISABLE_MEM_POOLS 1
 
@@ -23,11 +23,13 @@ Notes on GLib's internal and global macros
 ==========================================
 
 
-ENABLE_GC_FRIENDLY
+ENABLE_GC_FRIENDLY_DEFAULT
 	Newly allocated memory that isn't directly initialized, as well
 	as memory being freed should be reset to 0. The point here is to
 	allow memory checkers and similar programs that use bohem GC alike
 	algorithms to produce more accurate results.
+	This can also be accomplished by setting the environment variable
+	G_DEBUG=gc-friendly.
 DISABLE_MEM_POOLS
 	Many small chunks of memory are often allocated via collective pools
 	in GLib and are cached after release to speed up reallocations.

glib/garray.c

@@ -197,10 +197,8 @@ g_array_set_size (GArray *farray,
       if (array->clear)
 	g_array_elt_zero (array, array->len, length - array->len);
     }
-#ifdef ENABLE_GC_FRIENDLY  
+  else if (G_UNLIKELY (g_mem_gc_friendly) && length < array->len)
-  else if (length < array->len)
     g_array_elt_zero (array, length, array->len - length);
-#endif /* ENABLE_GC_FRIENDLY */  
   
   array->len = length;
   
@@ -226,11 +224,10 @@ g_array_remove_index (GArray* farray,
   
   array->len -= 1;
 
-#ifdef ENABLE_GC_FRIENDLY
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  g_array_elt_zero (array, array->len, 1);
+    g_array_elt_zero (array, array->len, 1);
-#else /* !ENABLE_GC_FRIENDLY */
+  else
-  g_array_zero_terminate (array);
+    g_array_zero_terminate (array);
-#endif /* ENABLE_GC_FRIENDLY */  
 
   return farray;
 }
@@ -252,11 +249,10 @@ g_array_remove_index_fast (GArray* farray,
   
   array->len -= 1;
 
-#ifdef ENABLE_GC_FRIENDLY
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  g_array_elt_zero (array, array->len, 1);
+    g_array_elt_zero (array, array->len, 1);
-#else /* !ENABLE_GC_FRIENDLY */
+  else
-  g_array_zero_terminate (array);
+    g_array_zero_terminate (array);
-#endif /* ENABLE_GC_FRIENDLY */  
 
   return farray;
 }
@@ -278,11 +274,10 @@ g_array_remove_range (GArray       *farray,
                (array->len - (index_ + length)) * array->elt_size);
 
   array->len -= length;
-#ifdef ENABLE_GC_FRIENDLY
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  g_array_elt_zero (array, array->len, length);
+    g_array_elt_zero (array, array->len, length);
-#else /* !ENABLE_GC_FRIENDLY */
+  else
-  g_array_zero_terminate (array);
+    g_array_zero_terminate (array);
-#endif /* ENABLE_GC_FRIENDLY */
 
   return farray;
 }
@@ -343,9 +338,8 @@ g_array_maybe_expand (GRealArray *array,
 
       array->data = g_realloc (array->data, want_alloc);
 
-#ifdef ENABLE_GC_FRIENDLY
+      if (G_UNLIKELY (g_mem_gc_friendly))
-      memset (array->data + array->alloc, 0, want_alloc - array->alloc);
+        memset (array->data + array->alloc, 0, want_alloc - array->alloc);
-#endif /* ENABLE_GC_FRIENDLY */
 
       array->alloc = want_alloc;
     }
@@ -414,16 +408,13 @@ g_ptr_array_maybe_expand (GRealPtrArray *array,
 {
   if ((array->len + len) > array->alloc)
     {
-#ifdef ENABLE_GC_FRIENDLY
       guint old_alloc = array->alloc;
-#endif /* ENABLE_GC_FRIENDLY */
       array->alloc = g_nearest_pow (array->len + len);
       array->alloc = MAX (array->alloc, MIN_ARRAY_SIZE);
       array->pdata = g_realloc (array->pdata, sizeof (gpointer) * array->alloc);
-#ifdef ENABLE_GC_FRIENDLY
+      if (G_UNLIKELY (g_mem_gc_friendly))
-      for ( ; old_alloc < array->alloc; old_alloc++)
+        for ( ; old_alloc < array->alloc; old_alloc++)
-	array->pdata [old_alloc] = NULL;
+          array->pdata [old_alloc] = NULL;
-#endif /* ENABLE_GC_FRIENDLY */
     }
 }
 
@@ -448,14 +439,12 @@ g_ptr_array_set_size  (GPtrArray   *farray,
       for (i = array->len; i < length; i++)
 	array->pdata[i] = NULL;
     }
-#ifdef ENABLE_GC_FRIENDLY  
+  if (G_UNLIKELY (g_mem_gc_friendly) && length < array->len)
-  else if (length < array->len)
     {
       int i;
       for (i = length; i < array->len; i++)
 	array->pdata[i] = NULL;
     }
-#endif /* ENABLE_GC_FRIENDLY */  
 
   array->len = length;
 }
@@ -479,9 +468,8 @@ g_ptr_array_remove_index (GPtrArray* farray,
   
   array->len -= 1;
 
-#ifdef ENABLE_GC_FRIENDLY  
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  array->pdata[array->len] = NULL;
+    array->pdata[array->len] = NULL;
-#endif /* ENABLE_GC_FRIENDLY */  
 
   return result;
 }
@@ -504,9 +492,8 @@ g_ptr_array_remove_index_fast (GPtrArray* farray,
 
   array->len -= 1;
 
-#ifdef ENABLE_GC_FRIENDLY  
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  array->pdata[array->len] = NULL;
+    array->pdata[array->len] = NULL;
-#endif /* ENABLE_GC_FRIENDLY */  
 
   return result;
 }
@@ -528,13 +515,12 @@ g_ptr_array_remove_range (GPtrArray* farray,
                (array->len - (index_ + length)) * sizeof (gpointer));
 
   array->len -= length;
-#ifdef ENABLE_GC_FRIENDLY
+  if (G_UNLIKELY (g_mem_gc_friendly))
-  {
+    {
-    guint i;
+      guint i;
-    for (i = 0; i < length; i++)
+      for (i = 0; i < length; i++)
-      array->pdata[array->len + i] = NULL;
+        array->pdata[array->len + i] = NULL;
-  }
+    }
-#endif /* ENABLE_GC_FRIENDLY */
 }
 
 gboolean

glib/ghash.c

@@ -718,12 +718,6 @@ g_hash_node_destroy (GHashNode      *hash_node,
     key_destroy_func (hash_node->key);
   if (value_destroy_func)
     value_destroy_func (hash_node->value);
-  
-#ifdef ENABLE_GC_FRIENDLY
-  hash_node->key = NULL;
-  hash_node->value = NULL;
-#endif /* ENABLE_GC_FRIENDLY */
-
   g_slice_free (GHashNode, hash_node);
 }
 

glib/gmem.c

@@ -38,17 +38,24 @@
 #include "gthreadinit.h"
 #include "galias.h"
 
+#define MEM_PROFILE_TABLE_SIZE 4096
+
+
 /* notes on macros:
  * having G_DISABLE_CHECKS defined disables use of glib_mem_profiler_table and
  * g_mem_profile().
  * REALLOC_0_WORKS is defined if g_realloc (NULL, x) works.
  * SANE_MALLOC_PROTOS is defined if the systems malloc() and friends functions
  * match the corresponding GLib prototypes, keep configure.in and gmem.h in sync here.
- * if ENABLE_GC_FRIENDLY is defined, freed memory should be 0-wiped.
+ * g_mem_gc_friendly is TRUE, freed memory should be 0-wiped.
  */
 
-#define MEM_PROFILE_TABLE_SIZE 4096
+/* --- prototypes --- */
+static gboolean g_mem_initialized = FALSE;
+static void     g_mem_init_nomessage (void);
 
+
+/* --- malloc wrappers --- */
 #ifndef	REALLOC_0_WORKS
 static gpointer
 standard_realloc (gpointer mem,
@@ -115,7 +122,9 @@ static GMemVTable glib_mem_vtable = {
 gpointer
 g_malloc (gulong n_bytes)
 {
-  if (n_bytes)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (n_bytes))
     {
       gpointer mem;
 
@@ -132,7 +141,9 @@ g_malloc (gulong n_bytes)
 gpointer
 g_malloc0 (gulong n_bytes)
 {
-  if (n_bytes)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (n_bytes))
     {
       gpointer mem;
 
@@ -150,7 +161,9 @@ gpointer
 g_realloc (gpointer mem,
 	   gulong   n_bytes)
 {
-  if (n_bytes)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (n_bytes))
     {
       mem = glib_mem_vtable.realloc (mem, n_bytes);
       if (mem)
@@ -168,14 +181,18 @@ g_realloc (gpointer mem,
 void
 g_free (gpointer mem)
 {
-  if (mem)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (mem))
     glib_mem_vtable.free (mem);
 }
 
 gpointer
 g_try_malloc (gulong n_bytes)
 {
-  if (n_bytes)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (n_bytes))
     return glib_mem_vtable.try_malloc (n_bytes);
   else
     return NULL;
@@ -198,7 +215,9 @@ gpointer
 g_try_realloc (gpointer mem,
 	       gulong   n_bytes)
 {
-  if (n_bytes)
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+  if (G_LIKELY (n_bytes))
     return glib_mem_vtable.try_realloc (mem, n_bytes);
 
   if (mem)
@@ -373,6 +392,9 @@ g_mem_profile (void)
   gulong local_zinit;
   gulong local_frees;
 
+  if (G_UNLIKELY (!g_mem_initialized))
+    g_mem_init_nomessage();
+
   g_mutex_lock (gmem_profile_mutex);
 
   local_allocs = profile_allocs;
@@ -655,12 +677,38 @@ g_allocator_free (GAllocator *allocator)
 {
 }
 
+#ifdef ENABLE_GC_FRIENDLY_DEFAULT
+gboolean g_mem_gc_friendly = TRUE;
+#else
+gboolean g_mem_gc_friendly = FALSE;
+#endif
+
+static void
+g_mem_init_nomessage (void)
+{
+  if (g_mem_initialized)
+    return;
+  /* don't use g_malloc/g_message here */
+  gchar buffer[1024];
+  const gchar *val = _g_getenv_nomalloc ("G_DEBUG", buffer);
+  static const GDebugKey keys[] = {
+    { "gc-friendly", 1 },
+  };
+  gint flags = !val ? 0 : g_parse_debug_string (val, keys, G_N_ELEMENTS (keys));
+  if (flags & 1)        /* gc-friendly */
+    {
+      g_mem_gc_friendly = TRUE;
+    }
+  g_mem_initialized = TRUE;
+}
+
 void
 _g_mem_thread_init_noprivate_nomessage (void)
 {
-  /* we may only create mutexes here, locking/unlocking itself
+  /* we may only create mutexes here, locking/
-   * does not yet work.
+   * unlocking a mutex does not yet work.
    */
+  g_mem_init_nomessage();
 #ifndef G_DISABLE_CHECKS
   gmem_profile_mutex = g_mutex_new ();
 #endif

glib/gmem.h

@@ -92,6 +92,8 @@ struct _GMemVTable
 void	 g_mem_set_vtable (GMemVTable	*vtable);
 gboolean g_mem_is_system_malloc (void);
 
+GLIB_VAR gboolean g_mem_gc_friendly;
+
 /* Memory profiler and checker, has to be enabled via g_mem_set_vtable()
  */
 GLIB_VAR GMemVTable	*glib_mem_profiler_table;

glib/gslice.c

@@ -327,6 +327,9 @@ g_slice_init_nomessage (void)
   allocator->max_slab_chunk_size_for_magazine_cache = MAX_SLAB_CHUNK_SIZE (allocator);
   if (allocator->config.always_malloc || allocator->config.bypass_magazines)
     allocator->max_slab_chunk_size_for_magazine_cache = 0;      /* non-optimized cases */
+  /* at this point, g_mem_gc_friendly() should be initialized, this
+   * should have been accomplished by the above g_malloc/g_new calls
+   */
 }
 
 static inline guint
@@ -791,8 +794,8 @@ g_slice_free1 (gsize    mem_size,
   gsize chunk_size = P2ALIGN (mem_size);
   guint acat = allocator_categorize (chunk_size);
   if (G_UNLIKELY (!mem_block))
-    /* pass */;
+    return;
-  else if (G_LIKELY (acat == 1))        /* allocate through magazine layer */
+  if (G_LIKELY (acat == 1))             /* allocate through magazine layer */
     {
       ThreadMemory *tmem = thread_memory_from_self();
       guint ix = SLAB_INDEX (allocator, chunk_size);
@@ -802,16 +805,24 @@ g_slice_free1 (gsize    mem_size,
           if (G_UNLIKELY (thread_memory_magazine2_is_full (tmem, ix)))
             thread_memory_magazine2_unload (tmem, ix);
         }
+      if (G_UNLIKELY (g_mem_gc_friendly))
+        memset (mem_block, 0, chunk_size);
       thread_memory_magazine2_free (tmem, ix, mem_block);
     }
   else if (acat == 2)                   /* allocate through slab allocator */
     {
+      if (G_UNLIKELY (g_mem_gc_friendly))
+        memset (mem_block, 0, chunk_size);
       g_mutex_lock (allocator->slab_mutex);
       slab_allocator_free_chunk (chunk_size, mem_block);
       g_mutex_unlock (allocator->slab_mutex);
     }
   else                                  /* delegate to system malloc */
-    g_free (mem_block);
+    {
+      if (G_UNLIKELY (g_mem_gc_friendly))
+        memset (mem_block, 0, mem_size);
+      g_free (mem_block);
+    }
 }
 
 void
@@ -851,6 +862,8 @@ g_slice_free_chain_with_offset (gsize    mem_size,
               if (G_UNLIKELY (thread_memory_magazine2_is_full (tmem, ix)))
                 thread_memory_magazine2_unload (tmem, ix);
             }
+          if (G_UNLIKELY (g_mem_gc_friendly))
+            memset (current, 0, chunk_size);
           thread_memory_magazine2_free (tmem, ix, current);
         }
     }
@@ -861,6 +874,8 @@ g_slice_free_chain_with_offset (gsize    mem_size,
         {
           guint8 *current = slice;
           slice = *(gpointer*) (current + next_offset);
+          if (G_UNLIKELY (g_mem_gc_friendly))
+            memset (current, 0, chunk_size);
           slab_allocator_free_chunk (chunk_size, current);
         }
       g_mutex_unlock (allocator->slab_mutex);
@@ -870,6 +885,8 @@ g_slice_free_chain_with_offset (gsize    mem_size,
       {
         guint8 *current = slice;
         slice = *(gpointer*) (current + next_offset);
+        if (G_UNLIKELY (g_mem_gc_friendly))
+          memset (current, 0, mem_size);
         g_free (current);
       }
 }

glib/gtree.c

@@ -259,14 +259,6 @@ g_tree_destroy (GTree *tree)
 	tree->key_destroy_func (node->key);
       if (tree->value_destroy_func)
 	tree->value_destroy_func (node->value);
-      
-#ifdef ENABLE_GC_FRIENDLY
-      node->left = NULL;
-      node->right = NULL;
-      node->key = NULL;
-      node->value = NULL;
-#endif /* ENABLE_GC_FRIENDLY */
-
       g_slice_free (GTreeNode, node);
 
       node = next;
@@ -724,13 +716,6 @@ g_tree_remove_internal (GTree         *tree,
         tree->value_destroy_func (node->value);
     }
 
-#ifdef ENABLE_GC_FRIENDLY
-  node->left = NULL;
-  node->right = NULL;
-  node->key = NULL;
-  node->value = NULL;
-#endif /* ENABLE_GC_FRIENDLY */
-
   g_slice_free (GTreeNode, node);
 
   tree->nnodes--;

tests/slice-test.c

@@ -126,6 +126,8 @@ test_memchunk_thread (gpointer data)
   for (i = 0; i < prime_size; i++)
     if (memchunks[i])
       old_mem_chunk_destroy (memchunks[i]);
+  g_free (ps);
+  g_free (ss);
 
   return NULL;
 }
@@ -178,6 +180,8 @@ test_sliced_mem_thread (gpointer data)
       for (j = 0; j < k; j++)
         g_slice_free1 (sz, ps[j]);
     }
+  g_free (ps);
+  g_free (ss);
 
   return NULL;
 }