From 6687a21e5ce22518e1e09627f876bb067471d583 Mon Sep 17 00:00:00 2001 From: Jason_000 Date: Fri, 1 Dec 2023 13:10:32 +0000 Subject: [PATCH 1/2] Fix an issue with internal table type --- src/vm/table.c | 58 +++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/vm/table.c b/src/vm/table.c index 554d97ca..e3f2b330 100644 --- a/src/vm/table.c +++ b/src/vm/table.c @@ -10,22 +10,22 @@ void initTable(Table *table) { table->count = 0; - table->capacityMask = -1; + table->capacityMask = 0; table->entries = NULL; } void freeTable(DictuVM *vm, Table *table) { - FREE_ARRAY(vm, Entry, table->entries, table->capacityMask + 1); + FREE_ARRAY(vm, Entry, table->entries, table->capacityMask); initTable(table); } static Entry *findEntry(Entry *entries, int capacityMask, ObjString *key) { - uint32_t index = key->hash & capacityMask; - Entry *tombstone = NULL; + uint32_t index = key->hash & (capacityMask - 1); + Entry* tombstone = NULL; for (;;) { - Entry *entry = &entries[index]; + Entry* entry = &entries[index]; if (entry->key == NULL) { if (IS_NIL(entry->value)) { @@ -40,7 +40,7 @@ static Entry *findEntry(Entry *entries, int capacityMask, return entry; } - index = (index + 1) & capacityMask; + index = (index + 1) & (capacityMask - 1); } } @@ -55,34 +55,33 @@ bool tableGet(Table *table, ObjString *key, Value *value) { } static void adjustCapacity(DictuVM *vm, Table *table, int capacityMask) { - Entry *entries = ALLOCATE(vm, Entry, capacityMask + 1); - for (int i = 0; i <= capacityMask; i++) { + Entry* entries = ALLOCATE(vm, Entry, capacityMask); + for (int i = 0; i < capacityMask; i++) { entries[i].key = NULL; entries[i].value = NIL_VAL; } table->count = 0; - for (int i = 0; i <= table->capacityMask; i++) { - Entry *entry = &table->entries[i]; + for (int i = 0; i < table->capacityMask; i++) { + Entry* entry = &table->entries[i]; if (entry->key == NULL) continue; - Entry *dest = findEntry(entries, capacityMask, entry->key); + Entry* dest = findEntry(entries, capacityMask, entry->key); dest->key = entry->key; dest->value = entry->value; table->count++; } - FREE_ARRAY(vm, Entry, table->entries, table->capacityMask + 1); + FREE_ARRAY(vm, Entry, table->entries, table->capacityMask); table->entries = entries; table->capacityMask = capacityMask; } bool tableSet(DictuVM *vm, Table *table, ObjString *key, Value value) { - if (table->count + 1 > (table->capacityMask + 1) * TABLE_MAX_LOAD) { - // Figure out the new table size. - int capacityMask = GROW_CAPACITY(table->capacityMask + 1) - 1; - adjustCapacity(vm, table, capacityMask); + if (table->count + 1 > table->capacityMask * TABLE_MAX_LOAD) { + int capacity = GROW_CAPACITY(table->capacityMask); + adjustCapacity(vm, table, capacity); } Entry *entry = findEntry(table->entries, table->capacityMask, key); @@ -99,20 +98,19 @@ bool tableDelete(DictuVM *vm, Table *table, ObjString *key) { UNUSED(vm); if (table->count == 0) return false; - Entry *entry = findEntry(table->entries, table->capacityMask, key); + // Find the entry. + Entry* entry = findEntry(table->entries, table->capacityMask, key); if (entry->key == NULL) return false; // Place a tombstone in the entry. - table->count--; entry->key = NULL; entry->value = BOOL_VAL(true); - return true; } void tableAddAll(DictuVM *vm, Table *from, Table *to) { - for (int i = 0; i <= from->capacityMask; i++) { - Entry *entry = &from->entries[i]; + for (int i = 0; i < from->capacityMask; i++) { + Entry* entry = &from->entries[i]; if (entry->key != NULL) { tableSet(vm, to, entry->key, entry->value); } @@ -127,13 +125,12 @@ ObjString *tableFindString(Table *table, const char *chars, int length, // Figure out where to insert it in the table. Use open addressing and // basic linear probing. - - uint32_t index = hash & table->capacityMask; + uint32_t index = hash & (table->capacityMask - 1); for (;;) { - Entry *entry = &table->entries[index]; - + Entry* entry = &table->entries[index]; if (entry->key == NULL) { + // Stop if we find an empty non-tombstone entry. if (IS_NIL(entry->value)) return NULL; } else if (entry->key->length == length && entry->key->hash == hash && @@ -142,14 +139,13 @@ ObjString *tableFindString(Table *table, const char *chars, int length, return entry->key; } - // Try the next slot. - index = (index + 1) & table->capacityMask; + index = (index + 1) & (table->capacityMask - 1); } } -void tableRemoveWhite(DictuVM *vm, Table *table) { - for (int i = 0; i <= table->capacityMask; i++) { - Entry *entry = &table->entries[i]; +void tableRemoveWhite(DictuVM *vm, Table* table) { + for (int i = 0; i < table->capacityMask; i++) { + Entry* entry = &table->entries[i]; if (entry->key != NULL && !entry->key->obj.isDark) { tableDelete(vm, table, entry->key); } @@ -157,7 +153,7 @@ void tableRemoveWhite(DictuVM *vm, Table *table) { } void grayTable(DictuVM *vm, Table *table) { - for (int i = 0; i <= table->capacityMask; i++) { + for (int i = 0; i < table->capacityMask; i++) { Entry *entry = &table->entries[i]; grayObject(vm, (Obj *) entry->key); grayValue(vm, entry->value); From 14196fd7c2d80ae288a40b1b1254b36766e375c9 Mon Sep 17 00:00:00 2001 From: Jason_000 Date: Fri, 1 Dec 2023 13:16:51 +0000 Subject: [PATCH 2/2] Correct use of capacity --- src/optionals/unittest/unittest.c | 2 +- src/vm/datatypes/class.c | 2 +- src/vm/datatypes/copy.c | 4 +-- src/vm/datatypes/enums.c | 2 +- src/vm/datatypes/instance.c | 10 +++---- src/vm/object.c | 6 ++-- src/vm/table.c | 48 +++++++++++++++---------------- src/vm/table.h | 2 +- src/vm/vm.c | 2 +- 9 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/optionals/unittest/unittest.c b/src/optionals/unittest/unittest.c index 23268c63..31dd627e 100644 --- a/src/optionals/unittest/unittest.c +++ b/src/optionals/unittest/unittest.c @@ -31,7 +31,7 @@ static Value mockNative(DictuVM *vm, int argCount, Value *args) { pop(vm); push(vm, OBJ_VAL(mockedClass)); - for (int i = 0; i <= klass->publicMethods.capacityMask; ++i) { + for (int i = 0; i < klass->publicMethods.capacity; ++i) { if (klass->publicMethods.entries[i].key == NULL) { continue; } diff --git a/src/vm/datatypes/class.c b/src/vm/datatypes/class.c index b783c46b..939295da 100644 --- a/src/vm/datatypes/class.c +++ b/src/vm/datatypes/class.c @@ -38,7 +38,7 @@ static Value methods(DictuVM *vm, int argCount, Value *args) { ObjList *list = newList(vm); push(vm, OBJ_VAL(list)); - for (int i = 0; i <= klass->publicMethods.capacityMask; ++i) { + for (int i = 0; i < klass->publicMethods.capacity; ++i) { if (klass->publicMethods.entries[i].key == NULL) { continue; } diff --git a/src/vm/datatypes/copy.c b/src/vm/datatypes/copy.c index 55c21b2d..c1862da5 100644 --- a/src/vm/datatypes/copy.c +++ b/src/vm/datatypes/copy.c @@ -70,7 +70,7 @@ ObjInstance *copyInstance(DictuVM* vm, ObjInstance *oldInstance, bool shallow) { if (shallow) { tableAddAll(vm, &oldInstance->publicAttributes, &instance->publicAttributes); } else { - for (int i = 0; i <= oldInstance->publicAttributes.capacityMask; i++) { + for (int i = 0; i < oldInstance->publicAttributes.capacity; i++) { Entry *entry = &oldInstance->publicAttributes.entries[i]; if (entry->key != NULL) { Value val = entry->value; @@ -90,7 +90,7 @@ ObjInstance *copyInstance(DictuVM* vm, ObjInstance *oldInstance, bool shallow) { } } - for (int i = 0; i <= oldInstance->privateAttributes.capacityMask; i++) { + for (int i = 0; i < oldInstance->privateAttributes.capacity; i++) { Entry *entry = &oldInstance->privateAttributes.entries[i]; if (entry->key != NULL) { Value val = entry->value; diff --git a/src/vm/datatypes/enums.c b/src/vm/datatypes/enums.c index 7a5c170c..2d3ac7ac 100644 --- a/src/vm/datatypes/enums.c +++ b/src/vm/datatypes/enums.c @@ -10,7 +10,7 @@ static Value values(DictuVM *vm, int argCount, Value *args) { ObjDict *dict = newDict(vm); push(vm, OBJ_VAL(dict)); - for (int i = 0; i < objEnum->values.capacityMask + 1; ++i) { + for (int i = 0; i < objEnum->values.capacity; ++i) { if (objEnum->values.entries[i].key == NULL) { continue; } diff --git a/src/vm/datatypes/instance.c b/src/vm/datatypes/instance.c index 0952169e..8d497de3 100644 --- a/src/vm/datatypes/instance.c +++ b/src/vm/datatypes/instance.c @@ -132,7 +132,7 @@ static Value getAttributes(DictuVM *vm, int argCount, Value *args) { // Walk the inheritance chain while (klass != NULL) { - for (int i = 0; i < klass->variables.capacityMask + 1; i++) { + for (int i = 0; i < klass->variables.capacity; i++) { if (klass->variables.entries[i].key == NULL) { continue; } @@ -144,7 +144,7 @@ static Value getAttributes(DictuVM *vm, int argCount, Value *args) { writeValueArray(vm, &fields->values, OBJ_VAL(klass->variables.entries[i].key)); } - for (int i = 0; i < klass->constants.capacityMask + 1; i++) { + for (int i = 0; i < klass->constants.capacity; i++) { if (klass->constants.entries[i].key == NULL) { continue; } @@ -170,7 +170,7 @@ static Value getAttributes(DictuVM *vm, int argCount, Value *args) { ObjList *attributes = newList(vm); push(vm, OBJ_VAL(attributes)); - for (int i = 0; i < instance->publicAttributes.capacityMask + 1; i++) { + for (int i = 0; i < instance->publicAttributes.capacity; i++) { if (instance->publicAttributes.entries[i].key == NULL) { continue; } @@ -193,7 +193,7 @@ static Value getAttributes(DictuVM *vm, int argCount, Value *args) { ObjList *methods = newList(vm); push(vm, OBJ_VAL(methods)); - for (int i = 0; i <= instance->klass->publicMethods.capacityMask; ++i) { + for (int i = 0; i < instance->klass->publicMethods.capacity; ++i) { if (instance->klass->publicMethods.entries[i].key == NULL) { continue; } @@ -295,7 +295,7 @@ static Value methods(DictuVM *vm, int argCount, Value *args) { ObjList *list = newList(vm); push(vm, OBJ_VAL(list)); - for (int i = 0; i <= instance->klass->publicMethods.capacityMask; ++i) { + for (int i = 0; i < instance->klass->publicMethods.capacity; ++i) { if (instance->klass->publicMethods.entries[i].key == NULL) { continue; } diff --git a/src/vm/object.c b/src/vm/object.c index 9257f605..ac677abd 100644 --- a/src/vm/object.c +++ b/src/vm/object.c @@ -550,21 +550,21 @@ ObjDict *classToDict(DictuVM *vm, Value value) { push(vm, OBJ_VAL(methodsList)); while (klass != NULL) { - for (int i = 0; i < klass->variables.capacityMask + 1; i++) { + for (int i = 0; i < klass->variables.capacity; i++) { if (klass->variables.entries[i].key == NULL) { continue; } dictSet(vm, variablesDict, OBJ_VAL(klass->variables.entries[i].key), klass->variables.entries[i].value); } - for (int i = 0; i < klass->constants.capacityMask + 1; i++) { + for (int i = 0; i < klass->constants.capacity; i++) { if (klass->constants.entries[i].key == NULL) { continue; } dictSet(vm, constantsDict, OBJ_VAL(klass->constants.entries[i].key), klass->constants.entries[i].value); } - for (int i = 0; i < klass->publicMethods.capacityMask + 1; i++) { + for (int i = 0; i < klass->publicMethods.capacity; i++) { if (klass->publicMethods.entries[i].key == NULL) { continue; } diff --git a/src/vm/table.c b/src/vm/table.c index e3f2b330..e1c0e9d2 100644 --- a/src/vm/table.c +++ b/src/vm/table.c @@ -10,22 +10,22 @@ void initTable(Table *table) { table->count = 0; - table->capacityMask = 0; + table->capacity = 0; table->entries = NULL; } void freeTable(DictuVM *vm, Table *table) { - FREE_ARRAY(vm, Entry, table->entries, table->capacityMask); + FREE_ARRAY(vm, Entry, table->entries, table->capacity); initTable(table); } static Entry *findEntry(Entry *entries, int capacityMask, ObjString *key) { uint32_t index = key->hash & (capacityMask - 1); - Entry* tombstone = NULL; + Entry *tombstone = NULL; for (;;) { - Entry* entry = &entries[index]; + Entry *entry = &entries[index]; if (entry->key == NULL) { if (IS_NIL(entry->value)) { @@ -47,7 +47,7 @@ static Entry *findEntry(Entry *entries, int capacityMask, bool tableGet(Table *table, ObjString *key, Value *value) { if (table->count == 0) return false; - Entry *entry = findEntry(table->entries, table->capacityMask, key); + Entry *entry = findEntry(table->entries, table->capacity, key); if (entry->key == NULL) return false; *value = entry->value; @@ -55,7 +55,7 @@ bool tableGet(Table *table, ObjString *key, Value *value) { } static void adjustCapacity(DictuVM *vm, Table *table, int capacityMask) { - Entry* entries = ALLOCATE(vm, Entry, capacityMask); + Entry *entries = ALLOCATE(vm, Entry, capacityMask); for (int i = 0; i < capacityMask; i++) { entries[i].key = NULL; entries[i].value = NIL_VAL; @@ -63,28 +63,28 @@ static void adjustCapacity(DictuVM *vm, Table *table, int capacityMask) { table->count = 0; - for (int i = 0; i < table->capacityMask; i++) { - Entry* entry = &table->entries[i]; + for (int i = 0; i < table->capacity; i++) { + Entry *entry = &table->entries[i]; if (entry->key == NULL) continue; - Entry* dest = findEntry(entries, capacityMask, entry->key); + Entry *dest = findEntry(entries, capacityMask, entry->key); dest->key = entry->key; dest->value = entry->value; table->count++; } - FREE_ARRAY(vm, Entry, table->entries, table->capacityMask); + FREE_ARRAY(vm, Entry, table->entries, table->capacity); table->entries = entries; - table->capacityMask = capacityMask; + table->capacity = capacityMask; } bool tableSet(DictuVM *vm, Table *table, ObjString *key, Value value) { - if (table->count + 1 > table->capacityMask * TABLE_MAX_LOAD) { - int capacity = GROW_CAPACITY(table->capacityMask); + if (table->count + 1 > table->capacity * TABLE_MAX_LOAD) { + int capacity = GROW_CAPACITY(table->capacity); adjustCapacity(vm, table, capacity); } - Entry *entry = findEntry(table->entries, table->capacityMask, key); + Entry *entry = findEntry(table->entries, table->capacity, key); bool isNewKey = entry->key == NULL; entry->key = key; entry->value = value; @@ -99,7 +99,7 @@ bool tableDelete(DictuVM *vm, Table *table, ObjString *key) { if (table->count == 0) return false; // Find the entry. - Entry* entry = findEntry(table->entries, table->capacityMask, key); + Entry *entry = findEntry(table->entries, table->capacity, key); if (entry->key == NULL) return false; // Place a tombstone in the entry. @@ -109,8 +109,8 @@ bool tableDelete(DictuVM *vm, Table *table, ObjString *key) { } void tableAddAll(DictuVM *vm, Table *from, Table *to) { - for (int i = 0; i < from->capacityMask; i++) { - Entry* entry = &from->entries[i]; + for (int i = 0; i < from->capacity; i++) { + Entry *entry = &from->entries[i]; if (entry->key != NULL) { tableSet(vm, to, entry->key, entry->value); } @@ -125,10 +125,10 @@ ObjString *tableFindString(Table *table, const char *chars, int length, // Figure out where to insert it in the table. Use open addressing and // basic linear probing. - uint32_t index = hash & (table->capacityMask - 1); + uint32_t index = hash & (table->capacity - 1); for (;;) { - Entry* entry = &table->entries[index]; + Entry *entry = &table->entries[index]; if (entry->key == NULL) { // Stop if we find an empty non-tombstone entry. if (IS_NIL(entry->value)) return NULL; @@ -139,13 +139,13 @@ ObjString *tableFindString(Table *table, const char *chars, int length, return entry->key; } - index = (index + 1) & (table->capacityMask - 1); + index = (index + 1) & (table->capacity - 1); } } -void tableRemoveWhite(DictuVM *vm, Table* table) { - for (int i = 0; i < table->capacityMask; i++) { - Entry* entry = &table->entries[i]; +void tableRemoveWhite(DictuVM *vm, Table *table) { + for (int i = 0; i < table->capacity; i++) { + Entry *entry = &table->entries[i]; if (entry->key != NULL && !entry->key->obj.isDark) { tableDelete(vm, table, entry->key); } @@ -153,7 +153,7 @@ void tableRemoveWhite(DictuVM *vm, Table* table) { } void grayTable(DictuVM *vm, Table *table) { - for (int i = 0; i < table->capacityMask; i++) { + for (int i = 0; i < table->capacity; i++) { Entry *entry = &table->entries[i]; grayObject(vm, (Obj *) entry->key); grayValue(vm, entry->value); diff --git a/src/vm/table.h b/src/vm/table.h index a4e9cbff..d42e577f 100644 --- a/src/vm/table.h +++ b/src/vm/table.h @@ -14,7 +14,7 @@ typedef struct { typedef struct { int count; - int capacityMask; + int capacity; Entry *entries; } Table; diff --git a/src/vm/vm.c b/src/vm/vm.c index 8819d190..3e6e8306 100644 --- a/src/vm/vm.c +++ b/src/vm/vm.c @@ -2276,7 +2276,7 @@ static DictuInterpretResult run(DictuVM *vm) { ObjClass *klass = AS_CLASS(peek(vm, 0)); // If super class is abstract, ensure we have defined all abstract methods - for (int i = 0; i < klass->abstractMethods.capacityMask + 1; i++) { + for (int i = 0; i < klass->abstractMethods.capacity; i++) { if (klass->abstractMethods.entries[i].key == NULL) { continue; }