From 46d32a333e90a3326c1e0e393612be0b4cea29b3 Mon Sep 17 00:00:00 2001 From: hboehm Date: Thu, 13 Nov 2008 01:26:56 +0000 Subject: [PATCH] 2008-11-12 Hans Boehm (Really mostly Ivan Maidansky) * win32_threads.c: Remove mark lock spinning. * win32_threads.c, pthread_support.c: Update GC_unlocked_count, GC_spin_count, and GC_block_count using atomic operations. * tests/test.c: Declare n_tests as AO_t only if we have threads. --- ChangeLog | 6 +++ pthread_support.c | 12 +++--- tests/test.c | 6 ++- win32_threads.c | 120 ++++++++---------------------------------------------- 4 files changed, 35 insertions(+), 109 deletions(-) diff --git a/ChangeLog b/ChangeLog index e734193..e6281da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2008-11-12 Hans Boehm (Really mostly Ivan Maidansky) + * win32_threads.c: Remove mark lock spinning. + * win32_threads.c, pthread_support.c: Update GC_unlocked_count, + GC_spin_count, and GC_block_count using atomic operations. + * tests/test.c: Declare n_tests as AO_t only if we have threads. + 2008-11-11 Hans Boehm (Really almost entirely Ivan Maidansky) * win32_threads.c: Support PARALLEL_MARK. Make printf arg diff --git a/pthread_support.c b/pthread_support.c index 0e92a85..dff43e3 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -1259,9 +1259,9 @@ volatile GC_bool GC_collecting = 0; /* #define LOCK_STATS */ #ifdef LOCK_STATS - unsigned long GC_spin_count = 0; - unsigned long GC_block_count = 0; - unsigned long GC_unlocked_count = 0; + AO_t GC_spin_count = 0; + AO_t GC_block_count = 0; + AO_t GC_unlocked_count = 0; #endif STATIC void GC_generic_lock(pthread_mutex_t * lock) @@ -1272,7 +1272,7 @@ STATIC void GC_generic_lock(pthread_mutex_t * lock) if (0 == pthread_mutex_trylock(lock)) { # ifdef LOCK_STATS - ++GC_unlocked_count; + (void)AO_fetch_and_add1(&GC_unlocked_count); # endif return; } @@ -1283,7 +1283,7 @@ STATIC void GC_generic_lock(pthread_mutex_t * lock) switch(pthread_mutex_trylock(lock)) { case 0: # ifdef LOCK_STATS - ++GC_spin_count; + (void)AO_fetch_and_add1(&GC_spin_count); # endif return; case EBUSY: @@ -1294,7 +1294,7 @@ STATIC void GC_generic_lock(pthread_mutex_t * lock) } #endif /* !NO_PTHREAD_TRYLOCK */ # ifdef LOCK_STATS - ++GC_block_count; + (void)AO_fetch_and_add1(&GC_block_count); # endif pthread_mutex_lock(lock); } diff --git a/tests/test.c b/tests/test.c index b782c23..e03a441 100644 --- a/tests/test.c +++ b/tests/test.c @@ -879,7 +879,11 @@ void tree_test() # endif } -AO_t n_tests = 0; +#ifdef THREADS + AO_t n_tests = 0; +#else + unsigned n_tests = 0; +#endif GC_word bm_huge[10] = { 0xffffffff, diff --git a/win32_threads.c b/win32_threads.c index d4893ae..2c56ffd 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -1188,7 +1188,7 @@ STATIC pthread_cond_t builder_cv = PTHREAD_COND_INITIALIZER; /* as in pthread_support.c except that GC_generic_lock() is not used. */ #ifdef LOCK_STATS - unsigned long GC_block_count = 0; + AO_t GC_block_count = 0; #endif void GC_acquire_mark_lock(void) @@ -1197,7 +1197,7 @@ void GC_acquire_mark_lock(void) ABORT("pthread_mutex_lock failed"); } # ifdef LOCK_STATS - ++GC_block_count; + (void)AO_fetch_and_add1(&GC_block_count); # endif /* GC_generic_lock(&mark_mutex); */ # ifdef GC_ASSERTIONS @@ -1305,109 +1305,31 @@ STATIC HANDLE builder_cv = (HANDLE)0; /* Event with manual reset */ /* mark_mutex_event, builder_cv, mark_cv are initialized in GC_thr_init(). */ -#ifdef MARKLOCK_USES_SPIN -/* MARKLOCK_USES_SPIN has the opposite meaning to NO_PTHREAD_TRYLOCK. */ - -/* GC_pause() is the same as in pthread_support.c */ - -/* Spend a few cycles in a way that can't introduce contention with */ -/* other threads. */ -STATIC void GC_pause(void) -{ - int i; -# if !defined(__GNUC__) || defined(__INTEL_COMPILER) - volatile word dummy = 0; -# endif - - for (i = 0; i < 10; ++i) { -# if defined(__GNUC__) && !defined(__INTEL_COMPILER) - __asm__ __volatile__ (" " : : : "memory"); -# else - /* Something that's unlikely to be optimized away. */ - GC_noop1(++dummy); -# endif - } -} - -#ifndef SPIN_MAX -#define SPIN_MAX 128 /* Maximum number of calls to GC_pause before */ -#endif /* give up. */ - -volatile unsigned GC_spinning_disabled; /* don't spin if non-zero */ - -STATIC int GC_spin_and_trylock(volatile AO_t *pmutex_waitcnt) -{ - unsigned pause_length; - unsigned i; - GC_pause(); - for (pause_length = 2; pause_length <= SPIN_MAX; pause_length <<= 1) { - /* FIXME: Should we use ..._full version of CAS here (and below)? */ - if (AO_compare_and_swap(pmutex_waitcnt, 0, 1)) - return 0; - if (GC_spinning_disabled) - break; - for (i = 0; i < pause_length; ++i) - GC_pause(); - } - /* Failed. */ - /* Don't do final AO_compare_and_swap() after pausing. */ - return -1; -} - -#endif /* MARKLOCK_USES_SPIN */ - /* #define LOCK_STATS */ #ifdef LOCK_STATS - unsigned long GC_spin_count = 0; - unsigned long GC_block_count = 0; - unsigned long GC_unlocked_count = 0; + AO_t GC_block_count = 0; + AO_t GC_unlocked_count = 0; #endif void GC_acquire_mark_lock(void) { -# ifdef MARKLOCK_USES_SPIN - /* Here we implement the same spinning algorithm as */ - /* in GC_generic_lock() in pthread_support.c. */ - - if (GC_spinning_disabled) - goto nospinning; - if (!AO_compare_and_swap(&GC_mark_mutex_waitcnt, 0, 1)) { - if (GC_spin_and_trylock(&GC_mark_mutex_waitcnt) < 0) { - nospinning: -# endif - - if (AO_fetch_and_add1_full(&GC_mark_mutex_waitcnt) != 0) { + if (AO_fetch_and_add1_acquire(&GC_mark_mutex_waitcnt) != 0) { # ifdef LOCK_STATS - ++GC_block_count; + (void)AO_fetch_and_add1(&GC_block_count); # endif if (WaitForSingleObject(mark_mutex_event, INFINITE) == WAIT_FAILED) ABORT("WaitForSingleObject() failed"); - } -# ifdef LOCK_STATS - else { -# ifdef MARKLOCK_USES_SPIN - ++GC_spin_count; -# else - ++GC_unlocked_count; -# endif - } -# endif - -# ifdef MARKLOCK_USES_SPIN } # ifdef LOCK_STATS - else ++GC_spin_count; + else { + (void)AO_fetch_and_add1(&GC_unlocked_count); + } # endif - } -# ifdef LOCK_STATS - else ++GC_unlocked_count; -# endif -# endif - GC_ASSERT(GC_mark_lock_holder == NO_THREAD); -# ifdef GC_ASSERTIONS - GC_mark_lock_holder = (unsigned long)GetCurrentThreadId(); -# endif + GC_ASSERT(GC_mark_lock_holder == NO_THREAD); +# ifdef GC_ASSERTIONS + GC_mark_lock_holder = (unsigned long)GetCurrentThreadId(); +# endif } void GC_release_mark_lock(void) @@ -1417,7 +1339,7 @@ void GC_release_mark_lock(void) GC_mark_lock_holder = NO_THREAD; # endif GC_ASSERT(GC_mark_mutex_waitcnt != 0); - if (AO_fetch_and_sub1_full(&GC_mark_mutex_waitcnt) > 1 && + if (AO_fetch_and_sub1_release(&GC_mark_mutex_waitcnt) > 1 && SetEvent(mark_mutex_event) == FALSE) ABORT("SetEvent() failed"); } @@ -1487,15 +1409,13 @@ void GC_wait_marker(void) GC_mark_lock_holder = NO_THREAD; # endif - if ((waitcnt = GC_mark_mutex_waitcnt) > 1) { + /* FIXME: This looks dubious to me. Is it really OK if */ + /* GC_mark_mutex_waitcnt is decremented between the */ + /* following two lines?- HB */ + if ((waitcnt = AO_load(&GC_mark_mutex_waitcnt)) > 1) { (void)AO_fetch_and_sub1_full(&GC_mark_mutex_waitcnt); } else { GC_ASSERT(waitcnt != 0); -# ifdef MARKLOCK_USES_SPIN - /* Avoid spinning since GC_mark_mutex_waitcnt is guaranteed */ - /* to be non-zero till the end of this function. */ - GC_spinning_disabled++; -# endif } /* The state of mark_cv is non-signaled here. */ @@ -1517,10 +1437,6 @@ void GC_wait_marker(void) # ifdef GC_ASSERTIONS GC_mark_lock_holder = (unsigned long)GetCurrentThreadId(); # endif -# ifdef MARKLOCK_USES_SPIN - /* Allow spinning now */ - GC_spinning_disabled--; -# endif } } -- 2.7.4