vm: stage code block map fixup properly for GC

Don't update the map until the very last thing, and pass untranslated addresses to the iterator functors. Somewhat complicated by the fact that, for startup_fixup, the map is initialized with fixed-up values, so the fixup functor needs a flag indicating whether it operates with a fixed or unfixed code heap map.

diff --git a/vm/callstack.hpp b/vm/callstack.hpp
index bd16117b40b10bb8519a84ac4161fc634199eeef..acbd29a6f03686e3158797ea63ebc63de8e37165 100755 (executable)
--- a/vm/callstack.hpp
+++ b/vm/callstack.hpp
@@ -21,7 +21,9 @@ void factor_vm::iterate_callstack_object_reversed(callstack *stack_,
                void *frame_top = stack->frame_top_at(frame_offset);
                void *addr = frame_return_address(frame_top);
 
-               void *fixed_addr = (void*)fixup.translate_code((code_block*)addr);
+               void *fixed_addr = Fixup::translated_code_block_map
+                       ? (void*)fixup.translate_code((code_block*)addr)
+                       : addr;
                code_block *owner = code->code_block_for_address((cell)fixed_addr);
                cell frame_size = owner->stack_frame_size_for_address((cell)fixed_addr);
 
@@ -35,13 +37,13 @@ void factor_vm::iterate_callstack_object_reversed(callstack *stack_,
                FACTOR_ASSERT(frame_size == frame->size);
 #endif
 
-               iterator(frame_top, owner, fixed_addr);
+               iterator(frame_top, frame_size, owner, fixed_addr);
                frame_offset += frame_size;
        }
 }
 
-template<typename Iterator> void factor_vm::iterate_callstack_object(callstack *stack_,
-       Iterator &iterator)
+template<typename Iterator>
+void factor_vm::iterate_callstack_object(callstack *stack_, Iterator &iterator)
 {
        data_root<callstack> stack(stack_,this);
        fixnum frame_offset = factor::untag_fixnum(stack->length) - sizeof(stack_frame);
@@ -66,23 +68,32 @@ void factor_vm::iterate_callstack_reversed(context *ctx, Iterator &iterator, Fix
        {
                void *addr = frame_return_address((void*)frame_top);
                FACTOR_ASSERT(addr != 0);
-
-               void *fixed_addr = (void*)fixup.translate_code((code_block*)addr);
+               void *fixed_addr = Fixup::translated_code_block_map
+                       ? (void*)fixup.translate_code((code_block*)addr)
+                       : addr;
 
                code_block *owner = code->code_block_for_address((cell)fixed_addr);
-               cell frame_size = owner->stack_frame_size_for_address((cell)fixed_addr);
+               code_block *fixed_owner = Fixup::translated_code_block_map
+                       ? owner
+                       : fixup.translate_code(owner);
+
+               cell frame_size = fixed_owner->stack_frame_size_for_address((cell)fixed_addr);
 
 #ifdef FACTOR_DEBUG
                // check our derived owner and frame size against the ones stored in the frame
                // by the function prolog
                stack_frame *frame = (stack_frame*)(frame_top + frame_size) - 1;
-               void *fixed_entry_point =
-                       (void*)fixup.translate_code((code_block*)frame->entry_point);
+               void *fixed_entry_point = Fixup::translated_code_block_map
+                       ? (void*)fixup.translate_code((code_block*)frame->entry_point)
+                       : frame->entry_point;
                FACTOR_ASSERT(owner->entry_point() == fixed_entry_point);
                FACTOR_ASSERT(frame_size == frame->size);
 #endif
+               void *fixed_addr_for_iter = Fixup::translated_code_block_map
+                       ? fixed_addr
+                       : addr;
 
-               iterator(frame_top, owner, fixed_addr);
+               iterator(frame_top, frame_size, owner, fixed_addr_for_iter);
                frame_top += frame_size;
        }
 }
@@ -111,7 +122,7 @@ void factor_vm::iterate_callstack_reversed(context *ctx, Iterator &iterator)
                FACTOR_ASSERT(frame_size == frame->size);
 #endif
 
-               iterator(frame_top, owner, addr);
+               iterator(frame_top, frame_size, owner, addr);
                frame_top += frame_size;
        }
 }

diff --git a/vm/code_block_visitor.hpp b/vm/code_block_visitor.hpp
index 96881ed37f74009d4e74a6c5033afd72d033487a..53cd1befbb9502c875186dbde6a04567f44ea41b 100644 (file)
--- a/vm/code_block_visitor.hpp
+++ b/vm/code_block_visitor.hpp
@@ -42,13 +42,16 @@ struct call_frame_code_block_visitor {
        explicit call_frame_code_block_visitor(factor_vm *parent_, Fixup fixup_) :
                parent(parent_), fixup(fixup_) {}
 
-       void operator()(void *frame_top, code_block *owner, void *addr)
+       void operator()(void *frame_top, cell frame_size, code_block *owner, void *addr)
        {
-               set_frame_return_address(frame_top, addr);
+               code_block *compiled = Fixup::translated_code_block_map
+                       ? owner
+                       : fixup.fixup_code(owner);
+               void *fixed_addr = compiled->address_for_offset(owner->offset(addr));
+               set_frame_return_address(frame_top, fixed_addr);
                // XXX remove this when prolog data is removed
-               cell frame_size = owner->stack_frame_size_for_address((cell)addr);
                stack_frame *frame = (stack_frame*)((char*)frame_top + frame_size) - 1;
-               frame->entry_point = owner->entry_point();
+               frame->entry_point = compiled->entry_point();
        }
 };
 

diff --git a/vm/code_heap.cpp b/vm/code_heap.cpp
index 12f903f66a0081b7d0a0a3a328c756721b79be36..112769da7ada2b7c5ebd4c4dde52071b7527965f 100755 (executable)
--- a/vm/code_heap.cpp
+++ b/vm/code_heap.cpp
@@ -105,6 +105,7 @@ struct all_blocks_set_verifier {
        void operator()(code_block *block, cell size)
        {
                FACTOR_ASSERT(all_blocks->find((cell)block) != all_blocks->end());
+               // XXX check block size
        }
 };
 
@@ -121,20 +122,9 @@ code_block *code_heap::code_block_for_address(cell address)
        FACTOR_ASSERT(blocki != all_blocks.begin());
        --blocki;
        code_block* found_block = (code_block*)*blocki;
-#ifdef FACTOR_DEBUG
-       if (!((cell)found_block->entry_point() <= address
-               && address - (cell)found_block->entry_point() < found_block->size()))
-       {
-               std::cerr << "invalid block found in all_blocks set!" << std::endl;
-               std::cerr << "address " << (void*)address
-                       << " block " << (void*)found_block
-                       << " entry point " << (void*)found_block->entry_point()
-                       << " size " << found_block->size()
-                       << " free? " << found_block->free_p();
-               verify_all_blocks_set();
-               FACTOR_ASSERT(false);
-       }
-#endif
+       FACTOR_ASSERT((cell)found_block->entry_point() <= address
+               /* XXX this isn't valid during fixup. should store the size in the map
+               && address - (cell)found_block->entry_point() < found_block->size()*/);
        return found_block;
 }
 
@@ -154,6 +144,9 @@ void code_heap::initialize_all_blocks_set()
        all_blocks.clear();
        all_blocks_set_inserter inserter(this);
        allocator->iterate(inserter);
+#if defined(FACTOR_DEBUG)
+       verify_all_blocks_set();
+#endif
 }
 
 /* Allocate a code heap during startup */

diff --git a/vm/collector.hpp b/vm/collector.hpp
index 4a9eec59675529a50e3bd6b9b328f1f93ea7b9a3..454627f2ac83559febfa4a5184f3e4f94095d56e 100644 (file)
--- a/vm/collector.hpp
+++ b/vm/collector.hpp
@@ -4,6 +4,8 @@ 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;

diff --git a/vm/compaction.cpp b/vm/compaction.cpp
index 2b96e68002cc091a6718728be2cf2c3d3f5992d8..49cbde2bf275df1e5a30a61dee11e0e07fef14e4 100644 (file)
--- a/vm/compaction.cpp
+++ b/vm/compaction.cpp
@@ -3,6 +3,8 @@
 namespace factor {
 
 struct compaction_fixup {
+       static const bool translated_code_block_map = false;
+
        mark_bits<object> *data_forwarding_map;
        mark_bits<code_block> *code_forwarding_map;
        const object **data_finger;
@@ -192,6 +194,10 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
 {
        gc_event *event = current_gc->event;
 
+#if defined(FACTOR_DEBUG)
+       code->verify_all_blocks_set();
+#endif
+
        if(event) event->started_compaction();
 
        tenured_space *tenured = data->tenured;
@@ -224,8 +230,6 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
        code_block_compaction_updater<compaction_fixup> code_block_updater(this,fixup,data_forwarder,code_forwarder);
        code->allocator->compact(code_block_updater,fixup,&code_finger);
 
-       code->update_all_blocks_set(code_forwarding_map);
-
        data_forwarder.visit_roots();
        if(trace_contexts_p)
        {
@@ -242,6 +246,8 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
 }
 
 struct code_compaction_fixup {
+       static const bool translated_code_block_map = false;
+
        mark_bits<code_block> *code_forwarding_map;
        const code_block **code_finger;
 

diff --git a/vm/debug.cpp b/vm/debug.cpp
index 026ac7fd5ec7e87dd94d56f016214665dcb2059d..02896ae8de37a8fd2fdd857ff7378637798f999e 100755 (executable)
--- a/vm/debug.cpp
+++ b/vm/debug.cpp
@@ -220,10 +220,10 @@ struct stack_frame_printer {
        factor_vm *parent;
 
        explicit stack_frame_printer(factor_vm *parent_) : parent(parent_) {}
-       void operator()(void *frame_top, code_block *owner, void *addr)
+       void operator()(void *frame_top, cell frame_size, code_block *owner, void *addr)
        {
                std::cout << std::endl;
-               std::cout << "frame: " << frame_top << std::endl;
+               std::cout << "frame: " << frame_top << " size " << frame_size << std::endl;
                std::cout << "executing: ";
                parent->print_obj(owner->owner);
                std::cout << std::endl;

diff --git a/vm/fixup.hpp b/vm/fixup.hpp
index c92661a03bea634336ff7197063c8d441a10c9ac..3c768aff8d1f30d2eaa157145dce1208ae2bb868 100644 (file)
--- a/vm/fixup.hpp
+++ b/vm/fixup.hpp
@@ -10,6 +10,8 @@ struct identity {
 };
 
 struct no_fixup {
+       static const bool translated_code_block_map = false;
+
        object *fixup_data(object *obj)
        {
                return obj;

diff --git a/vm/image.cpp b/vm/image.cpp
index 806db02ca4b9cbce2363db3f59d026b8b059f75b..a1a930b325e9033fd877d34b932350c537216fa9 100755 (executable)
--- a/vm/image.cpp
+++ b/vm/image.cpp
@@ -57,6 +57,8 @@ void factor_vm::load_code_heap(FILE *file, image_header *h, vm_parameters *p)
 }
 
 struct startup_fixup {
+       static const bool translated_code_block_map = true;
+
        cell data_offset;
        cell code_offset;
 

diff --git a/vm/mark_bits.hpp b/vm/mark_bits.hpp
index b546f69d6a393eb22574c0a5fd6ddaf3426febfc..9a65f3a9d1e861e046d820cc0d3a72c394b3e6d8 100644 (file)
--- a/vm/mark_bits.hpp
+++ b/vm/mark_bits.hpp
@@ -126,8 +126,7 @@ template<typename Block> struct mark_bits {
        Block *forward_block(const Block *original)
        {
 #ifdef FACTOR_DEBUG
-               // XXX this condition seems to be harmless--ask slava
-               //FACTOR_ASSERT(marked_p(original));
+               FACTOR_ASSERT(marked_p(original));
 #endif
                std::pair<cell,cell> position = bitmap_deref(original);
                cell offset = (cell)original & (data_alignment - 1);

diff --git a/vm/slot_visitor.hpp b/vm/slot_visitor.hpp
index 85f5b4fe11cbdcf516f4f386673ad0dbbf01cf9d..03719e67440546e3534f618cc496c34f6d553f0e 100755 (executable)
--- a/vm/slot_visitor.hpp
+++ b/vm/slot_visitor.hpp
@@ -306,19 +306,22 @@ struct call_frame_slot_visitor {
                     [entry_point]
                     [size]
        */
-       void operator()(void *frame_top, code_block *owner, void *addr)
+       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();
+               code_block *compiled = Fixup::translated_code_block_map
+                       ? owner
+                       : visitor->fixup.translate_code(owner);
+               gc_info *info = compiled->block_gc_info();
 
-               FACTOR_ASSERT(return_address < owner->size());
+               FACTOR_ASSERT(return_address < compiled->size());
                cell callsite = info->return_address_index(return_address);
                if(callsite == (cell)-1)
                        return;
 
 #ifdef DEBUG_GC_MAPS
-               std::cout << "call frame code block " << owner << " with offset " << return_address << std::endl;
+               std::cout << "call frame code block " << compiled << " with offset " << return_address << std::endl;
 #endif
                cell *stack_pointer = (cell *)frame_top;
                u8 *bitmap = info->gc_info_bitmap();