Skip to content

Commit

Permalink
Remove the special method type for Fn.call(...).
Browse files Browse the repository at this point in the history 
Instead, using regular primitives for each of the call() methods.
Doesn't help performance (alas), but does seem cleaner to me.
  • Loading branch information
@munificent
munificent committed Jul 18, 2018
1 parent ed64a13 commit 5f29a72
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 105 deletions.
4 changes: 4 additions & 0 deletions Makefile
View file
Expand Up @@ -84,6 +84,10 @@ benchmark: release
$(V) $(MAKE) -f util/wren.mk api_test
$(V) ./util/benchmark.py -l wren $(suite)

benchmark_baseline: release
$(V) $(MAKE) -f util/wren.mk api_test
$(V) ./util/benchmark.py --generate-baseline

unit_test:
$(V) $(MAKE) -f util/wren.mk MODE=debug unit_test
$(V) ./build/debug/test/unit_wrend
Expand Down
85 changes: 55 additions & 30 deletions src/vm/wren_core.c
View file
Expand Up @@ -240,6 +240,44 @@ DEF_PRIMITIVE(fn_arity)
RETURN_NUM(AS_CLOSURE(args[0])->fn->arity);
}

static void call(WrenVM* vm, Value* args, int numArgs)
{
// We only care about missing arguments, not extras.
if (AS_CLOSURE(args[0])->fn->arity > numArgs)
{
vm->fiber->error = CONST_STRING(vm, "Function expects more arguments.");
return;
}

// +1 to include the function itself.
wrenCallFunction(vm, vm->fiber, AS_CLOSURE(args[0]), numArgs + 1);
}

#define DEF_FN_CALL(numArgs) \
DEF_PRIMITIVE(fn_call##numArgs) \
{ \
call(vm, args, numArgs); \
return false; \
} \

DEF_FN_CALL(0)
DEF_FN_CALL(1)
DEF_FN_CALL(2)
DEF_FN_CALL(3)
DEF_FN_CALL(4)
DEF_FN_CALL(5)
DEF_FN_CALL(6)
DEF_FN_CALL(7)
DEF_FN_CALL(8)
DEF_FN_CALL(9)
DEF_FN_CALL(10)
DEF_FN_CALL(11)
DEF_FN_CALL(12)
DEF_FN_CALL(13)
DEF_FN_CALL(14)
DEF_FN_CALL(15)
DEF_FN_CALL(16)

DEF_PRIMITIVE(fn_toString)
{
RETURN_VAL(CONST_STRING(vm, "<fn>"));
Expand Down Expand Up @@ -1097,19 +1135,6 @@ static ObjClass* defineClass(WrenVM* vm, ObjModule* module, const char* name)
return classObj;
}

// Defines one of the overloads of the special "call(...)" method on Fn.
//
// These methods have their own unique method type to handle pushing the
// function onto the callstack and checking its arity.
static void fnCall(WrenVM* vm, const char* signature)
{
int symbol = wrenSymbolTableEnsure(vm, &vm->methodNames, signature,
strlen(signature));
Method method;
method.type = METHOD_FN_CALL;
wrenBindMethod(vm, vm->fnClass, symbol, method);
}

void wrenInitializeCore(WrenVM* vm)
{
ObjModule* coreModule = wrenNewModule(vm, NULL);
Expand Down Expand Up @@ -1199,23 +1224,23 @@ void wrenInitializeCore(WrenVM* vm)
PRIMITIVE(vm->fnClass->obj.classObj, "new(_)", fn_new);

PRIMITIVE(vm->fnClass, "arity", fn_arity);
fnCall(vm, "call()");
fnCall(vm, "call(_)");
fnCall(vm, "call(_,_)");
fnCall(vm, "call(_,_,_)");
fnCall(vm, "call(_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_,_)");
fnCall(vm, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_)");
PRIMITIVE(vm->fnClass, "call()", fn_call0);
PRIMITIVE(vm->fnClass, "call(_)", fn_call1);
PRIMITIVE(vm->fnClass, "call(_,_)", fn_call2);
PRIMITIVE(vm->fnClass, "call(_,_,_)", fn_call3);
PRIMITIVE(vm->fnClass, "call(_,_,_,_)", fn_call4);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_)", fn_call5);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_)", fn_call6);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_)", fn_call7);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_)", fn_call8);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_)", fn_call9);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_)", fn_call10);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_)", fn_call11);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_,_)", fn_call12);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_,_,_)", fn_call13);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_)", fn_call14);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_,_)", fn_call15);
PRIMITIVE(vm->fnClass, "call(_,_,_,_,_,_,_,_,_,_,_,_,_,_,_,_)", fn_call16);
PRIMITIVE(vm->fnClass, "toString", fn_toString);

vm->nullClass = AS_CLASS(wrenFindVariable(vm, coreModule, "Null"));
Expand Down
2 changes: 1 addition & 1 deletion src/vm/wren_primitive.h
View file
Expand Up @@ -11,7 +11,7 @@
&vm->methodNames, name, strlen(name)); \
Method method; \
method.type = METHOD_PRIMITIVE; \
method.fn.primitive = prim_##function; \
method.as.primitive = prim_##function; \
wrenBindMethod(vm, cls, symbol, method); \
}

Expand Down
2 changes: 1 addition & 1 deletion src/vm/wren_value.c
View file
Expand Up @@ -1008,7 +1008,7 @@ static void blackenClass(WrenVM* vm, ObjClass* classObj)
{
if (classObj->methods.data[i].type == METHOD_BLOCK)
{
wrenGrayObj(vm, (Obj*)classObj->methods.data[i].fn.obj);
wrenGrayObj(vm, (Obj*)classObj->methods.data[i].as.closure);
}
}

Expand Down
9 changes: 2 additions & 7 deletions src/vm/wren_value.h
View file
Expand Up @@ -349,9 +349,6 @@ typedef enum
// A normal user-defined method.
METHOD_BLOCK,

// The special "call(...)" methods on function.
METHOD_FN_CALL,

// No method for the given symbol.
METHOD_NONE
} MethodType;
Expand All @@ -366,10 +363,8 @@ typedef struct
{
Primitive primitive;
WrenForeignMethodFn foreign;

// May be a [ObjFn] or [ObjClosure].
ObjClosure* obj;
} fn;
ObjClosure* closure;
} as;
} Method;

DECLARE_BUFFER(Method, Method);
Expand Down
78 changes: 14 additions & 64 deletions src/vm/wren_vm.c
View file
Expand Up @@ -342,12 +342,12 @@ static void bindMethod(WrenVM* vm, int methodType, int symbol,
{
const char* name = AS_CSTRING(methodValue);
method.type = METHOD_FOREIGN;
method.fn.foreign = findForeignMethod(vm, module->name->value,
method.as.foreign = findForeignMethod(vm, module->name->value,
className,
methodType == CODE_METHOD_STATIC,
name);

if (method.fn.foreign == NULL)
if (method.as.foreign == NULL)
{
vm->fiber->error = wrenStringFormat(vm,
"Could not find foreign method '@' for class $ in module '$'.",
Expand All @@ -357,11 +357,11 @@ static void bindMethod(WrenVM* vm, int methodType, int symbol,
}
else
{
method.fn.obj = AS_CLOSURE(methodValue);
method.as.closure = AS_CLOSURE(methodValue);
method.type = METHOD_BLOCK;

// Patch up the bytecode now that we know the superclass.
wrenBindMethodCode(classObj, method.fn.obj->fn);
wrenBindMethodCode(classObj, method.as.closure->fn);
}

wrenBindMethod(vm, classObj, symbol, method);
Expand Down Expand Up @@ -427,48 +427,6 @@ static void methodNotFound(WrenVM* vm, ObjClass* classObj, int symbol)
OBJ_VAL(classObj->name), vm->methodNames.data[symbol]->value);
}

// Checks that [value], which must be a closure, does not require more
// parameters than are provided by [numArgs].
//
// If there are not enough arguments, aborts the current fiber and returns
// `false`.
static bool checkArity(WrenVM* vm, Value value, int numArgs)
{
ASSERT(IS_CLOSURE(value), "Receiver must be a closure.");
ObjFn* fn = AS_CLOSURE(value)->fn;

// We only care about missing arguments, not extras. The "- 1" is because
// numArgs includes the receiver, the function itself, which we don't want to
// count.
if (numArgs - 1 >= fn->arity) return true;

vm->fiber->error = CONST_STRING(vm, "Function expects more arguments.");
return false;
}

// Pushes [closure] onto [fiber]'s callstack and invokes it. Expects [numArgs]
// arguments (including the receiver) to be on the top of the stack already.
static inline void callFunction(
WrenVM* vm, ObjFiber* fiber, ObjClosure* closure, int numArgs)
{
// Grow the call frame array if needed.
if (fiber->numFrames + 1 > fiber->frameCapacity)
{
int max = fiber->frameCapacity * 2;
fiber->frames = (CallFrame*)wrenReallocate(vm, fiber->frames,
sizeof(CallFrame) * fiber->frameCapacity,
sizeof(CallFrame) * max);
fiber->frameCapacity = max;
}

// Grow the stack if needed.
int stackSize = (int)(fiber->stackTop - fiber->stack);
int needed = stackSize + closure->fn->maxSlots;
wrenEnsureStack(vm, fiber, needed);

wrenAppendCallFrame(vm, fiber, closure, fiber->stackTop - numArgs);
}

// Looks up the previously loaded module with [name].
//
// Returns `NULL` if no module with that name has been loaded.
Expand Down Expand Up @@ -611,7 +569,7 @@ static void bindForeignClass(WrenVM* vm, ObjClass* classObj, ObjModule* module)
int symbol = wrenSymbolTableEnsure(vm, &vm->methodNames, "<allocate>", 10);
if (methods.allocate != NULL)
{
method.fn.foreign = methods.allocate;
method.as.foreign = methods.allocate;
wrenBindMethod(vm, classObj, symbol, method);
}

Expand All @@ -620,7 +578,7 @@ static void bindForeignClass(WrenVM* vm, ObjClass* classObj, ObjModule* module)
symbol = wrenSymbolTableEnsure(vm, &vm->methodNames, "<finalize>", 10);
if (methods.finalize != NULL)
{
method.fn.foreign = (WrenForeignMethodFn)methods.finalize;
method.as.foreign = (WrenForeignMethodFn)methods.finalize;
wrenBindMethod(vm, classObj, symbol, method);
}
}
Expand Down Expand Up @@ -669,7 +627,7 @@ static void createForeign(WrenVM* vm, ObjFiber* fiber, Value* stack)
Value* oldApiStack = vm->apiStack;
vm->apiStack = stack;

method->fn.foreign(vm);
method->as.foreign(vm);

vm->apiStack = oldApiStack;
// TODO: Check that allocateForeign was called.
Expand All @@ -693,7 +651,7 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign)

ASSERT(method->type == METHOD_FOREIGN, "Finalizer should be foreign.");

WrenFinalizerFn finalizer = (WrenFinalizerFn)method->fn.foreign;
WrenFinalizerFn finalizer = (WrenFinalizerFn)method->as.foreign;
finalizer(foreign->data);
}

Expand Down Expand Up @@ -1033,13 +991,13 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
switch (method->type)
{
case METHOD_PRIMITIVE:
if (method->fn.primitive(vm, args))
if (method->as.primitive(vm, args))
{
// The result is now in the first arg slot. Discard the other
// stack slots.
fiber->stackTop -= numArgs - 1;
} else {
// An error or fiber switch occurred.
// An error, fiber switch, or call frame change occurred.
STORE_FRAME();

// If we don't have a fiber to switch to, stop interpreting.
Expand All @@ -1051,21 +1009,13 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
break;

case METHOD_FOREIGN:
callForeign(vm, fiber, method->fn.foreign, numArgs);
callForeign(vm, fiber, method->as.foreign, numArgs);
if (!IS_NULL(fiber->error)) RUNTIME_ERROR();
break;

case METHOD_FN_CALL:
if (!checkArity(vm, args[0], numArgs)) RUNTIME_ERROR();

STORE_FRAME();
callFunction(vm, fiber, AS_CLOSURE(args[0]), numArgs);
LOAD_FRAME();
break;

case METHOD_BLOCK:
STORE_FRAME();
callFunction(vm, fiber, (ObjClosure*)method->fn.obj, numArgs);
wrenCallFunction(vm, fiber, (ObjClosure*)method->as.closure, numArgs);
LOAD_FRAME();
break;

Expand Down Expand Up @@ -1327,7 +1277,7 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
{
STORE_FRAME();
ObjClosure* closure = AS_CLOSURE(PEEK());
callFunction(vm, fiber, closure, 1);
wrenCallFunction(vm, fiber, closure, 1);
LOAD_FRAME();
}
else
Expand Down Expand Up @@ -1433,7 +1383,7 @@ WrenInterpretResult wrenCall(WrenVM* vm, WrenHandle* method)
// function has exactly one slot for each argument.
vm->fiber->stackTop = &vm->fiber->stack[closure->fn->maxSlots];

callFunction(vm, vm->fiber, closure, 0);
wrenCallFunction(vm, vm->fiber, closure, 0);
return runInterpreter(vm, vm->fiber);
}

Expand Down
26 changes: 24 additions & 2 deletions src/vm/wren_vm.h
View file
Expand Up @@ -174,10 +174,32 @@ int wrenDeclareVariable(WrenVM* vm, ObjModule* module, const char* name,
int wrenDefineVariable(WrenVM* vm, ObjModule* module, const char* name,
size_t length, Value value);

// Mark [obj] as a GC root so that it doesn't get collected.
// Pushes [closure] onto [fiber]'s callstack to invoke it. Expects [numArgs]
// arguments (including the receiver) to be on the top of the stack already.
static inline void wrenCallFunction(WrenVM* vm, ObjFiber* fiber,
ObjClosure* closure, int numArgs)
{
// Grow the call frame array if needed.
if (fiber->numFrames + 1 > fiber->frameCapacity)
{
int max = fiber->frameCapacity * 2;
fiber->frames = (CallFrame*)wrenReallocate(vm, fiber->frames,
sizeof(CallFrame) * fiber->frameCapacity, sizeof(CallFrame) * max);
fiber->frameCapacity = max;
}

// Grow the stack if needed.
int stackSize = (int)(fiber->stackTop - fiber->stack);
int needed = stackSize + closure->fn->maxSlots;
wrenEnsureStack(vm, fiber, needed);

wrenAppendCallFrame(vm, fiber, closure, fiber->stackTop - numArgs);
}

// Marks [obj] as a GC root so that it doesn't get collected.
void wrenPushRoot(WrenVM* vm, Obj* obj);

// Remove the most recently pushed temporary root.
// Removes the most recently pushed temporary root.
void wrenPopRoot(WrenVM* vm);

// Returns the class of [value].
Expand Down

0 comments on commit 5f29a72

Please sign in to comment.