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.

diff --git a/GNUmakefile b/GNUmakefile
index aa6155207b67fddb84bf49c5cf4b053e34500750..28259cb61bd36746f3a460b94dfeefc3ed56f8c3 100644 (file)
--- a/GNUmakefile
+++ b/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 \

diff --git a/vm/aging_collector.cpp b/vm/aging_collector.cpp
index 7be76101338e113a890cc4c74dceb41cc65f7960..335dbba7deb16514a1a26e91122b41a72bd8b188 100644 (file)
--- a/vm/aging_collector.cpp
+++ b/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;
+    }
 
-  void promoted_object(object* obj) {}
+    cell size = obj->size();
+    object* newpointer = aging->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
 
-  void visited_object(object* obj) {}
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+
+    return newpointer;
+  }
 };
 
 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();

diff --git a/vm/collector.hpp b/vm/collector.hpp
deleted file mode 100644 (file)
index f5aa010..0000000
--- a/vm/collector.hpp
+++ /dev/null
@@ -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;
-  }
-};
-
-}

diff --git a/vm/full_collector.cpp b/vm/full_collector.cpp
index adc531643643b3140ba57721095997dc867c1571..72c3930347bcd33d3bb293420352ba850ed00b76 100644 (file)
--- a/vm/full_collector.cpp
+++ b/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;
-
-  explicit full_policy(factor_vm* parent)
-      : parent(parent), tenured(parent->data->tenured) {}
-
-  bool should_copy_p(object* untagged) {
-    return !tenured->contains_p(untagged);
-  }
-
-  void promoted_object(object* obj) {
-    tenured->state.set_marked_p((cell)obj, obj->size());
-    parent->mark_stack.push_back((cell)obj);
+  code_heap* code;
+  std::vector<cell> *mark_stack;
+
+  full_collection_copier(tenured_space* tenured,
+                         code_heap* code,
+                         std::vector<cell> *mark_stack)
+      : tenured(tenured), code(code), mark_stack(mark_stack) { }
+
+  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 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();
 

diff --git a/vm/gc.hpp b/vm/gc.hpp
index 0592d03574f768e57772456580edfc666df633e8..11fbaa34cc84ee4af29ae59cfd880d46be332e65 100644 (file)
--- a/vm/gc.hpp
+++ b/vm/gc.hpp
@@ -1,5 +1,8 @@
 namespace factor {
 
+struct must_start_gc_again {
+};
+
 enum gc_op {
   collect_nursery_op,
   collect_aging_op,

diff --git a/vm/master.hpp b/vm/master.hpp
index feb05d6bd2c6c9616c1a86a9cf1452b4c7904e3a..8a3bcf43676afd341c64ac92d7d946c12a3fbc0f 100644 (file)
--- a/vm/master.hpp
+++ b/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"

diff --git a/vm/nursery_collector.cpp b/vm/nursery_collector.cpp
index 119d26bdfc9978f406eb88b7ce92c8443624707b..54e5a59d438cec04e8c4601e2f54f5b94c7d7c17 100644 (file)
--- a/vm/nursery_collector.cpp
+++ b/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;
+    }
 
-  void promoted_object(object* obj) {}
+    cell size = obj->size();
+    object* newpointer = aging->allot(size);
+    if (!newpointer)
+      throw must_start_gc_again();
 
-  void visited_object(object* obj) {}
+    memcpy(newpointer, obj, size);
+    obj->forward_to(newpointer);
+    return newpointer;
+  }
 };
 
 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();
 

diff --git a/vm/to_tenured_collector.cpp b/vm/to_tenured_collector.cpp
index 1d0f3029ea10fb8e6c2660488fe5ec4f1d586aec..3616945c522a58045effa6da3dd3c567ef0138d6 100644 (file)
--- a/vm/to_tenured_collector.cpp
+++ b/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;

diff --git a/vm/to_tenured_collector.hpp b/vm/to_tenured_collector.hpp
index 30beb8cb54f6b7e0a52f21a4613ea3363d9aa910..a46d982c3046772e19eb75fed48cf4ea1830561e 100644 (file)
--- a/vm/to_tenured_collector.hpp
+++ b/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;
+    }
 
-  void promoted_object(object* obj) {
-    parent->mark_stack.push_back((cell)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;
+    }
 
-  void visited_object(object* 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;
+  }
 };
 
 }