Another overhaul of the recent_activity / idle GC handling (#5991)
authorSimon Marlow <marlowsd@gmail.com>
Fri, 21 Sep 2012 14:49:22 +0000 (15:49 +0100)
committerSimon Marlow <marlowsd@gmail.com>
Mon, 24 Sep 2012 08:22:06 +0000 (09:22 +0100)
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

includes/rts/Flags.h
rts/RtsFlags.c
rts/Schedule.c
rts/Schedule.h
rts/Timer.c

index da71a4b..9ca7fb9 100644 (file)
@@ -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 */
 };
index 4ee19cf..3f38c8a 100644 (file)
@@ -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;
 
index 41f7f37..9bd0b6c 100644 (file)
@@ -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);
 
@@ -1671,7 +1677,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...
index 4eb3830..a44949e 100644 (file)
@@ -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
index 3f9bc8a..aa4b8d8 100644 (file)
 /* 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,8 +50,7 @@ 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
    * for threads that are deadlocked.
@@ -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