rts: TraverseHeap: Update resetStaticObjectForProfiling docs

Simon's concern in the old comment, specifically:

    So all of the calls to traverseMaybeInitClosureData() here are
    initialising retainer sets with the wrong flip.

Is actually exactly what the code was intended to do. It makes the closure
data valid, then at the beginning of the traversal the flip bit is flipped
resetting all closures across the heap to invalid.

Now it used to be that the profiling code using the traversal has it's own
sense of valid vs. invalid beyond what the traversal code does and indeed
the retainer profiler still does this, there a getClosureData of NULL is
considered an invalid retainer set.

So in effect there wasn't any difference in invalidating closure data
rather than just resetting it to a valid zero, which might be what confused
Simon at the time.

As the code is now it actually uses the value of the valid/invalid bit in
the form of the 'first_visit' argument to the 'visit' callback so there is
a difference.

1 files changed, 18 insertions, 22 deletions

diff --git a/rts/TraverseHeap.c b/rts/TraverseHeap.c
index d5e9c6dc04..40e51a3ca8 100644
--- a/rts/TraverseHeap.c
+++ b/rts/TraverseHeap.c
@@ -1348,29 +1348,25 @@ traverseInvalidateClosureData(traverseState* ts)
 }
 
 /**
- *  Traverse all static objects for which we compute retainer sets,
- *  and reset their rs fields to NULL, which is accomplished by
- *  invoking traverseMaybeInitClosureData(). This function must be called
- *  before zeroing all objects reachable from scavenged_static_objects
- *  in the case of major garbage collections. See GarbageCollect() in
- *  GC.c.
- *  Note:
- *    The mut_once_list of the oldest generation must also be traversed?
- *    Why? Because if the evacuation of an object pointed to by a static
- *    indirection object fails, it is put back to the mut_once_list of
- *    the oldest generation.
- *    However, this is not necessary because any static indirection objects
- *    are just traversed through to reach dynamic objects. In other words,
- *    they are not taken into consideration in computing retainer sets.
+ * Traverse all static objects and invalidate their traversal-data. This ensures
+ * that when doing the actual traversal no static closures will seem to have
+ * been visited already because they weren't visited in the last run.
+ *
+ * This function must be called before zeroing all objects reachable from
+ * scavenged_static_objects in the case of major garbage collections. See
+ * GarbageCollect() in GC.c.
+ *
+ * Note:
+ *
+ *   The mut_once_list of the oldest generation must also be traversed?
+ *
+ *   Why? Because if the evacuation of an object pointed to by a static
+ *   indirection object fails, it is put back to the mut_once_list of the oldest
+ *   generation.
  *
- * SDM (20/7/2011): I don't think this is doing anything sensible,
- * because it happens before retainerProfile() and at the beginning of
- * retainerProfil() we change the sense of 'flip'.  So all of the
- * calls to traverseMaybeInitClosureData() here are initialising retainer sets
- * with the wrong flip.  Also, I don't see why this is necessary.  I
- * added a traverseMaybeInitClosureData() call to retainRoot(), and that seems
- * to have fixed the assertion failure in retainerSetOf() I was
- * encountering.
+ *   However, this is not necessary because any static indirection objects are
+ *   just traversed through to reach dynamic objects. In other words, they are
+ *   never visited during traversal.
  */
 void
 resetStaticObjectForProfiling( const traverseState *ts, StgClosure *static_objects )