From 4b3c818ec5e2a7231d5aefc802768f7ede93bd2e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 10:22:42 -0800 Subject: [PATCH] Start moving embedding API towards "slot" register-API. - wrenGetArgumentCount() -> wrenGetSlotCount() - wrenGetArgument___() -> wrenGetSlot___() Also, the get functions assert that the value is the right type instead of checking at runtime. This puts the onus on the caller to be safe, but maximizes performance. --- src/include/wren.h | 43 ++++++++++++--------- src/module/io.c | 70 +++++++++++++++++----------------- src/module/timer.c | 4 +- src/optional/wren_opt_meta.c | 2 +- src/optional/wren_opt_random.c | 14 +++---- src/vm/wren_value.h | 1 + src/vm/wren_vm.c | 70 +++++++++++++++++----------------- test/api/benchmark.c | 13 ++++--- test/api/foreign_class.c | 26 ++++++------- test/api/value.c | 2 +- 10 files changed, 125 insertions(+), 120 deletions(-) diff --git a/src/include/wren.h b/src/include/wren.h index 29645f83..53cdc1a9 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -120,7 +120,7 @@ typedef struct // // If this is `NULL`, Wren discards any printed text. WrenWriteFn writeFn; - + // The number of bytes Wren will allocate before triggering the first garbage // collection. // @@ -237,8 +237,8 @@ void wrenReleaseValue(WrenVM* vm, WrenValue* value); // the foreign object's data. void* wrenAllocateForeign(WrenVM* vm, size_t size); -// Returns the number of arguments available to the current foreign method. -int wrenGetArgumentCount(WrenVM* vm); +// Returns the number of slots available to the current foreign method. +int wrenGetSlotCount(WrenVM* vm); // The following functions read one of the arguments passed to a foreign call. // They may only be called while within a function provided to @@ -260,32 +260,37 @@ int wrenGetArgumentCount(WrenVM* vm); // // It is an error to pass an invalid argument index. -// Reads a boolean argument for a foreign call. Returns false if the argument -// is not a boolean. -bool wrenGetArgumentBool(WrenVM* vm, int index); +// Reads a boolean value from [slot]. +// +// It is an error to call this if the slot does not contain a boolean value. +bool wrenGetSlotBool(WrenVM* vm, int slot); -// Reads a numeric argument for a foreign call. Returns 0 if the argument is not -// a number. -double wrenGetArgumentDouble(WrenVM* vm, int index); +// Reads a number from [slot]. +// +// It is an error to call this if the slot does not contain a number. +double wrenGetSlotDouble(WrenVM* vm, int slot); -// Reads a foreign object argument for a foreign call and returns a pointer to -// the foreign data stored with it. Returns NULL if the argument is not a -// foreign object. -void* wrenGetArgumentForeign(WrenVM* vm, int index); +// Reads a foreign object from [slot] and returns a pointer to the foreign data +// stored with it. +// +// It is an error to call this if the slot does not contain an instance of a +// foreign class. +void* wrenGetSlotForeign(WrenVM* vm, int slot); -// Reads an string argument for a foreign call. Returns NULL if the argument is -// not a string. +// Reads a string from [slot]. // // The memory for the returned string is owned by Wren. You can inspect it -// while in your foreign function, but cannot keep a pointer to it after the +// while in your foreign method, but cannot keep a pointer to it after the // function returns, since the garbage collector may reclaim it. -const char* wrenGetArgumentString(WrenVM* vm, int index); +// +// It is an error to call this if the slot does not contain a string. +const char* wrenGetSlotString(WrenVM* vm, int slot); -// Creates a handle for the value passed as an argument to a foreign call. +// Creates a handle for the value stored in [slot]. // // This will prevent the object that is referred to from being garbage collected // until the handle is released by calling [wrenReleaseValue()]. -WrenValue* wrenGetArgumentValue(WrenVM* vm, int index); +WrenValue* wrenGetSlotValue(WrenVM* vm, int slot); // The following functions provide the return value for a foreign method back // to Wren. Like above, they may only be called during a foreign call invoked diff --git a/src/module/io.c b/src/module/io.c index 9f3a836b..bd03bd52 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -33,7 +33,7 @@ static void shutdownStdin() free(stdinStream); stdinStream = NULL; } - + if (stdinOnData != NULL) { wrenReleaseValue(getVM(), stdinOnData); @@ -51,16 +51,16 @@ void fileAllocate(WrenVM* vm) // Store the file descriptor in the foreign data, so that we can get to it // in the finalizer. int* fd = (int*)wrenAllocateForeign(vm, sizeof(int)); - *fd = (int)wrenGetArgumentDouble(vm, 1); + *fd = (int)wrenGetSlotDouble(vm, 1); } void fileFinalize(WrenVM* vm) { - int fd = *(int*)wrenGetArgumentForeign(vm, 0); - + int fd = *(int*)wrenGetSlotForeign(vm, 0); + // Already closed. if (fd == -1) return; - + uv_fs_t request; uv_fs_close(getLoop(), &request, fd, NULL); uv_fs_req_cleanup(&request); @@ -73,7 +73,7 @@ void fileFinalize(WrenVM* vm) static bool handleRequestError(uv_fs_t* request) { if (request->result >= 0) return false; - + FileRequestData* data = (FileRequestData*)request->data; WrenValue* fiber = (WrenValue*)data->fiber; @@ -115,18 +115,18 @@ WrenValue* freeRequest(uv_fs_t* request) static void openCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + double fd = (double)request->result; WrenValue* fiber = freeRequest(request); - + schedulerResumeDouble(fiber, fd); } void fileOpen(WrenVM* vm) { - const char* path = wrenGetArgumentString(vm, 1); - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); - + const char* path = wrenGetSlotString(vm, 1); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + // TODO: Allow controlling flags and modes. uv_fs_open(getLoop(), request, path, O_RDONLY, 0, openCallback); } @@ -135,51 +135,51 @@ void fileOpen(WrenVM* vm) static void sizeCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + double size = (double)request->statbuf.st_size; WrenValue* fiber = freeRequest(request); - + schedulerResumeDouble(fiber, size); } void fileSizePath(WrenVM* vm) { - const char* path = wrenGetArgumentString(vm, 1); - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); + const char* path = wrenGetSlotString(vm, 1); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); uv_fs_stat(getLoop(), request, path, sizeCallback); } static void closeCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + WrenValue* fiber = freeRequest(request); schedulerResume(fiber); } void fileClose(WrenVM* vm) { - int* foreign = (int*)wrenGetArgumentForeign(vm, 0); + int* foreign = (int*)wrenGetSlotForeign(vm, 0); int fd = *foreign; - + // If it's already closed, we're done. if (fd == -1) { wrenReturnBool(vm, true); return; } - + // Mark it closed immediately. *foreign = -1; - - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 1)); + + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 1)); uv_fs_close(getLoop(), request, fd, closeCallback); wrenReturnBool(vm, false); } void fileDescriptor(WrenVM* vm) { - int* foreign = (int*)wrenGetArgumentForeign(vm, 0); + int* foreign = (int*)wrenGetSlotForeign(vm, 0); int fd = *foreign; wrenReturnDouble(vm, fd); } @@ -197,35 +197,35 @@ static void fileReadBytesCallback(uv_fs_t* request) // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. schedulerResumeBytes(fiber, buffer.base, buffer.len); - + // TODO: Likewise, freeing this after we resume is lame. free(buffer.base); } void fileReadBytes(WrenVM* vm) { - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); - - int fd = *(int*)wrenGetArgumentForeign(vm, 0); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + + int fd = *(int*)wrenGetSlotForeign(vm, 0); // TODO: Assert fd != -1. FileRequestData* data = (FileRequestData*)request->data; - size_t length = (size_t)wrenGetArgumentDouble(vm, 1); + size_t length = (size_t)wrenGetSlotDouble(vm, 1); data->buffer.len = length; data->buffer.base = (char*)malloc(length); - + // TODO: Allow passing in offset. uv_fs_read(getLoop(), request, fd, &data->buffer, 1, 0, fileReadBytesCallback); } void fileSize(WrenVM* vm) { - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 1)); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 1)); - int fd = *(int*)wrenGetArgumentForeign(vm, 0); + int fd = *(int*)wrenGetSlotForeign(vm, 0); // TODO: Assert fd != -1. - + uv_fs_fstat(getLoop(), request, fd, sizeCallback); } @@ -247,9 +247,9 @@ static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead, shutdownStdin(); return; } - + // TODO: Handle other errors. - + if (stdinOnData == NULL) { stdinOnData = wrenGetMethod(getVM(), "io", "Stdin", "onData_(_)"); @@ -259,7 +259,7 @@ static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead, // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. wrenCall(getVM(), stdinOnData, NULL, "a", buffer->base, numRead); - + // TODO: Likewise, freeing this after we resume is lame. free(buffer->base); } @@ -284,7 +284,7 @@ void stdinReadStart(WrenVM* vm) stdinStream = (uv_stream_t*)handle; } } - + uv_read_start(stdinStream, allocCallback, stdinReadCallback); // TODO: Check return. } diff --git a/src/module/timer.c b/src/module/timer.c index 43b8d662..a9d98eca 100644 --- a/src/module/timer.c +++ b/src/module/timer.c @@ -27,8 +27,8 @@ static void timerCallback(uv_timer_t* handle) void timerStartTimer(WrenVM* vm) { - int milliseconds = (int)wrenGetArgumentDouble(vm, 1); - WrenValue* fiber = wrenGetArgumentValue(vm, 2); + int milliseconds = (int)wrenGetSlotDouble(vm, 1); + WrenValue* fiber = wrenGetSlotValue(vm, 2); // Store the fiber to resume when the timer completes. uv_timer_t* handle = (uv_timer_t*)malloc(sizeof(uv_timer_t)); diff --git a/src/optional/wren_opt_meta.c b/src/optional/wren_opt_meta.c index 435ab678..5428cc23 100644 --- a/src/optional/wren_opt_meta.c +++ b/src/optional/wren_opt_meta.c @@ -18,7 +18,7 @@ void metaCompile(WrenVM* vm) : AS_CLOSURE(callingFn)->fn->module; // Compile it. - ObjFn* fn = wrenCompile(vm, module, wrenGetArgumentString(vm, 1), false); + ObjFn* fn = wrenCompile(vm, module, wrenGetSlotString(vm, 1), false); if (fn == NULL) return; // Return the result. We can't use the public API for this since we have a diff --git a/src/optional/wren_opt_random.c b/src/optional/wren_opt_random.c index b02c7ca7..3735a4db 100644 --- a/src/optional/wren_opt_random.c +++ b/src/optional/wren_opt_random.c @@ -45,7 +45,7 @@ static void randomAllocate(WrenVM* vm) static void randomSeed0(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); srand((uint32_t)time(NULL)); for (int i = 0; i < 16; i++) @@ -56,9 +56,9 @@ static void randomSeed0(WrenVM* vm) static void randomSeed1(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); - srand((uint32_t)wrenGetArgumentDouble(vm, 1)); + srand((uint32_t)wrenGetSlotDouble(vm, 1)); for (int i = 0; i < 16; i++) { well->state[i] = rand(); @@ -67,17 +67,17 @@ static void randomSeed1(WrenVM* vm) static void randomSeed16(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); for (int i = 0; i < 16; i++) { - well->state[i] = (uint32_t)wrenGetArgumentDouble(vm, i + 1); + well->state[i] = (uint32_t)wrenGetSlotDouble(vm, i + 1); } } static void randomFloat(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); // A double has 53 bits of precision in its mantissa, and we'd like to take // full advantage of that, so we need 53 bits of random source data. @@ -97,7 +97,7 @@ static void randomFloat(WrenVM* vm) static void randomInt0(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); wrenReturnDouble(vm, (double)advanceState(well)); } diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index 702538ff..21e871fd 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -74,6 +74,7 @@ #define IS_FN(value) (wrenIsObjType(value, OBJ_FN)) // ObjFn #define IS_FOREIGN(value) (wrenIsObjType(value, OBJ_FOREIGN)) // ObjForeign #define IS_INSTANCE(value) (wrenIsObjType(value, OBJ_INSTANCE)) // ObjInstance +#define IS_LIST(value) (wrenIsObjType(value, OBJ_LIST)) // ObjList #define IS_RANGE(value) (wrenIsObjType(value, OBJ_RANGE)) // ObjRange #define IS_STRING(value) (wrenIsObjType(value, OBJ_STRING)) // ObjString diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index e6157160..4d44d49f 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -439,21 +439,21 @@ static inline void callFunction( sizeof(CallFrame) * max); fiber->frameCapacity = max; } - + // Grow the stack if needed. int stackSize = (int)(fiber->stackTop - fiber->stack); int needed = stackSize + wrenUpwrapClosure(function)->maxSlots; - + if (fiber->stackCapacity < needed) { int capacity = wrenPowerOf2Ceil(needed); - + Value* oldStack = fiber->stack; fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, sizeof(Value) * fiber->stackCapacity, sizeof(Value) * capacity); fiber->stackCapacity = capacity; - + // If the reallocation moves the stack, then we need to shift every pointer // into the stack to point to its new location. if (fiber->stack != oldStack) @@ -461,13 +461,13 @@ static inline void callFunction( // Top of the stack. long offset = fiber->stack - oldStack; fiber->stackTop += offset; - + // Stack pointer for each call frame. for (int i = 0; i < fiber->numFrames; i++) { fiber->frames[i].stackStart += offset; } - + // Open upvalues. for (ObjUpvalue* upvalue = fiber->openUpvalues; upvalue != NULL; @@ -1148,10 +1148,10 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) // See if there's another fiber to return to. ObjFiber* callingFiber = fiber->caller; fiber->caller = NULL; - + fiber = callingFiber; vm->fiber = fiber; - + // If not, we're done. if (fiber == NULL) return WREN_RESULT_SUCCESS; @@ -1624,10 +1624,10 @@ void wrenPopRoot(WrenVM* vm) vm->numTempRoots--; } -int wrenGetArgumentCount(WrenVM* vm) +int wrenGetSlotCount(WrenVM* vm) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - + // If no fiber is executing, we must be in a finalizer, in which case the // "stack" just has one object, the object being finalized. if (vm->fiber == NULL) return 1; @@ -1635,54 +1635,52 @@ int wrenGetArgumentCount(WrenVM* vm) return (int)(vm->fiber->stackTop - vm->foreignStackStart); } -static void validateForeignArgument(WrenVM* vm, int index) +// Ensures that [slot] is a valid index into a foreign method's stack of slots. +static void validateForeignSlot(WrenVM* vm, int slot) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - ASSERT(index >= 0, "index cannot be negative."); - ASSERT(index < wrenGetArgumentCount(vm), "Not that many arguments."); + ASSERT(slot >= 0, "Slot cannot be negative."); + ASSERT(slot < wrenGetSlotCount(vm), "Not that many slots."); } -bool wrenGetArgumentBool(WrenVM* vm, int index) +bool wrenGetSlotBool(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_BOOL(vm->foreignStackStart[slot]), "Slot must hold a bool."); - if (!IS_BOOL(vm->foreignStackStart[index])) return false; - - return AS_BOOL(vm->foreignStackStart[index]); + return AS_BOOL(vm->foreignStackStart[slot]); } -double wrenGetArgumentDouble(WrenVM* vm, int index) +double wrenGetSlotDouble(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_NUM(vm->foreignStackStart[slot]), "Slot must hold a number."); - if (!IS_NUM(vm->foreignStackStart[index])) return 0.0; - - return AS_NUM(vm->foreignStackStart[index]); + return AS_NUM(vm->foreignStackStart[slot]); } -void* wrenGetArgumentForeign(WrenVM* vm, int index) +void* wrenGetSlotForeign(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_FOREIGN(vm->foreignStackStart[slot]), + "Slot must hold a foreign instance."); - if (!IS_FOREIGN(vm->foreignStackStart[index])) return NULL; - - return AS_FOREIGN(vm->foreignStackStart[index])->data; + return AS_FOREIGN(vm->foreignStackStart[slot])->data; } -const char* wrenGetArgumentString(WrenVM* vm, int index) +const char* wrenGetSlotString(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_STRING(vm->foreignStackStart[slot]), "Slot must hold a string."); - if (!IS_STRING(vm->foreignStackStart[index])) return NULL; - - return AS_CSTRING(vm->foreignStackStart[index]); + return AS_CSTRING(vm->foreignStackStart[slot]); } -WrenValue* wrenGetArgumentValue(WrenVM* vm, int index) +WrenValue* wrenGetSlotValue(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); - return wrenCaptureValue(vm, vm->foreignStackStart[index]); + return wrenCaptureValue(vm, vm->foreignStackStart[slot]); } void wrenReturnBool(WrenVM* vm, bool value) diff --git a/test/api/benchmark.c b/test/api/benchmark.c index 5c7f4ccb..d5c84a8f 100644 --- a/test/api/benchmark.c +++ b/test/api/benchmark.c @@ -5,17 +5,18 @@ static void arguments(WrenVM* vm) { double result = 0; - result += wrenGetArgumentDouble(vm, 1); - result += wrenGetArgumentDouble(vm, 2); - result += wrenGetArgumentDouble(vm, 3); - result += wrenGetArgumentDouble(vm, 4); - + + result += wrenGetSlotDouble(vm, 1); + result += wrenGetSlotDouble(vm, 2); + result += wrenGetSlotDouble(vm, 3); + result += wrenGetSlotDouble(vm, 4); + wrenReturnDouble(vm, result); } WrenForeignMethodFn benchmarkBindMethod(const char* signature) { if (strcmp(signature, "static Benchmark.arguments(_,_,_,_)") == 0) return arguments; - + return NULL; } diff --git a/test/api/foreign_class.c b/test/api/foreign_class.c index cf9fbfeb..cd544def 100644 --- a/test/api/foreign_class.c +++ b/test/api/foreign_class.c @@ -18,15 +18,15 @@ static void counterAllocate(WrenVM* vm) static void counterIncrement(WrenVM* vm) { - double* value = (double*)wrenGetArgumentForeign(vm, 0); - double increment = wrenGetArgumentDouble(vm, 1); + double* value = (double*)wrenGetSlotForeign(vm, 0); + double increment = wrenGetSlotDouble(vm, 1); *value += increment; } static void counterValue(WrenVM* vm) { - double value = *(double*)wrenGetArgumentForeign(vm, 0); + double value = *(double*)wrenGetSlotForeign(vm, 0); wrenReturnDouble(vm, value); } @@ -34,9 +34,9 @@ static void pointAllocate(WrenVM* vm) { double* coordinates = (double*)wrenAllocateForeign(vm, sizeof(double[3])); - // This gets called by both constructors, so sniff the argument count to see + // This gets called by both constructors, so sniff the slot count to see // which one was invoked. - if (wrenGetArgumentCount(vm) == 1) + if (wrenGetSlotCount(vm) == 1) { coordinates[0] = 0.0; coordinates[1] = 0.0; @@ -44,23 +44,23 @@ static void pointAllocate(WrenVM* vm) } else { - coordinates[0] = wrenGetArgumentDouble(vm, 1); - coordinates[1] = wrenGetArgumentDouble(vm, 2); - coordinates[2] = wrenGetArgumentDouble(vm, 3); + coordinates[0] = wrenGetSlotDouble(vm, 1); + coordinates[1] = wrenGetSlotDouble(vm, 2); + coordinates[2] = wrenGetSlotDouble(vm, 3); } } static void pointTranslate(WrenVM* vm) { - double* coordinates = (double*)wrenGetArgumentForeign(vm, 0); - coordinates[0] += wrenGetArgumentDouble(vm, 1); - coordinates[1] += wrenGetArgumentDouble(vm, 2); - coordinates[2] += wrenGetArgumentDouble(vm, 3); + double* coordinates = (double*)wrenGetSlotForeign(vm, 0); + coordinates[0] += wrenGetSlotDouble(vm, 1); + coordinates[1] += wrenGetSlotDouble(vm, 2); + coordinates[2] += wrenGetSlotDouble(vm, 3); } static void pointToString(WrenVM* vm) { - double* coordinates = (double*)wrenGetArgumentForeign(vm, 0); + double* coordinates = (double*)wrenGetSlotForeign(vm, 0); char result[100]; sprintf(result, "(%g, %g, %g)", coordinates[0], coordinates[1], coordinates[2]); diff --git a/test/api/value.c b/test/api/value.c index 5b7b6225..e26d4978 100644 --- a/test/api/value.c +++ b/test/api/value.c @@ -6,7 +6,7 @@ static WrenValue* value; static void setValue(WrenVM* vm) { - value = wrenGetArgumentValue(vm, 1); + value = wrenGetSlotValue(vm, 1); } static void getValue(WrenVM* vm)