From cea71c2fe4690485ca3cb1e3e613c2bc67ff0f90 Mon Sep 17 00:00:00 2001 From: Will Speak Date: Mon, 19 Oct 2015 22:45:15 +0100 Subject: [PATCH] Remove Recursive Mark from GC The previous GC implementation used a recursive mark method. This can result in stack overflows when attempting to mark deeply nested objects. This commit replaces the recursive approach with an iteritive one, moving the state stack from the C call stack to the `WrenVM` structure. As objects are 'grayed' they are pushed onto the VM's gray stack. When we have grayed all of the root objects we iterate until the stack is empty graying any obejcts which haven't been marked as dark before. At the end of the process we clean up all unmarked objects as before. This commit also adds a few new tests which check garbage collection by allocating some new deeply nested objects and triggering the GC a few times in the process. --- src/vm/wren_value.c | 74 +++++++++++++++++---------- src/vm/wren_value.h | 7 ++- src/vm/wren_vm.c | 26 ++++++++-- src/vm/wren_vm.h | 6 +++ test/language/deeply_nested_gc.wren | 7 +++ test/language/many_reallocations.wren | 14 +++++ 6 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 test/language/deeply_nested_gc.wren create mode 100644 test/language/many_reallocations.wren diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index 451d81d4..b1f660f4 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -37,7 +37,7 @@ DEFINE_BUFFER(Method, Method); static void initObj(WrenVM* vm, Obj* obj, ObjType type, ObjClass* classObj) { obj->type = type; - obj->marked = false; + obj->isDark = false; obj->classObj = classObj; obj->next = vm->first; vm->first = obj; @@ -858,21 +858,21 @@ ObjUpvalue* wrenNewUpvalue(WrenVM* vm, Value* value) static void markClass(WrenVM* vm, ObjClass* classObj) { // The metaclass. - wrenMarkObj(vm, (Obj*)classObj->obj.classObj); + wrenGrayObj(vm, (Obj*)classObj->obj.classObj); // The superclass. - wrenMarkObj(vm, (Obj*)classObj->superclass); + wrenGrayObj(vm, (Obj*)classObj->superclass); // Method function objects. for (int i = 0; i < classObj->methods.count; i++) { if (classObj->methods.data[i].type == METHOD_BLOCK) { - wrenMarkObj(vm, classObj->methods.data[i].fn.obj); + wrenGrayObj(vm, classObj->methods.data[i].fn.obj); } } - wrenMarkObj(vm, (Obj*)classObj->name); + wrenGrayObj(vm, (Obj*)classObj->name); // Keep track of how much memory is still in use. vm->bytesAllocated += sizeof(ObjClass); @@ -882,12 +882,12 @@ static void markClass(WrenVM* vm, ObjClass* classObj) static void markClosure(WrenVM* vm, ObjClosure* closure) { // Mark the function. - wrenMarkObj(vm, (Obj*)closure->fn); + wrenGrayObj(vm, (Obj*)closure->fn); // Mark the upvalues. for (int i = 0; i < closure->fn->numUpvalues; i++) { - wrenMarkObj(vm, (Obj*)closure->upvalues[i]); + wrenGrayObj(vm, (Obj*)closure->upvalues[i]); } // Keep track of how much memory is still in use. @@ -900,7 +900,7 @@ static void markFiber(WrenVM* vm, ObjFiber* fiber) // Stack functions. for (int i = 0; i < fiber->numFrames; i++) { - wrenMarkObj(vm, fiber->frames[i].fn); + wrenGrayObj(vm, fiber->frames[i].fn); } // Stack variables. @@ -913,12 +913,12 @@ static void markFiber(WrenVM* vm, ObjFiber* fiber) ObjUpvalue* upvalue = fiber->openUpvalues; while (upvalue != NULL) { - wrenMarkObj(vm, (Obj*)upvalue); + wrenGrayObj(vm, (Obj*)upvalue); upvalue = upvalue->next; } // The caller. - wrenMarkObj(vm, (Obj*)fiber->caller); + wrenGrayObj(vm, (Obj*)fiber->caller); wrenMarkValue(vm, fiber->error); // Keep track of how much memory is still in use. @@ -954,7 +954,7 @@ static void markForeign(WrenVM* vm, ObjForeign* foreign) static void markInstance(WrenVM* vm, ObjInstance* instance) { - wrenMarkObj(vm, (Obj*)instance->obj.classObj); + wrenGrayObj(vm, (Obj*)instance->obj.classObj); // Mark the fields. for (int i = 0; i < instance->obj.classObj->numFields; i++) @@ -1002,7 +1002,7 @@ static void markModule(WrenVM* vm, ObjModule* module) wrenMarkValue(vm, module->variables.data[i]); } - wrenMarkObj(vm, (Obj*)module->name); + wrenGrayObj(vm, (Obj*)module->name); // Keep track of how much memory is still in use. vm->bytesAllocated += sizeof(ObjModule); @@ -1030,20 +1030,9 @@ static void markUpvalue(WrenVM* vm, ObjUpvalue* upvalue) vm->bytesAllocated += sizeof(ObjUpvalue); } -void wrenMarkObj(WrenVM* vm, Obj* obj) +static void darkenObject(WrenVM* vm, Obj* obj) { - if (obj == NULL) return; - - // Stop if the object is already marked so we don't get stuck in a cycle. - if (obj->marked) return; - - // It's been reached. - obj->marked = true; - #if WREN_DEBUG_TRACE_MEMORY - static int indent = 0; - indent++; - for (int i = 0; i < indent; i++) printf(" "); printf("mark "); wrenDumpValue(OBJ_VAL(obj)); printf(" @ %p\n", obj); @@ -1065,16 +1054,45 @@ void wrenMarkObj(WrenVM* vm, Obj* obj) case OBJ_STRING: markString( vm, (ObjString*) obj); break; case OBJ_UPVALUE: markUpvalue( vm, (ObjUpvalue*) obj); break; } +} -#if WREN_DEBUG_TRACE_MEMORY - indent--; -#endif +void wrenDarkenObjs(WrenVM* vm) +{ + do + { + // pop an item from the gray stack + Obj* obj = vm->gray[--vm->grayDepth]; + darkenObject(vm, obj); + } while (vm->grayDepth > 0); +} + +void wrenGrayObj(WrenVM* vm, Obj* obj) +{ + if (obj == NULL) return; + + // Stop if the object is already marked so we don't get stuck in a cycle. + if (obj->isDark) return; + + // It's been reached. + obj->isDark = true; + + // Add it to the gray list so it can be recursively explored for + // more marks later. + if (vm->grayDepth >= vm->maxGray) + { + size_t oldSize = vm->maxGray * sizeof(Obj*); + size_t newSize = vm->grayDepth * 2 * sizeof(Obj*); + vm->gray = (Obj**)wrenReallocate(vm, vm->gray, oldSize, newSize); + vm->maxGray = vm->grayDepth * 2; + } + + vm->gray[vm->grayDepth++] = obj; } void wrenMarkValue(WrenVM* vm, Value value) { if (!IS_OBJ(value)) return; - wrenMarkObj(vm, AS_OBJ(value)); + wrenGrayObj(vm, AS_OBJ(value)); } void wrenMarkBuffer(WrenVM* vm, ValueBuffer* buffer) diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index fd0d054e..25207d7d 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -107,7 +107,7 @@ typedef struct sObjClass ObjClass; typedef struct sObj { ObjType type; - bool marked; + bool isDark; // The object's class. ObjClass* classObj; @@ -747,12 +747,15 @@ void wrenMarkValue(WrenVM* vm, Value value); // Mark [obj] as reachable and still in use. This should only be called // during the sweep phase of a garbage collection. -void wrenMarkObj(WrenVM* vm, Obj* obj); +void wrenGrayObj(WrenVM* vm, Obj* obj); // Mark the values in [buffer] as reachable and still in use. This should only // be called during the sweep phase of a garbage collection. void wrenMarkBuffer(WrenVM* vm, ValueBuffer* buffer); +// Expand the makred objects to all those reacable from the current state. +void wrenDarkenObjs(WrenVM* vm); + // Releases all memory owned by [obj], including [obj] itself. void wrenFreeObj(WrenVM* vm, Obj* obj); diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 4ac1b3c1..a9727fe3 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -46,6 +46,13 @@ void wrenInitConfiguration(WrenConfiguration* config) config->heapGrowthPercent = 50; } +static void initializeGC(WrenVM* vm) +{ + vm->gray = (Obj**)wrenReallocate(vm, NULL, 0, 5 * sizeof(Obj*)); + vm->grayDepth = 0; + vm->maxGray = 5; +} + WrenVM* wrenNewVM(WrenConfiguration* config) { WrenVM* vm = (WrenVM*)config->reallocateFn(NULL, sizeof(*vm)); @@ -60,6 +67,8 @@ WrenVM* wrenNewVM(WrenConfiguration* config) wrenInitializeCore(vm); + initializeGC(vm); + // TODO: Lazy load these. #if WREN_OPT_META wrenLoadMetaModule(vm); @@ -84,6 +93,9 @@ void wrenFreeVM(WrenVM* vm) obj = next; } + // Free up the GC gray set + wrenReallocate(vm, vm->gray, vm->maxGray * sizeof(Obj*), 0); + // Tell the user if they didn't free any handles. We don't want to just free // them here because the host app may still have pointers to them that they // may try to use. Better to tell them about the bug early. @@ -120,16 +132,16 @@ void wrenCollectGarbage(WrenVM* vm) // already been freed. vm->bytesAllocated = 0; - wrenMarkObj(vm, (Obj*)vm->modules); + wrenGrayObj(vm, (Obj*)vm->modules); // Temporary roots. for (int i = 0; i < vm->numTempRoots; i++) { - wrenMarkObj(vm, vm->tempRoots[i]); + wrenGrayObj(vm, vm->tempRoots[i]); } // The current fiber. - wrenMarkObj(vm, (Obj*)vm->fiber); + wrenGrayObj(vm, (Obj*)vm->fiber); // The value handles. for (WrenValue* value = vm->valueHandles; @@ -142,11 +154,15 @@ void wrenCollectGarbage(WrenVM* vm) // Any object the compiler is using (if there is one). if (vm->compiler != NULL) wrenMarkCompiler(vm, vm->compiler); + // Now we have makred all of the root objects expand the marks out + // to all of the live objects. + wrenDarkenObjs(vm); + // Collect any unmarked objects. Obj** obj = &vm->first; while (*obj != NULL) { - if (!((*obj)->marked)) + if (!((*obj)->isDark)) { // This object wasn't reached, so remove it from the list and free it. Obj* unreached = *obj; @@ -157,7 +173,7 @@ void wrenCollectGarbage(WrenVM* vm) { // This object was reached, so unmark it (for the next GC) and move on to // the next. - (*obj)->marked = false; + (*obj)->isDark = false; obj = &(*obj)->next; } } diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index dd73ff72..a5b420fe 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -69,6 +69,12 @@ struct WrenVM // The first object in the linked list of all currently allocated objects. Obj* first; + // The 'gray' set for the garbage collector. This is the stack of unprocessed + // objects while a garbace collection pass is in process. + Obj** gray; + int grayDepth; + int maxGray; + // The list of temporary roots. This is for temporary or new objects that are // not otherwise reachable but should not be collected. // diff --git a/test/language/deeply_nested_gc.wren b/test/language/deeply_nested_gc.wren new file mode 100644 index 00000000..c40fa4c7 --- /dev/null +++ b/test/language/deeply_nested_gc.wren @@ -0,0 +1,7 @@ +var head + +for (i in 1..400000) { + head = { "next" : head } +} + +System.print("done") // expect: done \ No newline at end of file diff --git a/test/language/many_reallocations.wren b/test/language/many_reallocations.wren new file mode 100644 index 00000000..06a96285 --- /dev/null +++ b/test/language/many_reallocations.wren @@ -0,0 +1,14 @@ +var found = [] +for (i in 1..1000) { + var foo = 1337 + for (i in 1..1000) { + foo = { "a" : foo, "b": foo } + } + var bar = foo + for (i in 1..1000) { + bar = bar["a"] + } + found.add(bar) +} +System.print(found.all {|i| i == 1337}) // expect: true +System.print("DONE!") // expect: DONE!