VM: move the stack scrubbing logic to call_frame_slot_visitor to avoid visiting the...

primitive_minor_gc() iterates the stack twice, first to scrub stack
locations, then to trace overinitialized ones and gc roots. By running
visit_callstack() before visit_stack_elements() you only need to do it
once.

diff --git a/basis/compiler/cfg/stacks/vacant/vacant-docs.factor b/basis/compiler/cfg/stacks/vacant/vacant-docs.factor
index e42fb039e24d4c60ace04d0df620524228f4ed6d..0745dc2f8509010b8625cc04bf666695b073beb2 100644 (file)
--- a/basis/compiler/cfg/stacks/vacant/vacant-docs.factor
+++ b/basis/compiler/cfg/stacks/vacant/vacant-docs.factor
@@ -10,7 +10,7 @@ ARTICLE: "compiler.cfg.stacks.vacant" "Uninitialized/overinitialized stack locat
   "##replace ... D 0"
   "##replace ... D 1"
 }
-"The GC check runs before stack locations 0 and 1 have been initialized, and so the GC needs to scrub them so that they don't get traced. This is achieved by computing uninitialized locations with a dataflow analysis, and recording the information in GC maps. The scrub_contexts() method on vm/gc.cpp reads this information from GC maps and performs the scrubbing." ;
+"The GC check runs before stack locations 0 and 1 have been initialized, and so the GC needs to scrub them so that they don't get traced. This is achieved by computing uninitialized locations with a dataflow analysis, and recording the information in GC maps. The call_frame_slot_visitor object in vm/slot_visitor.hpp reads this information from GC maps and performs the scrubbing." ;
 
 HELP: initial-state
 { $description "Initially the stack bottom is at 0 for both the data and retain stacks and no replaces have been registered." } ;

diff --git a/vm/contexts.cpp b/vm/contexts.cpp
index ce82bc0d9e63a3fb48c29b14873d9742d85cd7c6..162be34c85f3bee2f113c2f9f1a85e0262f47005 100644 (file)
--- a/vm/contexts.cpp
+++ b/vm/contexts.cpp
@@ -49,36 +49,6 @@ void context::fix_stacks() {
     reset_retainstack();
 }
 
-void context::scrub_stacks(gc_info* info, cell index) {
-  uint8_t* bitmap = info->gc_info_bitmap();
-
-  {
-    cell base = info->callsite_scrub_d(index);
-
-    for (cell loc = 0; loc < info->scrub_d_count; loc++) {
-      if (bitmap_p(bitmap, base + loc)) {
-#ifdef DEBUG_GC_MAPS
-        std::cout << "scrubbing datastack location " << loc << std::endl;
-#endif
-        *((cell*)datastack - loc) = 0;
-      }
-    }
-  }
-
-  {
-    cell base = info->callsite_scrub_r(index);
-
-    for (cell loc = 0; loc < info->scrub_r_count; loc++) {
-      if (bitmap_p(bitmap, base + loc)) {
-#ifdef DEBUG_GC_MAPS
-        std::cout << "scrubbing retainstack location " << loc << std::endl;
-#endif
-        *((cell*)retainstack - loc) = 0;
-      }
-    }
-  }
-}
-
 context::~context() {
   delete datastack_seg;
   delete retainstack_seg;

diff --git a/vm/gc.cpp b/vm/gc.cpp
index 6dce41083d6cffcaf8662008f8c89f0e74173230..9bd47b0a27a14b05037bf6e525cb3005c3f3cedf 100644 (file)
--- a/vm/gc.cpp
+++ b/vm/gc.cpp
@@ -186,47 +186,7 @@ void factor_vm::gc(gc_op op, cell requested_size, bool trace_contexts_p) {
   FACTOR_ASSERT(!data->high_fragmentation_p());
 }
 
-/* primitive_minor_gc() is invoked by inline GC checks, and it needs to fill in
-   uninitialized stack locations before actually calling the GC. See the
-   comment in compiler.cfg.stacks.uninitialized for details. */
-
-struct call_frame_scrubber {
-  factor_vm* parent;
-  context* ctx;
-
-  call_frame_scrubber(factor_vm* parent, context* ctx)
-      : parent(parent), ctx(ctx) {}
-
-  void operator()(void* frame_top, cell frame_size, code_block* owner,
-                  void* addr) {
-    cell return_address = owner->offset(addr);
-
-    gc_info* info = owner->block_gc_info();
-
-    FACTOR_ASSERT(return_address < owner->size());
-    cell index = info->return_address_index(return_address);
-    if (index != (cell)-1)
-      ctx->scrub_stacks(info, index);
-  }
-};
-
-void factor_vm::scrub_context(context* ctx) {
-  call_frame_scrubber scrubber(this, ctx);
-  iterate_callstack(ctx, scrubber);
-}
-
-void factor_vm::scrub_contexts() {
-  std::set<context*>::const_iterator begin = active_contexts.begin();
-  std::set<context*>::const_iterator end = active_contexts.end();
-  while (begin != end) {
-    scrub_context(*begin);
-    begin++;
-  }
-}
-
 void factor_vm::primitive_minor_gc() {
-  scrub_contexts();
-
   gc(collect_nursery_op, 0, /* requested size */
      true /* trace contexts? */);
 }

diff --git a/vm/slot_visitor.hpp b/vm/slot_visitor.hpp
index e288739d77e5060eafb8e1fa40049c01d12b59f0..1c95906b85f5bb91276b231be391ddcf321d58da 100644 (file)
--- a/vm/slot_visitor.hpp
+++ b/vm/slot_visitor.hpp
@@ -125,6 +125,7 @@ template <typename Fixup> struct slot_visitor {
   void visit_roots();
   void visit_callstack_object(callstack* stack);
   void visit_callstack(context* ctx);
+  void visit_context(context *ctx);
   void visit_contexts();
   void visit_code_block_objects(code_block* compiled);
   void visit_embedded_literals(code_block* compiled);
@@ -261,6 +262,16 @@ template <typename Fixup> void slot_visitor<Fixup>::visit_roots() {
                      parent->special_objects + special_object_count);
 }
 
+/* primitive_minor_gc() is invoked by inline GC checks, and it needs to fill in
+   uninitialized stack locations before actually calling the GC. See the
+   documentation in compiler.cfg.stacks.vacant for details.
+
+   So for each call frame:
+
+    - scrub some uninitialized locations
+    - trace some overinitialized locations
+    - trace roots in spill slots
+*/
 template <typename Fixup> struct call_frame_slot_visitor {
   factor_vm* parent;
   slot_visitor<Fixup>* visitor;
@@ -299,20 +310,28 @@ template <typename Fixup> struct call_frame_slot_visitor {
     cell* stack_pointer = (cell*)frame_top;
     uint8_t* bitmap = info->gc_info_bitmap();
 
-    /* Subtract old value of base pointer from every derived pointer. */
-    for (cell spill_slot = 0; spill_slot < info->derived_root_count;
-         spill_slot++) {
-      uint32_t base_pointer = info->lookup_base_pointer(callsite, spill_slot);
-      if (base_pointer != (uint32_t)-1) {
+    /* Scrub vacant stack locations. */
+    cell callsite_scrub_d = info->callsite_scrub_d(callsite);
+    for (cell loc = 0; loc < info->scrub_d_count; loc++) {
+      if (bitmap_p(bitmap, callsite_scrub_d + loc)) {
 #ifdef DEBUG_GC_MAPS
-        std::cout << "visiting derived root " << spill_slot
-                  << " with base pointer " << base_pointer << std::endl;
+        std::cout << "scrubbing datastack location " << loc << std::endl;
 #endif
-        stack_pointer[spill_slot] -= stack_pointer[base_pointer];
+        *((cell*)ctx->datastack - loc) = 0;
+      }
+    }
+
+    cell callsite_scrub_r = info->callsite_scrub_r(callsite);
+    for (cell loc = 0; loc < info->scrub_r_count; loc++) {
+      if (bitmap_p(bitmap, callsite_scrub_r + loc)) {
+#ifdef DEBUG_GC_MAPS
+        std::cout << "scrubbing retainstack location " << loc << std::endl;
+#endif
+        *((cell*)ctx->retainstack - loc) = 0;
       }
     }
 
-    /* Trace all overinitialized stack locations. */
+    /* Trace overinitialized stack locations. */
     cell callsite_check_d = info->callsite_check_d(callsite);
     for (uint32_t loc = 0; loc < info->check_d_count; loc++) {
       if (bitmap_p(bitmap, callsite_check_d + loc)) {
@@ -335,6 +354,19 @@ template <typename Fixup> struct call_frame_slot_visitor {
       }
     }
 
+    /* Subtract old value of base pointer from every derived pointer. */
+    for (cell spill_slot = 0; spill_slot < info->derived_root_count;
+         spill_slot++) {
+      uint32_t base_pointer = info->lookup_base_pointer(callsite, spill_slot);
+      if (base_pointer != (uint32_t)-1) {
+#ifdef DEBUG_GC_MAPS
+        std::cout << "visiting derived root " << spill_slot
+                  << " with base pointer " << base_pointer << std::endl;
+#endif
+        stack_pointer[spill_slot] -= stack_pointer[base_pointer];
+      }
+    }
+
     /* Update all GC roots, including base pointers. */
     cell callsite_gc_roots = info->callsite_gc_roots(callsite);
 
@@ -370,17 +402,24 @@ void slot_visitor<Fixup>::visit_callstack(context* ctx) {
   parent->iterate_callstack(ctx, call_frame_visitor, fixup);
 }
 
+template <typename Fixup>
+void slot_visitor<Fixup>::visit_context(context* ctx) {
+  /* Callstack is visited first because it scrubs the data and retain
+     stacks. */
+  visit_callstack(ctx);
+
+  visit_stack_elements(ctx->datastack_seg, (cell*)ctx->datastack);
+  visit_stack_elements(ctx->retainstack_seg, (cell*)ctx->retainstack);
+  visit_object_array(ctx->context_objects,
+                     ctx->context_objects + context_object_count);
+
+}
+
 template <typename Fixup> void slot_visitor<Fixup>::visit_contexts() {
   std::set<context*>::const_iterator begin = parent->active_contexts.begin();
   std::set<context*>::const_iterator end = parent->active_contexts.end();
   while (begin != end) {
-    context* ctx = *begin;
-
-    visit_stack_elements(ctx->datastack_seg, (cell*)ctx->datastack);
-    visit_stack_elements(ctx->retainstack_seg, (cell*)ctx->retainstack);
-    visit_object_array(ctx->context_objects,
-                       ctx->context_objects + context_object_count);
-    visit_callstack(ctx);
+    visit_context(*begin);
     begin++;
   }
 }