From 73e88c2a30cbb2fc9cf8c394c620c0f3edcdd0eb Mon Sep 17 00:00:00 2001 From: Simon Marlow Date: Mon, 9 Mar 2009 14:00:04 +0000 Subject: [PATCH] Fix a bug which sometimes caused extra major GCs to be performed A long-running GC would cause the timer signal to declare the system to be idle, which would cause a major GC immediately following the current GC. This only happened with +RTS -N2 or greater. --- rts/Schedule.c | 33 +++++++++++++++++++++------------ rts/sm/GC.c | 12 ++++++------ rts/sm/GC.h | 1 + 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/rts/Schedule.c b/rts/Schedule.c index 28e54f9..47636a3 100644 --- a/rts/Schedule.c +++ b/rts/Schedule.c @@ -1606,29 +1606,38 @@ delete_threads_and_gc: heap_census = scheduleNeedHeapProfile(rtsTrue); +#if defined(THREADED_RTS) + debugTrace(DEBUG_sched, "doing GC"); + // reset waiting_for_gc *before* GC, so that when the GC threads + // emerge they don't immediately re-enter the GC. + waiting_for_gc = 0; + GarbageCollect(force_major || heap_census, gc_type, cap); +#else + GarbageCollect(force_major || heap_census, 0, cap); +#endif + if (recent_activity == ACTIVITY_INACTIVE && force_major) { // We are doing a GC because the system has been idle for a // timeslice and we need to check for deadlock. Record the // fact that we've done a GC and turn off the timer signal; // it will get re-enabled if we run any threads after the GC. - // - // Note: this is done before GC, because after GC there might - // be threads already running (GarbageCollect() releases the - // GC threads when it completes), so we risk turning off the - // timer signal when it should really be on. recent_activity = ACTIVITY_DONE_GC; stopTimer(); } + else + { + // the GC might have taken long enough for the timer to set + // recent_activity = ACTIVITY_INACTIVE, but we aren't + // necessarily deadlocked: + recent_activity = ACTIVITY_YES; + } #if defined(THREADED_RTS) - debugTrace(DEBUG_sched, "doing GC"); - // reset waiting_for_gc *before* GC, so that when the GC threads - // emerge they don't immediately re-enter the GC. - waiting_for_gc = 0; - GarbageCollect(force_major || heap_census, gc_type, cap); -#else - GarbageCollect(force_major || heap_census, 0, cap); + if (gc_type == PENDING_GC_PAR) + { + releaseGCThreads(cap); + } #endif if (heap_census) { diff --git a/rts/sm/GC.c b/rts/sm/GC.c index e44a310..ec9cd07 100644 --- a/rts/sm/GC.c +++ b/rts/sm/GC.c @@ -146,7 +146,6 @@ static nat inc_running (void); static nat dec_running (void); static void wakeup_gc_threads (nat n_threads, nat me); static void shutdown_gc_threads (nat n_threads, nat me); -static void continue_gc_threads (nat n_threads, nat me); #if 0 && defined(DEBUG) static void gcCAFs (void); @@ -787,8 +786,6 @@ GarbageCollect (rtsBool force_major_gc, } #endif - continue_gc_threads(n_gc_threads, gct->thread_index); - RELEASE_SM_LOCK; gct = saved_gct; @@ -1144,14 +1141,17 @@ shutdown_gc_threads (nat n_threads USED_IF_THREADS, nat me USED_IF_THREADS) #endif } -static void -continue_gc_threads (nat n_threads USED_IF_THREADS, nat me USED_IF_THREADS) +void +releaseGCThreads (Capability *cap USED_IF_THREADS) { #if defined(THREADED_RTS) + nat n_threads = RtsFlags.ParFlags.nNodes; + nat me = cap->no; nat i; for (i=0; i < n_threads; i++) { if (i == me) continue; - if (gc_threads[i]->wakeup != GC_THREAD_WAITING_TO_CONTINUE) barf("continue_gc_threads"); + if (gc_threads[i]->wakeup != GC_THREAD_WAITING_TO_CONTINUE) + barf("releaseGCThreads"); gc_threads[i]->wakeup = GC_THREAD_INACTIVE; ACQUIRE_SPIN_LOCK(&gc_threads[i]->gc_spin); diff --git a/rts/sm/GC.h b/rts/sm/GC.h index 5fb142f..366125d 100644 --- a/rts/sm/GC.h +++ b/rts/sm/GC.h @@ -43,6 +43,7 @@ extern StgWord64 whitehole_spin; void gcWorkerThread (Capability *cap); void initGcThreads (void); void waitForGcThreads (Capability *cap); +void releaseGCThreads (Capability *cap); #define WORK_UNIT_WORDS 128 -- 1.7.10.4