From: Ivan Maidanski Date: Wed, 7 Sep 2011 13:35:22 +0000 (+0400) Subject: HOTFIX: Remove locking in API GC_get_bytes_since_gc and friends. X-Git-Tag: gc7_2~138^2~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=92198c76397bf78c59cd170315ad2c857c5be584;p=gc HOTFIX: Remove locking in API GC_get_bytes_since_gc and friends. Add GC_get_heap_usage_safe() to API as a thread-safe alternative to GC_get_bytes_since_gc and friends. Remove newly-added lock-free GC_get_heap_size_inner and GC_get_free_bytes_inner from API. * include/gc.h (GC_get_heap_size, GC_get_free_bytes, GC_get_unmapped_bytes, GC_get_bytes_since_gc, GC_get_total_bytes): Refine comment. * include/gc.h (GC_HAVE_GET_HEAP_USAGE_SAFE): New macro. * include/gc.h (GC_get_heap_usage_safe): New API function. * misc.c (GC_get_heap_usage_safe): Likewise. * include/gc_mark.h (GC_get_heap_size_inner, GC_get_free_bytes_inner): Remove API function. * misc.c (GC_get_heap_size_inner, GC_get_free_bytes_inner): Likewise. * misc.c (GC_get_heap_size): Add comment. * misc.c (GC_get_heap_size, GC_get_free_bytes, GC_get_unmapped_bytes, GC_get_bytes_since_gc, GC_get_total_bytes): Remove locking. * misc.c (GC_get_unmapped_bytes): Do not test USE_MUNMAP (not needed after locking removal). * misc.c (GC_get_bytes_since_gc, GC_get_total_bytes): Cast result to size_t. --- diff --git a/include/gc.h b/include/gc.h index e0c4b43f..81a3a7b8 100644 --- a/include/gc.h +++ b/include/gc.h @@ -517,24 +517,52 @@ GC_API GC_stop_func GC_CALL GC_get_stop_func(void); /* data structures. Excludes the unmapped memory (returned to the OS). */ /* Includes empty blocks and fragmentation loss. Includes some pages */ /* that were allocated but never written. */ +/* This is an unsynchronized getter, so it should be called typically */ +/* with the GC lock held to avoid data races on multiprocessors (the */ +/* alternative is to use GC_get_heap_usage_safe API call instead). */ +/* This getter remains lock-free (unsynchronized) for compatibility */ +/* reason since some existing clients call it from a GC callback */ +/* holding the allocator lock. (This API function and the following */ +/* four ones bellow were made thread-safe in GC v7.2alpha1 and */ +/* reverted back in v7.2alpha7 for the reason described.) */ GC_API size_t GC_CALL GC_get_heap_size(void); /* Return a lower bound on the number of free bytes in the heap */ -/* (excluding the unmapped memory space). */ +/* (excluding the unmapped memory space). This is an unsynchronized */ +/* getter (see GC_get_heap_size comment regarding thread-safety). */ GC_API size_t GC_CALL GC_get_free_bytes(void); /* Return the size (in bytes) of the unmapped memory (which is returned */ /* to the OS but could be remapped back by the collector later unless */ -/* the OS runs out of system/virtual memory). */ +/* the OS runs out of system/virtual memory). This is an unsynchronized */ +/* getter (see GC_get_heap_size comment regarding thread-safety). */ GC_API size_t GC_CALL GC_get_unmapped_bytes(void); /* Return the number of bytes allocated since the last collection. */ +/* This is an unsynchronized getter (see GC_get_heap_size comment */ +/* regarding thread-safety). */ GC_API size_t GC_CALL GC_get_bytes_since_gc(void); /* Return the total number of bytes allocated in this process. */ -/* Never decreases, except due to wrapping. */ +/* Never decreases, except due to wrapping. This is an unsynchronized */ +/* getter (see GC_get_heap_size comment regarding thread-safety). */ GC_API size_t GC_CALL GC_get_total_bytes(void); +/* Return the heap usage information. This is a thread-safe (atomic) */ +/* alternative for the five above getters. (This function acquires */ +/* the allocator lock thus preventing data racing and returning the */ +/* consistent result.) Passing NULL pointer is allowed for any */ +/* argument. Returned (filled in) values are of word type. */ +/* (This API function and the accompanying macro were introduced in */ +/* GC v7.2alpha7 at the moment when GC_get_heap_size and the friends */ +/* were made lock-free again.) */ +#define GC_HAVE_GET_HEAP_USAGE_SAFE 1 +GC_API void GC_CALL GC_get_heap_usage_safe(GC_word * /* pheap_size */, + GC_word * /* pfree_bytes */, + GC_word * /* punmapped_bytes */, + GC_word * /* pbytes_since_gc */, + GC_word * /* ptotal_bytes */); + /* Disable garbage collection. Even GC_gcollect calls will be */ /* ineffective. */ GC_API void GC_CALL GC_disable(void); diff --git a/include/gc_mark.h b/include/gc_mark.h index d5594e64..34fedb54 100644 --- a/include/gc_mark.h +++ b/include/gc_mark.h @@ -207,10 +207,6 @@ GC_API void GC_CALL GC_register_describe_type_fn(int /* kind */, /* to be used when printing objects */ /* of a particular kind. */ -/* See gc.h for the description of these "inner" functions. */ -GC_API size_t GC_CALL GC_get_heap_size_inner(void); -GC_API size_t GC_CALL GC_get_free_bytes_inner(void); - /* Set and get the client notifier on collections. The client function */ /* is called at the start of every full GC (called with the allocation */ /* lock held). May be 0. This is a really tricky interface to use */ diff --git a/misc.c b/misc.c index 2ea209f2..c1644819 100644 --- a/misc.c +++ b/misc.c @@ -399,75 +399,62 @@ GC_API size_t GC_CALL GC_size(const void * p) return hhdr -> hb_sz; } + +/* These getters remain unsynchronized for compatibility (since some */ +/* clients could call some of them from a GC callback holding the */ +/* allocator lock). */ GC_API size_t GC_CALL GC_get_heap_size(void) { - size_t value; - DCL_LOCK_STATE; - LOCK(); /* ignore the memory space returned to OS (i.e. count only the */ /* space owned by the garbage collector) */ - value = (size_t)(GC_heapsize - GC_unmapped_bytes); - UNLOCK(); - return value; + return (size_t)(GC_heapsize - GC_unmapped_bytes); } GC_API size_t GC_CALL GC_get_free_bytes(void) { - size_t value; - DCL_LOCK_STATE; - LOCK(); /* ignore the memory space returned to OS */ - value = (size_t)(GC_large_free_bytes - GC_unmapped_bytes); - UNLOCK(); - return value; -} - -/* The _inner versions assume the caller holds the allocation lock. */ -/* Declared in gc_mark.h (where other public "inner" functions reside). */ -GC_API size_t GC_CALL GC_get_heap_size_inner(void) -{ - return (size_t)(GC_heapsize - GC_unmapped_bytes); -} - -GC_API size_t GC_CALL GC_get_free_bytes_inner(void) -{ return (size_t)(GC_large_free_bytes - GC_unmapped_bytes); } GC_API size_t GC_CALL GC_get_unmapped_bytes(void) { -# ifdef USE_MUNMAP - size_t value; - DCL_LOCK_STATE; - LOCK(); - value = (size_t)GC_unmapped_bytes; - UNLOCK(); - return value; -# else - return 0; -# endif + return (size_t)GC_unmapped_bytes; } GC_API size_t GC_CALL GC_get_bytes_since_gc(void) { - size_t value; - DCL_LOCK_STATE; - LOCK(); - value = GC_bytes_allocd; - UNLOCK(); - return value; + return (size_t)GC_bytes_allocd; } GC_API size_t GC_CALL GC_get_total_bytes(void) { - size_t value; - DCL_LOCK_STATE; - LOCK(); - value = GC_bytes_allocd+GC_bytes_allocd_before_gc; - UNLOCK(); - return value; + return (size_t)(GC_bytes_allocd + GC_bytes_allocd_before_gc); } +/* Return the heap usage information. This is a thread-safe (atomic) */ +/* alternative for the five above getters. NULL pointer is allowed for */ +/* any argument. Returned (filled in) values are of word type. */ +GC_API void GC_CALL GC_get_heap_usage_safe(GC_word *pheap_size, + GC_word *pfree_bytes, GC_word *punmapped_bytes, + GC_word *pbytes_since_gc, GC_word *ptotal_bytes) +{ + DCL_LOCK_STATE; + + LOCK(); + if (pheap_size != NULL) + *pheap_size = GC_heapsize - GC_unmapped_bytes; + if (pfree_bytes != NULL) + *pfree_bytes = GC_large_free_bytes - GC_unmapped_bytes; + if (punmapped_bytes != NULL) + *punmapped_bytes = GC_unmapped_bytes; + if (pbytes_since_gc != NULL) + *pbytes_since_gc = GC_bytes_allocd; + if (ptotal_bytes != NULL) + *ptotal_bytes = GC_bytes_allocd + GC_bytes_allocd_before_gc; + UNLOCK(); +} + + #ifdef THREADS GC_API int GC_CALL GC_get_suspend_signal(void) {