Change tryStealSpark so it does not consume fizzled sparks

We want to count fizzled sparks accurately. Now tryStealSpark returns
fizzled sparks, and the callers now update the fizzled spark count.

4 files changed, 36 insertions, 29 deletions

diff --git a/rts/Capability.c b/rts/Capability.c
index fe5dbdca40..a9bb743654 100644
--- a/rts/Capability.c
+++ b/rts/Capability.c
@@ -92,7 +92,11 @@ findSpark (Capability *cap)
       //   spark = reclaimSpark(cap->sparks);
       // However, measurements show that this makes at least one benchmark
       // slower (prsa) and doesn't affect the others.
-      spark = tryStealSpark(cap);
+      spark = tryStealSpark(cap->sparks);
+      while (spark != NULL && fizzledSpark(spark)) {
+          cap->sparks_fizzled++;
+          spark = tryStealSpark(cap->sparks);
+      }
       if (spark != NULL) {
           cap->sparks_converted++;
 
@@ -121,7 +125,11 @@ findSpark (Capability *cap)
           if (emptySparkPoolCap(robbed)) // nothing to steal here
               continue;
 
-          spark = tryStealSpark(robbed);
+          spark = tryStealSpark(robbed->sparks);
+          while (spark != NULL && fizzledSpark(spark)) {
+              cap->sparks_fizzled++;
+              spark = tryStealSpark(robbed->sparks);
+          }
           if (spark == NULL && !emptySparkPoolCap(robbed)) {
               // we conflicted with another thread while trying to steal;
               // try again later.
diff --git a/rts/Schedule.c b/rts/Schedule.c
index 50e0663577..125f9f0b74 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -783,6 +783,10 @@ schedulePushWork(Capability *cap USED_IF_THREADS,
 		if (emptySparkPoolCap(free_caps[i])) {
 		    spark = tryStealSpark(cap->sparks);
 		    if (spark != NULL) {
+                        /* TODO: if anyone wants to re-enable this code then
+                         * they must consider the fizzledSpark(spark) case
+                         * and update the per-cap spark statistics.
+                         */
 			debugTrace(DEBUG_sched, "pushing spark %p to capability %d", spark, free_caps[i]->no);
 
             traceEventStealSpark(free_caps[i], t, cap->no);
diff --git a/rts/Sparks.c b/rts/Sparks.c
index 18d9597ec2..5c78055e8c 100644
--- a/rts/Sparks.c
+++ b/rts/Sparks.c
@@ -73,32 +73,6 @@ newSpark (StgRegTable *reg, StgClosure *p)
     return 1;
 }
 
-/* -----------------------------------------------------------------------------
- * 
- * tryStealSpark: try to steal a spark from a Capability.
- *
- * Returns a valid spark, or NULL if the pool was empty, and can
- * occasionally return NULL if there was a race with another thread
- * stealing from the same pool.  In this case, try again later.
- *
- -------------------------------------------------------------------------- */
-
-StgClosure *
-tryStealSpark (Capability *cap)
-{
-  SparkPool *pool = cap->sparks;
-  StgClosure *stolen;
-
-  do { 
-      stolen = stealWSDeque_(pool); 
-      // use the no-loopy version, stealWSDeque_(), since if we get a
-      // spurious NULL here the caller may want to try stealing from
-      // other pools before trying again.
-  } while (stolen != NULL && !closure_SHOULD_SPARK(stolen));
-
-  return stolen;
-}
-
 /* --------------------------------------------------------------------------
  * Remove all sparks from the spark queues which should not spark any
  * more.  Called after GC. We assume exclusive access to the structure
diff --git a/rts/Sparks.h b/rts/Sparks.h
index d714cb5d09..e3ddc0cfaa 100644
--- a/rts/Sparks.h
+++ b/rts/Sparks.h
@@ -30,7 +30,7 @@ INLINE_HEADER StgClosure* reclaimSpark(SparkPool *pool);
 // if the pool is almost empty).
 INLINE_HEADER rtsBool looksEmpty(SparkPool* deque);
 
-StgClosure * tryStealSpark     (Capability *cap);
+INLINE_HEADER StgClosure * tryStealSpark (SparkPool *cap);
 INLINE_HEADER rtsBool      fizzledSpark  (StgClosure *);
 
 void         freeSparkPool     (SparkPool *pool);
@@ -65,6 +65,27 @@ INLINE_HEADER void discardSparks (SparkPool *pool)
     discardElements(pool);
 }
 
+/* ----------------------------------------------------------------------------
+ * 
+ * tryStealSpark: try to steal a spark from a Capability.
+ *
+ * Returns either:
+ *  (a) a useful spark;
+ *  (b) a fizzled spark (use fizzledSpark to check);
+ *  (c) or NULL if the pool was empty, and can occasionally return NULL
+ *      if there was a race with another thread stealing from the same
+ *      pool.  In this case, try again later.
+ *
+ -------------------------------------------------------------------------- */
+
+INLINE_HEADER StgClosure * tryStealSpark (SparkPool *pool)
+{
+    return stealWSDeque_(pool);
+    // use the no-loopy version, stealWSDeque_(), since if we get a
+    // spurious NULL here the caller may want to try stealing from
+    // other pools before trying again.
+}
+
 INLINE_HEADER rtsBool fizzledSpark (StgClosure *spark)
 {
     return (GET_CLOSURE_TAG(spark) != 0 || !closure_SHOULD_SPARK(spark));