From 278d66bdc1ffadf3050ccd926d53fdc311c8ae77 Mon Sep 17 00:00:00 2001
From: Bob Nystrom <robert@stuffwithstuff.com>
Date: Mon, 28 Dec 2015 05:43:08 +0000
Subject: [PATCH] feat: use dedicated fiber for foreign finalizer stack to
 avoid allocation during GC

The finalizer fiber is pre-allocated in the VM and reused for each finalizer call, replacing the previous approach that used a raw stack pointer. This ensures no memory allocation occurs during garbage collection when finalizers are invoked. The change also properly resets the fiber after each finalizer call and updates the GC rooting to include the new fiber.
---
 Makefile            |  2 +-
 src/vm/wren_value.c |  8 +++++---
 src/vm/wren_vm.c    | 30 ++++++++++++++----------------
 src/vm/wren_vm.h    |  6 ++++++
 4 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/Makefile b/Makefile
index c402486a..493b5868 100644
--- a/Makefile
+++ b/Makefile
@@ -48,7 +48,7 @@ test: debug
 
 benchmark: release
 	@ $(MAKE) -f util/wren.mk test
-	@ ./util/benchmark.py $(suite)
+	@ ./util/benchmark.py -l wren $(suite)
 
 # Generate the Wren site.
 docs:
diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c
index ea576b9a..ab3c8f1f 100644
--- a/src/vm/wren_value.c
+++ b/src/vm/wren_value.c
@@ -152,7 +152,9 @@ ObjFiber* wrenNewFiber(WrenVM* vm, Obj* fn)
   
   // Add one slot for the unused implicit receiver slot that the compiler
   // assumes all functions have.
-  int stackCapacity = wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots + 1);
+  int stackCapacity = fn == NULL
+      ? 1
+      : wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots + 1);
   Value* stack = ALLOCATE_ARRAY(vm, Value, stackCapacity);
   
   ObjFiber* fiber = ALLOCATE(vm, ObjFiber);
@@ -174,10 +176,10 @@ void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, Obj* fn)
   fiber->caller = NULL;
   fiber->error = NULL_VAL;
   fiber->callerIsTrying = false;
+  fiber->numFrames = 0;
 
   // Initialize the first call frame.
-  fiber->numFrames = 0;
-  wrenAppendCallFrame(vm, fiber, fn, fiber->stack);
+  if (fn != NULL) wrenAppendCallFrame(vm, fiber, fn, fiber->stack);
 }
 
 ObjForeign* wrenNewForeign(WrenVM* vm, ObjClass* classObj, size_t size)
diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c
index e517fb65..83ee85e5 100644
--- a/src/vm/wren_vm.c
+++ b/src/vm/wren_vm.c
@@ -65,6 +65,8 @@ WrenVM* wrenNewVM(WrenConfiguration* config)
   vm->modules = wrenNewMap(vm);
 
   wrenInitializeCore(vm);
+  
+  vm->finalizerFiber = wrenNewFiber(vm, NULL);
 
   // TODO: Lazy load these.
   #if WREN_OPT_META
@@ -137,8 +139,9 @@ void wrenCollectGarbage(WrenVM* vm)
     wrenGrayObj(vm, vm->tempRoots[i]);
   }
 
-  // The current fiber.
+  // The rooted fibers.
   wrenGrayObj(vm, (Obj*)vm->fiber);
+  wrenGrayObj(vm, (Obj*)vm->finalizerFiber);
 
   // The value handles.
   for (WrenValue* value = vm->valueHandles;
@@ -740,10 +743,16 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign)
   ASSERT(method->type == METHOD_FOREIGN, "Finalizer should be foreign.");
 
   // Pass the foreign object to the finalizer.
-  Value slot = OBJ_VAL(foreign);
-  vm->foreignStackStart = &slot;
+  *vm->finalizerFiber->stackTop++ = OBJ_VAL(foreign);
+  ObjFiber* previousFiber = vm->fiber;
+  vm->fiber = vm->finalizerFiber;
+  vm->foreignStackStart = vm->finalizerFiber->stack;
 
   method->fn.foreign(vm);
+  
+  vm->fiber = previousFiber;
+  vm->foreignStackStart = NULL;
+  wrenResetFiber(vm, vm->finalizerFiber, NULL);
 }
 
 // The main bytecode interpreter loop. This is where the magic happens. It is
@@ -1637,28 +1646,17 @@ void wrenPopRoot(WrenVM* vm)
   vm->numTempRoots--;
 }
 
-// Returns true if the VM is in a foreign method that's a finalizer.
-//
-// Finalizers don't run in the context of a fiber and have a single magic stack
-// slot, so need to be handled a little specially.
-static bool isInFinalizer(WrenVM* vm)
-{
-  return vm->fiber == NULL ||
-         vm->foreignStackStart < vm->fiber->stack ||
-         vm->foreignStackStart > vm->fiber->stackTop;
-}
-
 int wrenGetSlotCount(WrenVM* vm)
 {
   ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call.");
 
-  if (isInFinalizer(vm)) return 1;
   return (int)(vm->fiber->stackTop - vm->foreignStackStart);
 }
 
 void wrenEnsureSlots(WrenVM* vm, int numSlots)
 {
-  ASSERT(!isInFinalizer(vm), "Cannot grow the stack in a finalizer.");
+  ASSERT(vm->fiber != vm->finalizerFiber,
+         "Cannot grow the stack in a finalizer.");
   
   int currentSize = (int)(vm->fiber->stackTop - vm->foreignStackStart);
   if (currentSize >= numSlots) return;
diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h
index 1d0b0776..5508e7d3 100644
--- a/src/vm/wren_vm.h
+++ b/src/vm/wren_vm.h
@@ -102,6 +102,12 @@ struct WrenVM
   // There is a single global symbol table for all method names on all classes.
   // Method calls are dispatched directly by index in this table.
   SymbolTable methodNames;
+  
+  // A special fiber whose stack is used for communicating with foreign
+  // finalizer methods. Finalizers run during GC, so we don't want to do any
+  // allocation in order to execute one, so we set up this fiber ahead of time
+  // to ensure it's available.
+  ObjFiber* finalizerFiber;
 };
 
 // A generic allocation function that handles all explicit memory management.