Composition instead of inheritance for ChunkedStack.

Also, rename TraceStack to TraversalWorklist because the fact
that we're using a stack is an implementation detail.

Author: rudi-c

src/gc/collector.cpp

@@ -125,16 +125,14 @@ class ChunkedStack {
         }
     }
 
-    inline void _push(void* p) {
-        *cur++ = p;
-        if (cur == end) {
-            chunks.push_back(start);
-            get_chunk();
-        }
-    }
-
 public:
     ChunkedStack() { get_chunk(); }
+    ~ChunkedStack() {
+        RELEASE_ASSERT(end - cur == CHUNK_SIZE, "destroying non-empty ChunkedStack");
+        // We always have a block available in case we want to push items onto the TraversalWorklist,
+        // but that chunk needs to be released after use to avoid a memory leak.
+        release_chunk(start);
+    }
 
     void* pop() {
         if (cur > start) {
@@ -145,41 +143,50 @@ class ChunkedStack {
 
         return previous_pop;
     }
+
+    void push(void* p) {
+        *cur++ = p;
+        if (cur == end) {
+            chunks.push_back(start);
+            get_chunk();
+        }
+    }
 };
 std::vector<void**> ChunkedStack::free_chunks;
 
-enum TraceStackType {
+enum TraversalType {
     MarkPhase,
     FinalizationOrderingFindReachable,
     FinalizationOrderingRemoveTemporaries,
     MapReferencesPhase,
 };
 
-class TraceStack : public ChunkedStack {
-    TraceStackType visit_type;
+class Worklist {
+protected:
+    ChunkedStack stack;
+
+public:
+    void* next() { return stack.pop(); }
+};
+
+class TraversalWorklist : public Worklist {
+    TraversalType visit_type;
 
 public:
-    TraceStack(TraceStackType type) : ChunkedStack(), visit_type(type) {}
-    TraceStack(TraceStackType type, const std::unordered_set<void*>& roots) : TraceStack(type) {
+    TraversalWorklist(TraversalType type) : visit_type(type) {}
+    TraversalWorklist(TraversalType type, const std::unordered_set<void*>& roots) : TraversalWorklist(type) {
         for (void* p : roots) {
             ASSERT(!isMarked(GCAllocation::fromUserData(p)), "");
-            push(p);
+            addWork(p);
         }
     }
-    ~TraceStack() {
-        RELEASE_ASSERT(end - cur == CHUNK_SIZE, "destroying non-empty TraceStack");
 
-        // We always have a block available in case we want to push items onto the TraceStack,
-        // but that chunk needs to be released after use to avoid a memory leak.
-        release_chunk(start);
-    }
-
-    void push(void* p) {
+    void addWork(void* p) {
         GC_TRACE_LOG("Pushing %p\n", p);
         GCAllocation* al = GCAllocation::fromUserData(p);
 
         switch (visit_type) {
-            case TraceStackType::MarkPhase:
+            case TraversalType::MarkPhase:
 // Use this to print the directed edges of the GC graph traversal.
 // i.e. print every a -> b where a is a pointer and b is something a references
 #if 0
@@ -208,7 +215,7 @@ class TraceStack : public ChunkedStack {
                 break;
             // See PyPy's finalization ordering algorithm:
             // http://pypy.readthedocs.org/en/latest/discussion/finalizer-order.html
-            case TraceStackType::FinalizationOrderingFindReachable:
+            case TraversalType::FinalizationOrderingFindReachable:
                 if (orderingState(al) == FinalizationState::UNREACHABLE) {
                     setOrderingState(al, FinalizationState::TEMPORARY);
                 } else if (orderingState(al) == FinalizationState::REACHABLE_FROM_FINALIZER) {
@@ -217,7 +224,7 @@ class TraceStack : public ChunkedStack {
                     return;
                 }
                 break;
-            case TraceStackType::FinalizationOrderingRemoveTemporaries:
+            case TraversalType::FinalizationOrderingRemoveTemporaries:
                 if (orderingState(al) == FinalizationState::TEMPORARY) {
                     setOrderingState(al, FinalizationState::REACHABLE_FROM_FINALIZER);
                 } else {
@@ -228,22 +235,22 @@ class TraceStack : public ChunkedStack {
                 assert(false);
         }
 
-        _push(p);
+        stack.push(p);
     }
 };
 
-class ReferenceMapStack : public ChunkedStack {
+class ReferenceMapWorklist : public Worklist {
     ReferenceMap* refmap;
 
 public:
-    ReferenceMapStack(ReferenceMap* refmap) : ChunkedStack(), refmap(refmap) {}
-    ReferenceMapStack(ReferenceMap* refmap, const std::unordered_set<void*>& roots) : ChunkedStack(), refmap(refmap) {
+    ReferenceMapWorklist(ReferenceMap* refmap) : refmap(refmap) {}
+    ReferenceMapWorklist(ReferenceMap* refmap, const std::unordered_set<void*>& roots) : refmap(refmap) {
         for (void* p : roots) {
-            push(GCAllocation::fromUserData(p), NULL);
+            addWork(GCAllocation::fromUserData(p), NULL);
         }
     }
 
-    void push(GCAllocation* al, GCAllocation* source) {
+    void addWork(GCAllocation* al, GCAllocation* source) {
         assert(refmap);
 
         auto it = refmap->references.find(al);
@@ -270,7 +277,7 @@ class ReferenceMapStack : public ChunkedStack {
                 }
             }
 
-            _push(al->user_data);
+            stack.push(al->user_data);
         } else {
             if (source) {
                 // We found that there exists a pointer from `source` to `al`
@@ -435,7 +442,7 @@ void GCVisitor::_visit(void** ptr_address) {
     }
 
     ASSERT(global_heap.getAllocationFromInteriorPointer(p)->user_data == p, "%p", p);
-    stack->push(p);
+    worklist->addWork(p);
 }
 
 void GCVisitor::_visitRange(void** start, void** end) {
@@ -454,7 +461,7 @@ void GCVisitor::_visitRange(void** start, void** end) {
 void GCVisitor::visitPotential(void* p) {
     GCAllocation* a = global_heap.getAllocationFromInteriorPointer(p);
     if (a) {
-        stack->push(a->user_data);
+        worklist->addWork(a->user_data);
     }
 }
 
@@ -491,14 +498,14 @@ void GCVisitorPinning::_visit(void** ptr_address) {
 
     GCAllocation* al = global_heap.getAllocationFromInteriorPointer(p);
     ASSERT(al->user_data == p, "%p", p);
-    stack->push(al, source);
+    worklist->addWork(al, source);
 }
 
 void GCVisitorPinning::visitPotential(void* p) {
     GCAllocation* a = global_heap.getAllocationFromInteriorPointer(p);
     if (a) {
-        stack->pin(a);
-        stack->push(a, source);
+        worklist->pin(a);
+        worklist->addWork(a, source);
     }
 }
 
@@ -538,11 +545,11 @@ static void finalizationOrderingFindReachable(Box* obj) {
     static StatCounter sc_us("us_gc_mark_finalizer_ordering_1");
     Timer _t("finalizationOrderingFindReachable", /*min_usec=*/10000);
 
-    TraceStack stack(TraceStackType::FinalizationOrderingFindReachable);
-    GCVisitor visitor(&stack);
+    TraversalWorklist worklist(TraversalType::FinalizationOrderingFindReachable);
+    GCVisitor visitor(&worklist);
 
-    stack.push(obj);
-    while (void* p = stack.pop()) {
+    worklist.addWork(obj);
+    while (void* p = worklist.next()) {
         sc_marked_objs.log();
 
         visitByGCKind(p, visitor);
@@ -556,11 +563,11 @@ static void finalizationOrderingRemoveTemporaries(Box* obj) {
     static StatCounter sc_us("us_gc_mark_finalizer_ordering_2");
     Timer _t("finalizationOrderingRemoveTemporaries", /*min_usec=*/10000);
 
-    TraceStack stack(TraceStackType::FinalizationOrderingRemoveTemporaries);
-    GCVisitor visitor(&stack);
+    TraversalWorklist worklist(TraversalType::FinalizationOrderingRemoveTemporaries);
+    GCVisitor visitor(&worklist);
 
-    stack.push(obj);
-    while (void* p = stack.pop()) {
+    worklist.addWork(obj);
+    while (void* p = worklist.next()) {
         GCAllocation* al = GCAllocation::fromUserData(p);
         assert(orderingState(al) != FinalizationState::UNREACHABLE);
         visitByGCKind(p, visitor);
@@ -606,12 +613,12 @@ static void orderFinalizers() {
     sc_us.log(us);
 }
 
-static void graphTraversalMarking(ChunkedStack& stack, GCVisitor& visitor) {
+static void graphTraversalMarking(Worklist& worklist, GCVisitor& visitor) {
     static StatCounter sc_us("us_gc_mark_phase_graph_traversal");
     static StatCounter sc_marked_objs("gc_marked_object_count");
     Timer _t("traversing", /*min_usec=*/10000);
 
-    while (void* p = stack.pop()) {
+    while (void* p = worklist.next()) {
         sc_marked_objs.log();
 
         GCAllocation* al = GCAllocation::fromUserData(p);
@@ -741,12 +748,12 @@ static void markPhase() {
     GC_TRACE_LOG("Starting collection %d\n", ncollections);
 
     GC_TRACE_LOG("Looking at roots\n");
-    TraceStack stack(TraceStackType::MarkPhase, roots);
-    GCVisitor visitor(&stack);
+    TraversalWorklist worklist(TraversalType::MarkPhase, roots);
+    GCVisitor visitor(&worklist);
 
     visitRoots(visitor);
 
-    graphTraversalMarking(stack, visitor);
+    graphTraversalMarking(worklist, visitor);
 
     // Objects with finalizers cannot be freed in any order. During the call to a finalizer
     // of an object, the finalizer expects the object's references to still point to valid
@@ -775,16 +782,16 @@ static void sweepPhase(std::vector<Box*>& weakly_referenced) {
 }
 
 static void mapReferencesPhase(ReferenceMap& refmap) {
-    ReferenceMapStack stack(&refmap, roots);
-    GCVisitorPinning visitor(&stack);
+    ReferenceMapWorklist worklist(&refmap, roots);
+    GCVisitorPinning visitor(&worklist);
 
     visitRoots(visitor);
 
     for (auto obj : objects_with_ordered_finalizers) {
         visitor.visit(&obj);
     }
 
-    graphTraversalMarking(stack, visitor);
+    graphTraversalMarking(worklist, visitor);
 }
 
 // Move objects around memory randomly. The purpose is to test whether the rest

src/gc/collector.h

@@ -82,7 +82,7 @@ class GCVisitorNoRedundancy : public GCVisitor {
 // Code to prototype a moving GC.
 //
 
-class ReferenceMapStack;
+class ReferenceMapWorklist;
 
 #if MOVING_GC
 #define MOVING_OVERRIDE override
@@ -93,12 +93,12 @@ class ReferenceMapStack;
 // Bulds the reference map, and also determine which objects cannot be moved.
 class GCVisitorPinning : public GCVisitorNoRedundancy {
 private:
-    ReferenceMapStack* stack;
+    ReferenceMapWorklist* worklist;
 
     void _visit(void** ptr_address) MOVING_OVERRIDE;
 
 public:
-    GCVisitorPinning(ReferenceMapStack* stack) : stack(stack) {}
+    GCVisitorPinning(ReferenceMapWorklist* worklist) : worklist(worklist) {}
     virtual ~GCVisitorPinning() {}
 
     void visitPotential(void* p) MOVING_OVERRIDE;

src/gc/gc.h

@@ -46,9 +46,9 @@ class Box;
 namespace gc {
 
 class GCAllocation;
-class TraceStack;
+class TraversalWorklist;
 
-// The base version of the GC visitor is used for marking, in conjuction with a TraceStack.
+// The base version of the GC visitor is used for marking, in conjuction with a TraversalWorklist.
 //
 // Conceptually, GCVisitor should be abstract and the 'marking' behavior should be specific
 // to a subclass of GCVisitor. However, that requires the use of virtual functions which
@@ -57,7 +57,7 @@ class TraceStack;
 // the virtualness property is #if'd out for the regular use case with only mark-and-sweep.
 class GCVisitor {
 private:
-    TraceStack* stack = NULL;
+    TraversalWorklist* worklist = NULL;
 
 #if MOVING_GC
     virtual void _visit(void** ptr_address);
@@ -75,7 +75,7 @@ class GCVisitor {
 
 public:
     GCVisitor() {}
-    GCVisitor(TraceStack* stack) : stack(stack) {}
+    GCVisitor(TraversalWorklist* worklist) : worklist(worklist) {}
     virtual ~GCVisitor() {}
 
 #if MOVING_GC