VM: big refactoring removing the gc_workhorse

Each collector were a slot_visitor containing a gc_workhorse containing
a policy class. This commit removes the gc_workhorse and the policies
and instead "inlines" the common code in the collector bodies. So there
is more code duplication, but the control flow doesn't "jump around" so
much so it is easier to understand. It also makes the nursery gc faster
because its collection code can be optimized better without the
workhorse/policy system.

GNUmakefile

@@ -111,7 +111,6 @@ ifdef CONFIG
 		vm/generic_arrays.hpp \
 		vm/callstack.hpp \
 		vm/slot_visitor.hpp \
-		vm/collector.hpp \
 		vm/to_tenured_collector.hpp \
 		vm/arrays.hpp \
 		vm/math.hpp \

vm/aging_collector.cpp

@@ -2,20 +2,38 @@
 
 namespace factor {
 
-struct aging_policy {
+struct to_aging_copier : no_fixup {
   aging_space* aging;
   tenured_space* tenured;
 
-  explicit aging_policy(factor_vm* parent)
-      : aging(parent->data->aging), tenured(parent->data->tenured) {}
+  to_aging_copier(aging_space* aging, tenured_space* tenured)
+      : aging(aging), tenured(tenured) { }
 
-  bool should_copy_p(object* untagged) {
-    return !(aging->contains_p(untagged) || tenured->contains_p(untagged));
+  object* fixup_data(object* obj) {
+    if (aging->contains_p(obj) || tenured->contains_p(obj)) {
+      return obj;
+    }
+
+    // Is there another forwarding pointer?
+    while (obj->forwarding_pointer_p()) {
+      object* dest = obj->forwarding_pointer();
+      obj = dest;
+    }
+
+    if (aging->contains_p(obj) || tenured->contains_p(obj)) {
+      return obj;
+    }
+
+    cell size = obj->size();
+    object* newpointer = aging->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
+
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+
+    return newpointer;
   }
-
-  void promoted_object(object* obj) {}
-
-  void visited_object(object* obj) {}
 };
 
 void factor_vm::collect_aging() {
@@ -25,10 +43,8 @@ void factor_vm::collect_aging() {
     // Change the op so that if we fail here, an assertion will be raised.
     current_gc->op = collect_to_tenured_op;
 
-    gc_workhorse<tenured_space, to_tenured_policy>
-        workhorse(this, data->tenured, to_tenured_policy(this));
-    slot_visitor<gc_workhorse<tenured_space, to_tenured_policy>>
-        visitor(this, workhorse);
+    slot_visitor<from_tenured_refs_copier>
+        visitor(this, from_tenured_refs_copier(data->tenured, &mark_stack));
 
     gc_event* event = current_gc->event;
 
@@ -53,15 +69,14 @@ void factor_vm::collect_aging() {
     std::swap(data->aging, data->aging_semispace);
     data->reset_aging();
 
-    aging_space *target = data->aging;
-    gc_workhorse<aging_space, aging_policy>
-        workhorse(this, target, aging_policy(this));
-    slot_visitor<gc_workhorse<aging_space, aging_policy>>
-        visitor(this, workhorse);
-    cell scan = target->start + target->occupied_space();
+    aging_space *aging = data->aging;
+    slot_visitor<to_aging_copier>
+        visitor(this, to_aging_copier(aging, data->tenured));
+
+    cell scan = aging->start + aging->occupied_space();
 
     visitor.visit_all_roots();
-    visitor.cheneys_algorithm(target, scan);
+    visitor.cheneys_algorithm(aging, scan);
 
     data->reset_nursery();
     code->clear_remembered_set();

vm/collector.hpp

@@ -1,62 +0,0 @@
-namespace factor {
-
-struct must_start_gc_again {
-};
-
-template <typename TargetGeneration, typename Policy>
-struct gc_workhorse : no_fixup {
-  static const bool translated_code_block_map = false;
-
-  factor_vm* parent;
-  TargetGeneration* target;
-  Policy policy;
-  code_heap* code;
-
-  gc_workhorse(factor_vm* parent, TargetGeneration* target, Policy policy)
-      : parent(parent), target(target), policy(policy), code(parent->code) {}
-
-  object* fixup_data(object* obj) {
-    FACTOR_ASSERT((parent->current_gc &&
-                   parent->current_gc->op == collect_growing_heap_op) ||
-                  parent->data->seg->in_segment_p((cell)obj));
-
-    if (!policy.should_copy_p(obj)) {
-      policy.visited_object(obj);
-      return obj;
-    }
-
-    // is there another forwarding pointer?
-    while (obj->forwarding_pointer_p()) {
-      object* dest = obj->forwarding_pointer();
-      obj = dest;
-    }
-
-    if (!policy.should_copy_p(obj)) {
-      policy.visited_object(obj);
-      return obj;
-    }
-
-    cell size = obj->size();
-    object* newpointer = target->allot(size);
-    if (!newpointer)
-      throw must_start_gc_again();
-
-    memcpy(newpointer, obj, size);
-    obj->forward_to(newpointer);
-
-    policy.promoted_object(newpointer);
-
-    return newpointer;
-  }
-
-  code_block* fixup_code(code_block* compiled) {
-    if (!code->allocator->state.marked_p((cell)compiled)) {
-      code->allocator->state.set_marked_p((cell)compiled, compiled->size());
-      parent->mark_stack.push_back((cell)compiled + 1);
-    }
-
-    return compiled;
-  }
-};
-
-}

vm/full_collector.cpp

@@ -2,25 +2,57 @@
 
 namespace factor {
 
-struct full_policy {
-  factor_vm* parent;
+struct full_collection_copier : no_fixup {
   tenured_space* tenured;
+  code_heap* code;
+  std::vector<cell> *mark_stack;
 
-  explicit full_policy(factor_vm* parent)
-      : parent(parent), tenured(parent->data->tenured) {}
+  full_collection_copier(tenured_space* tenured,
+                         code_heap* code,
+                         std::vector<cell> *mark_stack)
+      : tenured(tenured), code(code), mark_stack(mark_stack) { }
 
-  bool should_copy_p(object* untagged) {
-    return !tenured->contains_p(untagged);
+  object* fixup_data(object* obj) {
+    if (tenured->contains_p(obj)) {
+      if (!tenured->state.marked_p((cell)obj)) {
+        tenured->state.set_marked_p((cell)obj, obj->size());
+        mark_stack->push_back((cell)obj);
+      }
+      return obj;
+    }
+
+    // Is there another forwarding pointer?
+    while (obj->forwarding_pointer_p()) {
+      object* dest = obj->forwarding_pointer();
+      obj = dest;
+    }
+
+    if (tenured->contains_p(obj)) {
+      if (!tenured->state.marked_p((cell)obj)) {
+        tenured->state.set_marked_p((cell)obj, obj->size());
+        mark_stack->push_back((cell)obj);
+      }
+      return obj;
+    }
+
+    cell size = obj->size();
+    object* newpointer = tenured->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+
+    tenured->state.set_marked_p((cell)newpointer, newpointer->size());
+    mark_stack->push_back((cell)newpointer);
+    return newpointer;
   }
 
-  void promoted_object(object* obj) {
-    tenured->state.set_marked_p((cell)obj, obj->size());
-    parent->mark_stack.push_back((cell)obj);
-  }
-
-  void visited_object(object* obj) {
-    if (!tenured->state.marked_p((cell)obj))
-      promoted_object(obj);
+  code_block* fixup_code(code_block* compiled) {
+    if (!code->allocator->state.marked_p((cell)compiled)) {
+      code->allocator->state.set_marked_p((cell)compiled, compiled->size());
+      mark_stack->push_back((cell)compiled + 1);
+    }
+    return compiled;
   }
 };
 
@@ -40,10 +72,8 @@ void factor_vm::update_code_roots_for_sweep() {
 }
 
 void factor_vm::collect_mark_impl() {
-  gc_workhorse<tenured_space, full_policy>
-      workhorse(this, this->data->tenured, full_policy(this));
-  slot_visitor<gc_workhorse<tenured_space, full_policy>>
-      visitor(this, workhorse);
+  slot_visitor<full_collection_copier>
+      visitor(this, full_collection_copier(data->tenured, code, &mark_stack));
 
   mark_stack.clear();
 

vm/gc.hpp

@@ -1,5 +1,8 @@
 namespace factor {
 
+struct must_start_gc_again {
+};
+
 enum gc_op {
   collect_nursery_op,
   collect_aging_op,

vm/master.hpp

@@ -129,7 +129,6 @@ namespace factor { struct factor_vm; }
 #include "generic_arrays.hpp"
 #include "callstack.hpp"
 #include "slot_visitor.hpp"
-#include "collector.hpp"
 #include "to_tenured_collector.hpp"
 #include "arrays.hpp"
 #include "math.hpp"

vm/nursery_collector.cpp

@@ -2,28 +2,40 @@
 
 namespace factor {
 
-struct nursery_policy {
+struct nursery_copier : no_fixup {
   bump_allocator* nursery;
+  aging_space* aging;
 
-  explicit nursery_policy(bump_allocator* nursery) : nursery(nursery) {}
+  nursery_copier(bump_allocator* nursery, aging_space* aging)
+      : nursery(nursery), aging(aging) { }
 
-  bool should_copy_p(object* obj) {
-    return nursery->contains_p(obj);
+  object* fixup_data(object* obj) {
+    if (!nursery->contains_p(obj)) {
+      return obj;
+    }
+
+    if (obj->forwarding_pointer_p()) {
+      object* dest = obj->forwarding_pointer();
+      FACTOR_ASSERT(!nursery->contains_p(dest));
+      return dest;
+    }
+
+    cell size = obj->size();
+    object* newpointer = aging->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
+
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+    return newpointer;
   }
-
-  void promoted_object(object* obj) {}
-
-  void visited_object(object* obj) {}
 };
 
 void factor_vm::collect_nursery() {
-
   // Copy live objects from the nursery (as determined by the root set and
   // marked cards in aging and tenured) to aging space.
-  gc_workhorse<aging_space, nursery_policy>
-      workhorse(this, data->aging, nursery_policy(data->nursery));
-  slot_visitor<gc_workhorse<aging_space, nursery_policy>>
-      visitor(this, workhorse);
+  slot_visitor<nursery_copier>
+      visitor(this, nursery_copier(data->nursery, data->aging));
 
   cell scan = data->aging->start + data->aging->occupied_space();
 

vm/to_tenured_collector.cpp

@@ -4,12 +4,9 @@ namespace factor {
 
 void factor_vm::collect_to_tenured() {
   // Copy live objects from aging space to tenured space.
-  gc_workhorse<tenured_space, to_tenured_policy>
-      workhorse(this, data->tenured, to_tenured_policy(this));
-  slot_visitor<gc_workhorse<tenured_space, to_tenured_policy>>
-      visitor(this, workhorse);
-
   mark_stack.clear();
+  slot_visitor<from_tenured_refs_copier>
+      visitor(this, from_tenured_refs_copier(data->tenured, &mark_stack));
 
   visitor.visit_all_roots();
   gc_event* event = current_gc->event;

vm/to_tenured_collector.hpp

@@ -1,21 +1,39 @@
 namespace factor {
 
-struct to_tenured_policy {
-  factor_vm* parent;
+struct from_tenured_refs_copier : no_fixup {
   tenured_space* tenured;
+  std::vector<cell> *mark_stack;
 
-  explicit to_tenured_policy(factor_vm* parent)
-      : parent(parent), tenured(parent->data->tenured) {}
+  from_tenured_refs_copier(tenured_space* tenured,
+                           std::vector<cell> *mark_stack)
+      : tenured(tenured), mark_stack(mark_stack) { }
 
-  bool should_copy_p(object* untagged) {
-    return !tenured->contains_p(untagged);
+  object* fixup_data(object* obj) {
+    if (tenured->contains_p(obj)) {
+      return obj;
+    }
+
+    // Is there another forwarding pointer?
+    while (obj->forwarding_pointer_p()) {
+      object* dest = obj->forwarding_pointer();
+      obj = dest;
+    }
+
+    if (tenured->contains_p(obj)) {
+      return obj;
+    }
+
+    cell size = obj->size();
+    object* newpointer = tenured->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
+
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+
+    mark_stack->push_back((cell)newpointer);
+    return newpointer;
   }
-
-  void promoted_object(object* obj) {
-    parent->mark_stack.push_back((cell)obj);
-  }
-
-  void visited_object(object* obj) {}
 };
 
 }