Small refactoring, and add comments

I discovered a new invariant while experimenting (blackholing is not
optional when using parallel GC), so documented it.

diff --git a/rts/ThreadPaused.c b/rts/ThreadPaused.c
index 674d0d9..2a3f355 100644 (file)
--- a/rts/ThreadPaused.c
+++ b/rts/ThreadPaused.c
@@ -195,6 +195,10 @@ threadPaused(Capability *cap, StgTSO *tso)
     maybePerformBlockedException (cap, tso);
     if (tso->what_next == ThreadKilled) { return; }
 
+    // NB. Blackholing is *not* optional, we must either do lazy
+    // blackholing, or eager blackholing consistently.  See Note
+    // [upd-black-hole] in sm/Scav.c.
+
     stack_end = &tso->stack[tso->stack_size];
     
     frame = (StgClosure *)tso->sp;

diff --git a/rts/sm/Scav.c b/rts/sm/Scav.c
index 24f19c9..d396b9f 100644 (file)
--- a/rts/sm/Scav.c
+++ b/rts/sm/Scav.c
@@ -1680,24 +1680,34 @@ scavenge_stack(StgPtr p, StgPtr stack_end)
        // the indirection into an IND_PERM, so that evacuate will
        // copy the indirection into the old generation instead of
        // discarding it.
+        //
+        // Note [upd-black-hole]
+        // One slight hiccup is that the THUNK_SELECTOR machinery can
+        // overwrite the updatee with an IND.  In parallel GC, this
+        // could even be happening concurrently, so we can't check for
+        // the IND.  Fortunately if we assume that blackholing is
+        // happening (either lazy or eager), then we can be sure that
+        // the updatee is never a THUNK_SELECTOR and we're ok.
+        // NB. this is a new invariant: blackholing is not optional.
     {
         nat type;
         const StgInfoTable *i;
+        StgClosure *updatee;
 
-        i = ((StgUpdateFrame *)p)->updatee->header.info;
+        updatee = ((StgUpdateFrame *)p)->updatee;
+        i = updatee->header.info;
         if (!IS_FORWARDING_PTR(i)) {
-            type = get_itbl(((StgUpdateFrame *)p)->updatee)->type;
+            type = get_itbl(updatee)->type;
             if (type == IND) {
-                ((StgUpdateFrame *)p)->updatee->header.info = 
-                    (StgInfoTable *)&stg_IND_PERM_info;
+                updatee->header.info = &stg_IND_PERM_info;
             } else if (type == IND_OLDGEN) {
-                ((StgUpdateFrame *)p)->updatee->header.info = 
-                    (StgInfoTable *)&stg_IND_OLDGEN_PERM_info;
+                updatee->header.info = &stg_IND_OLDGEN_PERM_info;
             }            
-            evacuate(&((StgUpdateFrame *)p)->updatee);
-            p += sizeofW(StgUpdateFrame);
-            continue;
         }
+        evacuate(&((StgUpdateFrame *)p)->updatee);
+        ASSERT(GET_CLOSURE_TAG(((StgUpdateFrame *)p)->updatee) == 0);
+        p += sizeofW(StgUpdateFrame);
+        continue;
     }
 
       // small bitmap (< 32 entries, or 64 on a 64-bit machine)