From 9236c05e604e3a8b7352c6c39fa0fc1732dd5b6b Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Tue, 6 Dec 2011 09:14:56 -0800 Subject: [PATCH] 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. --- vm/callstack.hpp | 33 ++++++++++++++++++++++----------- vm/code_block_visitor.hpp | 11 +++++++---- vm/code_heap.cpp | 21 +++++++-------------- vm/collector.hpp | 2 ++ vm/compaction.cpp | 10 ++++++++-- vm/debug.cpp | 4 ++-- vm/fixup.hpp | 2 ++ vm/image.cpp | 2 ++ vm/mark_bits.hpp | 3 +-- vm/slot_visitor.hpp | 11 +++++++---- 10 files changed, 60 insertions(+), 39 deletions(-) diff --git a/vm/callstack.hpp b/vm/callstack.hpp index bd16117b40..acbd29a6f0 100755 --- 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 void factor_vm::iterate_callstack_object(callstack *stack_, - Iterator &iterator) +template +void factor_vm::iterate_callstack_object(callstack *stack_, Iterator &iterator) { data_root 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 96881ed37f..53cd1befbb 100644 --- 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 12f903f66a..112769da7a 100755 --- 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 4a9eec5967..454627f2ac 100644 --- a/vm/collector.hpp +++ b/vm/collector.hpp @@ -4,6 +4,8 @@ namespace factor struct must_start_gc_again {}; template 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 2b96e68002..49cbde2bf2 100644 --- 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 *data_forwarding_map; mark_bits *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 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_forwarding_map; const code_block **code_finger; diff --git a/vm/debug.cpp b/vm/debug.cpp index 026ac7fd5e..02896ae8de 100755 --- 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 c92661a03b..3c768aff8d 100644 --- 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 806db02ca4..a1a930b325 100755 --- 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 b546f69d6a..9a65f3a9d1 100644 --- a/vm/mark_bits.hpp +++ b/vm/mark_bits.hpp @@ -126,8 +126,7 @@ template 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 position = bitmap_deref(original); cell offset = (cell)original & (data_alignment - 1); diff --git a/vm/slot_visitor.hpp b/vm/slot_visitor.hpp index 85f5b4fe11..03719e6744 100755 --- 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();