vm: during tenuring stage of aging collection, if tenured space fills up, it would attempt a to_tenured collection. this will succeed if all roots were tenured. however, this is unsound, because there's now an untraced segment of tenured space. fix: if tenuring fails, go on to do a full collection instead

vm/aging_collector.cpp

@@ -4,12 +4,21 @@ namespace factor
 {
 
 aging_collector::aging_collector(factor_vm *myvm_) :
-	copying_collector<aging_space,aging_policy>
+	copying_collector<aging_space,aging_policy>(
-	(myvm_,myvm_->data->aging,aging_policy(myvm_)) {}
+		myvm_,
+		&myvm_->gc_stats.aging_stats,
+		myvm_->data->aging,
+		aging_policy(myvm_)) {}
 
 void factor_vm::collect_aging()
 {
 	{
+		/* Change the op so that if we fail here, we proceed to a full
+		tenured collection. We are collecting to tenured space, and
+		cards were unmarked, so we can't proceed with a to_tenured
+		collection. */
+		current_gc->op = collect_to_tenured_op;
+
 		to_tenured_collector collector(this);
 		collector.trace_cards(data->tenured,
 			card_points_to_aging,
@@ -17,6 +26,9 @@ void factor_vm::collect_aging()
 		collector.cheneys_algorithm();
 	}
 	{
+		/* If collection fails here, do a to_tenured collection. */
+		current_gc->op = collect_aging_op;
+
 		std::swap(data->aging,data->aging_semispace);
 		reset_generation(data->aging);
 

vm/code_heap.cpp

@@ -229,7 +229,7 @@ critical here */
 void factor_vm::compact_code_heap()
 {
 	/* Free all unreachable code blocks, don't trace contexts */
-	garbage_collection(tenured_gen,false,false,0);
+	garbage_collection(collect_full_op,false,0);
 
 	/* Figure out where the code heap blocks are going to end up */
 	cell size = code->compute_heap_forwarding();

vm/collector.hpp

@@ -6,14 +6,16 @@ template<typename TargetGeneration, typename Policy> struct collector {
 	data_heap *data;
 	code_heap *code;
 	gc_state *current_gc;
+	generation_statistics *stats;
 	TargetGeneration *target;
 	Policy policy;
 
-	explicit collector(factor_vm *myvm_, TargetGeneration *target_, Policy policy_) :
+	explicit collector(factor_vm *myvm_, generation_statistics *stats_, TargetGeneration *target_, Policy policy_) :
 		myvm(myvm_),
 		data(myvm_->data),
 		code(myvm_->code),
 		current_gc(myvm_->current_gc),
+		stats(stats_),
 		target(target_),
 		policy(policy_) {}
 
@@ -74,7 +76,6 @@ template<typename TargetGeneration, typename Policy> struct collector {
 		memcpy(newpointer,untagged,size);
 		untagged->h.forward_to(newpointer);
 
-		generation_statistics *stats = &myvm->gc_stats.generations[current_gc->collecting_gen];
 		stats->object_count++;
 		stats->bytes_copied += size;
 

vm/copying_collector.hpp

@@ -15,8 +15,8 @@ template<typename TargetGeneration, typename Policy>
 struct copying_collector : collector<TargetGeneration,Policy> {
 	cell scan;
 
-	explicit copying_collector(factor_vm *myvm_, TargetGeneration *target_, Policy policy_) :
+	explicit copying_collector(factor_vm *myvm_, generation_statistics *stats_, TargetGeneration *target_, Policy policy_) :
-		collector<TargetGeneration,Policy>(myvm_,target_,policy_), scan(target_->here) {}
+		collector<TargetGeneration,Policy>(myvm_,stats_,target_,policy_), scan(target_->here) {}
 
 	inline cell first_card_in_deck(cell deck)
 	{
@@ -28,11 +28,6 @@ struct copying_collector : collector<TargetGeneration,Policy> {
 		return first_card_in_deck(deck + 1);
 	}
 
-	inline cell card_to_addr(cell c)
-	{
-		return c << card_bits + this->data->start;
-	}
-
 	inline cell card_deck_for_address(cell a)
 	{
 		return addr_to_deck(a - this->data->start);

vm/data_heap.hpp

@@ -27,9 +27,4 @@ struct data_heap {
 	data_heap *grow(cell requested_size);
 };
 
-static const cell nursery_gen = 0;
-static const cell aging_gen = 1;
-static const cell tenured_gen = 2;
-static const cell gen_count = 3;
-
 }

vm/debug.cpp

@@ -211,9 +211,9 @@ void factor_vm::dump_memory(cell from, cell to)
 		dump_cell(from);
 }
 
-void factor_vm::dump_zone(cell gen, zone *z)
+void factor_vm::dump_zone(char *name, zone *z)
 {
-	print_string("Generation "); print_cell(gen); print_string(": ");
+	print_string(name); print_string(": ");
 	print_string("Start="); print_cell(z->start);
 	print_string(", size="); print_cell(z->size);
 	print_string(", here="); print_cell(z->here - z->start); nl();
@@ -221,9 +221,9 @@ void factor_vm::dump_zone(cell gen, zone *z)
 
 void factor_vm::dump_generations()
 {
-	dump_zone(nursery_gen,&nursery);
+	dump_zone("Nursery",&nursery);
-	dump_zone(aging_gen,data->aging);
+	dump_zone("Aging",data->aging);
-	dump_zone(tenured_gen,data->tenured);
+	dump_zone("Tenured",data->tenured);
 
 	print_string("Cards: base=");
 	print_cell((cell)data->cards);

vm/full_collector.cpp

@@ -4,7 +4,11 @@ namespace factor
 {
 
 full_collector::full_collector(factor_vm *myvm_) :
-	copying_collector<tenured_space,full_policy>(myvm_,myvm_->data->tenured,full_policy(myvm_)) {}
+	copying_collector<tenured_space,full_policy>(
+		myvm_,
+		&myvm_->gc_stats.full_stats,
+		myvm_->data->tenured,
+		full_policy(myvm_)) {}
 
 struct stack_frame_marker {
 	factor_vm *myvm;

vm/gc.cpp

@@ -3,14 +3,9 @@
 namespace factor
 {
 
-gc_state::gc_state(data_heap *data_, bool growing_data_heap_, cell collecting_gen_) :
+gc_state::gc_state(gc_op op_) : op(op_), start_time(current_micros()) {}
-	data(data_),
-	growing_data_heap(growing_data_heap_),
-	collecting_gen(collecting_gen_),
-        collecting_aging_again(false),
-	start_time(current_micros()) { }
 
-gc_state::~gc_state() { }
+gc_state::~gc_state() {}
 
 void factor_vm::update_dirty_code_blocks(std::set<code_block *> *remembered_set)
 {
@@ -21,80 +16,81 @@ void factor_vm::update_dirty_code_blocks(std::set<code_block *> *remembered_set)
 	for(; iter != end; iter++) update_literal_references(*iter);
 }
 
-void factor_vm::record_gc_stats()
+void factor_vm::record_gc_stats(generation_statistics *stats)
 {
-	generation_statistics *s = &gc_stats.generations[current_gc->collecting_gen];
-
 	cell gc_elapsed = (current_micros() - current_gc->start_time);
-	s->collections++;
+	stats->collections++;
-	s->gc_time += gc_elapsed;
+	stats->gc_time += gc_elapsed;
-	if(s->max_gc_time < gc_elapsed)
+	if(stats->max_gc_time < gc_elapsed)
-		s->max_gc_time = gc_elapsed;
+		stats->max_gc_time = gc_elapsed;
 }
 
 /* Collect gen and all younger generations.
 If growing_data_heap_ is true, we must grow the data heap to such a size that
 an allocation of requested_bytes won't fail */
-void factor_vm::garbage_collection(cell collecting_gen_, bool growing_data_heap_, bool trace_contexts_p, cell requested_bytes)
+void factor_vm::garbage_collection(gc_op op, bool trace_contexts_p, cell requested_bytes)
 {
 	assert(!gc_off);
 	assert(!current_gc);
 
 	save_stacks();
 
-	current_gc = new gc_state(data,growing_data_heap_,collecting_gen_);
+	current_gc = new gc_state(op);
 
 	/* Keep trying to GC higher and higher generations until we don't run out
 	of space */
 	if(setjmp(current_gc->gc_unwind))
 	{
 		/* We come back here if a generation is full */
-
+		switch(current_gc->op)
-		/* We have no older generations we can try collecting, so we
-		resort to growing the data heap */
-		if(current_gc->collecting_tenured_p())
 		{
-			assert(!current_gc->growing_data_heap);
+		case collect_nursery_op:
-			current_gc->growing_data_heap = true;
+			current_gc->op = collect_aging_op;
-
+			break;
+		case collect_aging_op:
+			current_gc->op = collect_to_tenured_op;
+			break;
+		case collect_to_tenured_op:
+			current_gc->op = collect_full_op;
+			break;
+		case collect_full_op:
 			/* Since we start tracing again, any previously
 			marked code blocks must be re-marked and re-traced */
 			code->clear_mark_bits();
-		}
+			current_gc->op = collect_growing_heap_op;
-		/* we try collecting aging space twice before going on to
+			break;
-		collect tenured */
+		default:
-		else if(current_gc->collecting_aging_p()
+			critical_error("Bad GC op\n",op);
-			&& !current_gc->collecting_aging_again)
+			break;
-		{
-			current_gc->collecting_aging_again = true;
-		}
-		/* Collect the next oldest generation */
-		else
-		{
-			current_gc->collecting_gen++;
 		}
 	}
 
-	if(current_gc->collecting_nursery_p())
+	switch(current_gc->op)
+	{
+	case collect_nursery_op:
 		collect_nursery();
-	else if(current_gc->collecting_aging_p())
+		record_gc_stats(&gc_stats.nursery_stats);
-	{
+		break;
-		if(current_gc->collecting_aging_again)
+	case collect_aging_op:
-			collect_to_tenured();
-		else
 		collect_aging();
-	}
+		record_gc_stats(&gc_stats.aging_stats);
-        else if(current_gc->collecting_tenured_p())
+		break;
-	{
+	case collect_to_tenured_op:
-		if(current_gc->growing_data_heap)
+		collect_to_tenured();
-			collect_growing_heap(requested_bytes,trace_contexts_p);
+		record_gc_stats(&gc_stats.aging_stats);
-		else
+		break;
+	case collect_full_op:
 		collect_full(trace_contexts_p);
+		record_gc_stats(&gc_stats.full_stats);
+		break;
+	case collect_growing_heap_op:
+		collect_growing_heap(requested_bytes,trace_contexts_p);
+		record_gc_stats(&gc_stats.full_stats);
+		break;
+	default:
+		critical_error("Bad GC op\n",op);
+		break;
 	}
-	else
-		critical_error("Bug in GC",0);
-
-	record_gc_stats();
 
 	delete current_gc;
 	current_gc = NULL;
@@ -102,7 +98,7 @@ void factor_vm::garbage_collection(cell collecting_gen_, bool growing_data_heap_
 
 void factor_vm::gc()
 {
-	garbage_collection(tenured_gen,false,true,0);
+	garbage_collection(collect_full_op,true,0);
 }
 
 void factor_vm::primitive_gc()
@@ -110,25 +106,28 @@ void factor_vm::primitive_gc()
 	gc();
 }
 
+void factor_vm::add_gc_stats(generation_statistics *stats, growable_array *result)
+{
+	result->add(allot_cell(stats->collections));
+	result->add(tag<bignum>(long_long_to_bignum(stats->gc_time)));
+	result->add(tag<bignum>(long_long_to_bignum(stats->max_gc_time)));
+	result->add(allot_cell(stats->collections == 0 ? 0 : stats->gc_time / stats->collections));
+	result->add(allot_cell(stats->object_count));
+	result->add(tag<bignum>(long_long_to_bignum(stats->bytes_copied)));
+}
+
 void factor_vm::primitive_gc_stats()
 {
 	growable_array result(this);
 
-	cell i;
+	add_gc_stats(&gc_stats.nursery_stats,&result);
-	u64 total_gc_time = 0;
+	add_gc_stats(&gc_stats.aging_stats,&result);
+	add_gc_stats(&gc_stats.full_stats,&result);
 
-	for(i = 0; i < gen_count; i++)
+	u64 total_gc_time =
-	{
+		gc_stats.nursery_stats.gc_time +
-		generation_statistics *s = &gc_stats.generations[i];
+		gc_stats.aging_stats.gc_time +
-		result.add(allot_cell(s->collections));
+		gc_stats.full_stats.gc_time;
-		result.add(tag<bignum>(long_long_to_bignum(s->gc_time)));
-		result.add(tag<bignum>(long_long_to_bignum(s->max_gc_time)));
-		result.add(allot_cell(s->collections == 0 ? 0 : s->gc_time / s->collections));
-		result.add(allot_cell(s->object_count));
-		result.add(tag<bignum>(long_long_to_bignum(s->bytes_copied)));
-
-		total_gc_time += s->gc_time;
-	}
 
 	result.add(tag<bignum>(ulong_long_to_bignum(total_gc_time)));
 	result.add(tag<bignum>(ulong_long_to_bignum(gc_stats.cards_scanned)));
@@ -186,7 +185,7 @@ void factor_vm::inline_gc(cell *gc_roots_base, cell gc_roots_size)
 	for(cell i = 0; i < gc_roots_size; i++)
 		gc_locals.push_back((cell)&gc_roots_base[i]);
 
-	garbage_collection(nursery_gen,false,true,0);
+	garbage_collection(collect_nursery_op,true,0);
 
 	for(cell i = 0; i < gc_roots_size; i++)
 		gc_locals.pop_back();
@@ -215,7 +214,7 @@ object *factor_vm::allot_object(header header, cell size)
 	{
 		/* If there is insufficient room, collect the nursery */
 		if(nursery.here + size > nursery.end)
-			garbage_collection(nursery_gen,false,true,0);
+			garbage_collection(collect_nursery_op,true,0);
 
 		obj = nursery.allot(size);
 	}
@@ -229,7 +228,7 @@ object *factor_vm::allot_object(header header, cell size)
 
 		/* If it still won't fit, grow the heap */
 		if(data->tenured->here + size > data->tenured->end)
-			garbage_collection(tenured_gen,true,true,size);
+			garbage_collection(collect_growing_heap_op,true,size);
 
 		obj = data->tenured->allot(size);
 

vm/gc.hpp

@@ -1,6 +1,14 @@
 namespace factor
 {
 
+enum gc_op {
+	collect_nursery_op,
+	collect_aging_op,
+	collect_to_tenured_op,
+	collect_full_op,
+	collect_growing_heap_op
+};
+
 /* statistics */
 struct generation_statistics {
 	cell collections;
@@ -11,7 +19,9 @@ struct generation_statistics {
 };
 
 struct gc_statistics {
-	generation_statistics generations[gen_count];
+	generation_statistics nursery_stats;
+	generation_statistics aging_stats;
+	generation_statistics full_stats;
 	u64 cards_scanned;
 	u64 decks_scanned;
 	u64 card_scan_time;
@@ -19,47 +29,12 @@ struct gc_statistics {
 };
 
 struct gc_state {
-	/* The data heap we're collecting */
+	gc_op op;
-	data_heap *data;
-
-	/* sometimes we grow the heap */
-	bool growing_data_heap;
-
-	/* Which generation is being collected */
-	cell collecting_gen;
-
-	/* If true, we are collecting aging space for the second time, so if it is still
-	   full, we go on to collect tenured */
-	bool collecting_aging_again;
-
-	/* GC start time, for benchmarking */
 	u64 start_time;
-
         jmp_buf gc_unwind;
 
-	explicit gc_state(data_heap *data_, bool growing_data_heap_, cell collecting_gen_);
+	explicit gc_state(gc_op op_);
 	~gc_state();
-
-	inline bool collecting_nursery_p()
-	{
-		return collecting_gen == nursery_gen;
-	}
-
-	inline bool collecting_aging_p()
-	{
-		return collecting_gen == aging_gen;
-	}
-
-	inline bool collecting_tenured_p()
-	{
-		return collecting_gen == tenured_gen;
-	}
-
-	inline bool collecting_accumulation_gen_p()
-	{
-		return ((collecting_aging_p() && !collecting_aging_again)
-			|| collecting_tenured_p());
-	}
 };
 
 VM_C_API void inline_gc(cell *gc_roots_base, cell gc_roots_size, factor_vm *myvm);

vm/nursery_collector.cpp

@@ -4,8 +4,11 @@ namespace factor
 {
 
 nursery_collector::nursery_collector(factor_vm *myvm_) :
-	copying_collector<aging_space,nursery_policy>
+	copying_collector<aging_space,nursery_policy>(
-	(myvm_,myvm_->data->aging,nursery_policy(myvm_)) {}
+		myvm_,
+		&myvm_->gc_stats.nursery_stats,
+		myvm_->data->aging,
+		nursery_policy(myvm_)) {}
 
 void factor_vm::collect_nursery()
 {

vm/to_tenured_collector.cpp

@@ -4,8 +4,11 @@ namespace factor
 {
 
 to_tenured_collector::to_tenured_collector(factor_vm *myvm_) :
-	copying_collector<tenured_space,to_tenured_policy>
+	copying_collector<tenured_space,to_tenured_policy>(
-	(myvm_,myvm_->data->tenured,to_tenured_policy(myvm_)) {}
+		myvm_,
+		&myvm_->gc_stats.aging_stats,
+		myvm_->data->tenured,
+		to_tenured_policy(myvm_)) {}
 
 void factor_vm::collect_to_tenured()
 {

vm/vm.hpp

@@ -1,6 +1,8 @@
 namespace factor
 {
 
+struct growable_array;
+
 struct factor_vm
 {
 	// First five fields accessed directly by assembler. See vm.factor
@@ -240,8 +242,8 @@ struct factor_vm
 	void collect_full_impl(bool trace_contexts_p);
 	void collect_growing_heap(cell requested_bytes, bool trace_contexts_p);
 	void collect_full(bool trace_contexts_p);
-	void record_gc_stats();
+	void record_gc_stats(generation_statistics *stats);
-	void garbage_collection(cell gen, bool growing_data_heap, bool trace_contexts_p, cell requested_bytes);
+	void garbage_collection(gc_op op, bool trace_contexts_p, cell requested_bytes);
 	void gc();
 	void primitive_gc();
 	void primitive_gc_stats();
@@ -249,6 +251,7 @@ struct factor_vm
 	void primitive_become();
 	void inline_gc(cell *gc_roots_base, cell gc_roots_size);
 	object *allot_object(header header, cell size);
+	void add_gc_stats(generation_statistics *stats, growable_array *result);
 	void primitive_clear_gc_stats();
 
 	template<typename Type> Type *allot(cell size)
@@ -259,7 +262,7 @@ struct factor_vm
 	inline void check_data_pointer(object *pointer)
 	{
 	#ifdef FACTOR_DEBUG
-		if(!(current_gc && current_gc->growing_data_heap))
+		if(!(current_gc && current_gc->op == collect_growing_heap_op))
 		{
 			assert((cell)pointer >= data->seg->start
 			       && (cell)pointer < data->seg->end);
@@ -298,7 +301,7 @@ struct factor_vm
 	void print_callstack();
 	void dump_cell(cell x);
 	void dump_memory(cell from, cell to);
-	void dump_zone(cell gen, zone *z);
+	void dump_zone(char *name, zone *z);
 	void dump_generations();
 	void dump_objects(cell type);
 	void find_data_references_step(cell *scan);