[commit: ghc] ghc-7.6: Another overhaul of the recent_activity / idle GC handling (#5991) (f9077d9)

Repository : ssh://darcs.haskell.org//srv/darcs/ghc

On branch  : ghc-7.6

http://hackage.haskell.org/trac/ghc/changeset/f9077d98b9e58b0eca518c8ae370b0f5c4df8800

>---------------------------------------------------------------

commit f9077d98b9e58b0eca518c8ae370b0f5c4df8800
Author: Simon Marlow <[email protected]>
Date:   Fri Sep 21 15:49:22 2012 +0100

    Another overhaul of the recent_activity / idle GC handling (#5991)
    
    Improvements:
    
     - we now turn off the timer signal in the non-threaded RTS after
       idleGCDelay.  This should make the xmonad users on #5991 happy.
    
     - we now turn off the timer signal after idleGCDelay even if the
       idle GC is disabled with +RTS -I0.
    
     - we now do *not* turn off the timer when profiling.
    
     - more comments to explain the meaning of the various ACTIVITY_*
       values
    
    MERGED from commit 0b79d5cd4687dacf7efd430df7fba9d9a5a5ce32

>---------------------------------------------------------------

 includes/rts/Flags.h |    1 +
 rts/RtsFlags.c       |   14 ++++++++++++--
 rts/Schedule.c       |   16 ++++++++++++----
 rts/Schedule.h       |   28 +++++++++++++++++++++++-----
 rts/Timer.c          |   27 ++++++++++++++-------------
 5 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/includes/rts/Flags.h b/includes/rts/Flags.h
index da71a4b..9ca7fb9 100644
--- a/includes/rts/Flags.h
+++ b/includes/rts/Flags.h
@@ -53,6 +53,7 @@ struct GC_FLAGS {
     rtsBool frontpanel;
 
     Time    idleGCDelayTime;    /* units: TIME_RESOLUTION */
+    rtsBool doIdleGC;
 
     StgWord heapBase;           /* address to ask the OS for memory */
 };
diff --git a/rts/RtsFlags.c b/rts/RtsFlags.c
index cfe5c1b..bb260c2 100644
--- a/rts/RtsFlags.c
+++ b/rts/RtsFlags.c
@@ -115,6 +115,11 @@ void initRtsFlagsDefaults(void)
     RtsFlags.GcFlags.frontpanel         = rtsFalse;
 #endif
     RtsFlags.GcFlags.idleGCDelayTime    = USToTime(300000); // 300ms
+#ifdef THREADED_RTS
+    RtsFlags.GcFlags.doIdleGC           = rtsTrue;
+#else
+    RtsFlags.GcFlags.doIdleGC           = rtsFalse;
+#endif
 
 #if osf3_HOST_OS
 /* ToDo: Perhaps by adjusting this value we can make linking without
@@ -915,8 +920,13 @@ error = rtsTrue;
 		if (rts_argv[arg][2] == '\0') {
 		  /* use default */
 		} else {
-                    RtsFlags.GcFlags.idleGCDelayTime =
-                        fsecondsToTime(atof(rts_argv[arg]+2));
+                    Time t = fsecondsToTime(atof(rts_argv[arg]+2));
+                    if (t == 0) {
+                        RtsFlags.GcFlags.doIdleGC = rtsFalse;
+                    } else {
+                        RtsFlags.GcFlags.doIdleGC = rtsTrue;
+                        RtsFlags.GcFlags.idleGCDelayTime = t;
+                    }
 		}
 		break;
 
diff --git a/rts/Schedule.c b/rts/Schedule.c
index a3e11bb..0ab6d69 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -452,23 +452,29 @@ run_thread:
     dirty_TSO(cap,t);
     dirty_STACK(cap,t->stackobj);
 
-#if defined(THREADED_RTS)
-    if (recent_activity == ACTIVITY_DONE_GC) {
+    switch (recent_activity)
+    {
+    case ACTIVITY_DONE_GC: {
         // ACTIVITY_DONE_GC means we turned off the timer signal to
         // conserve power (see #1623).  Re-enable it here.
         nat prev;
         prev = xchg((P_)&recent_activity, ACTIVITY_YES);
+#ifndef PROFILING
         if (prev == ACTIVITY_DONE_GC) {
             startTimer();
         }
-    } else if (recent_activity != ACTIVITY_INACTIVE) {
+#endif
+        break;
+    }
+    case ACTIVITY_INACTIVE:
         // If we reached ACTIVITY_INACTIVE, then don't reset it until
         // we've done the GC.  The thread running here might just be
         // the IO manager thread that handle_tick() woke up via
         // wakeUpRts().
+        break;
+    default:
         recent_activity = ACTIVITY_YES;
     }
-#endif
 
     traceEventRunThread(cap, t);
 
@@ -1661,7 +1667,9 @@ delete_threads_and_gc:
             // 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.
             recent_activity = ACTIVITY_DONE_GC;
+#ifndef PROFILING
             stopTimer();
+#endif
             break;
         }
         // fall through...
diff --git a/rts/Schedule.h b/rts/Schedule.h
index 4eb3830..a44949e 100644
--- a/rts/Schedule.h
+++ b/rts/Schedule.h
@@ -61,12 +61,30 @@ void scheduleWorker (Capability *cap, Task *task);
 extern volatile StgWord sched_state;
 
 /* 
- * flag that tracks whether we have done any execution in this time slice.
+ * flag that tracks whether we have done any execution in this time
+ * slice, and controls the disabling of the interval timer.
+ *
+ * The timer interrupt transitions ACTIVITY_YES into
+ * ACTIVITY_MAYBE_NO, waits for RtsFlags.GcFlags.idleGCDelayTime,
+ * and then:
+ *   - if idle GC is no, set ACTIVITY_INACTIVE and wakeUpRts()
+ *   - if idle GC is off, set ACTIVITY_DONE_GC and stopTimer()
+ *
+ * If the scheduler finds ACTIVITY_INACTIVE, then it sets
+ * ACTIVITY_DONE_GC, performs the GC and calls stopTimer().
+ *
+ * If the scheduler finds ACTIVITY_DONE_GC and it has a thread to run,
+ * it enables the timer again with startTimer().
  */
-#define ACTIVITY_YES      0 /* there has been activity in the current slice */
-#define ACTIVITY_MAYBE_NO 1 /* no activity in the current slice */
-#define ACTIVITY_INACTIVE 2 /* a complete slice has passed with no activity */
-#define ACTIVITY_DONE_GC  3 /* like 2, but we've done a GC too */
+#define ACTIVITY_YES      0
+  // the RTS is active
+#define ACTIVITY_MAYBE_NO 1
+  // no activity since the last timer signal
+#define ACTIVITY_INACTIVE 2
+  // RtsFlags.GcFlags.idleGCDelayTime has passed with no activity
+#define ACTIVITY_DONE_GC  3
+  // like ACTIVITY_INACTIVE, but we've done a GC too (if idle GC is
+  // enabled) and the interval timer is now turned off.
 
 /* Recent activity flag.
  * Locks required  : Transition from MAYBE_NO to INACTIVE
diff --git a/rts/Timer.c b/rts/Timer.c
index 3f9bc8a..aa4b8d8 100644
--- a/rts/Timer.c
+++ b/rts/Timer.c
@@ -28,10 +28,8 @@
 /* ticks left before next pre-emptive context switch */
 static int ticks_to_ctxt_switch = 0;
 
-#if defined(THREADED_RTS)
 /* idle ticks left before we perform a GC */
 static int ticks_to_gc = 0;
-#endif
 
 /*
  * Function: handle_tick()
@@ -52,7 +50,6 @@ handle_tick(int unused STG_UNUSED)
       }
   }
 
-#if defined(THREADED_RTS)
   /* 
    * If we've been inactive for idleGCDelayTime (set by +RTS
    * -I), tell the scheduler to wake up and do a GC, to check
@@ -66,24 +63,28 @@ handle_tick(int unused STG_UNUSED)
       break;
   case ACTIVITY_MAYBE_NO:
       if (ticks_to_gc == 0) {
-          /* 0 ==> no idle GC */
-          recent_activity = ACTIVITY_DONE_GC;
-          // disable timer signals (see #1623)
-          stopTimer();
-      } else {
-          ticks_to_gc--;
-          if (ticks_to_gc == 0) {
-              ticks_to_gc = RtsFlags.GcFlags.idleGCDelayTime /
-                  RtsFlags.MiscFlags.tickInterval;
+          if (RtsFlags.GcFlags.doIdleGC) {
               recent_activity = ACTIVITY_INACTIVE;
+#ifdef THREADED_RTS
               wakeUpRts();
+              // The scheduler will call stopTimer() when it has done
+              // the GC.
+#endif
+          } else {
+              recent_activity = ACTIVITY_DONE_GC;
+              // disable timer signals (see #1623, #5991)
+              // but only if we're not profiling
+#ifndef PROFILING
+              stopTimer();
+#endif
           }
+      } else {
+          ticks_to_gc--;
       }
       break;
   default:
       break;
   }
-#endif
 }
 
 // This global counter is used to allow multiple threads to stop the