From 0a784899934e63e6da6c3554a90bde0795fb9526 Mon Sep 17 00:00:00 2001 From: kichikuou Date: Mon, 5 Aug 2024 07:35:29 +0900 Subject: [PATCH 1/3] Revert delegate memory management changes This reverts the following commits: * 4b1257f "Fix delegate memory management" * 9bb4665 "Add changes to vm.h" (partial revert) * 75d6386 "Fix ResumeSave/ResumeLoad bugs with delegates" (partial revert) We are going to take a different approach to implementing weak references in delegates. --- include/vm.h | 1 - include/vm/heap.h | 1 - include/vm/page.h | 37 +++---- src/heap.c | 8 -- src/page.c | 265 +++++++++++++++++++--------------------------- src/resume.c | 23 ---- src/vm.c | 58 ++++------ 7 files changed, 140 insertions(+), 253 deletions(-) diff --git a/include/vm.h b/include/vm.h index 8fced41..8d5268a 100644 --- a/include/vm.h +++ b/include/vm.h @@ -184,7 +184,6 @@ int vm_save_image(const char *key, const char *path); void vm_load_image(const char *key, const char *path); struct page *vm_load_image_comments(const char *key, const char *path, int *success); int vm_write_image_comments(const char *key, const char *path, struct page *comments); -void vm_register_delegate_structs(struct page *dg, int dg_i); #endif /* VM_PRIVATE */ #endif /* SYSTEM4_VM_H */ diff --git a/include/vm/heap.h b/include/vm/heap.h index 90aa83c..dbbc30c 100644 --- a/include/vm/heap.h +++ b/include/vm/heap.h @@ -65,7 +65,6 @@ bool page_index_valid(int index); bool string_index_valid(int index); struct page *heap_get_page(int index); -struct page *heap_get_struct_page(int index); struct page *heap_get_delegate_page(int index); struct string *heap_get_string(int index); void heap_set_page(int slot, struct page *page); diff --git a/include/vm/page.h b/include/vm/page.h index 8c32a9f..bde2487 100644 --- a/include/vm/page.h +++ b/include/vm/page.h @@ -66,18 +66,11 @@ struct page { int index; enum ain_data_type a_type; }; - union { - // array-specific metadata - struct { - int struct_type; - int rank; - } array; - // struct-specific metadata - struct { - int *delegates; - unsigned nr_delegates; - } struc; - }; + // array-specific metadata + struct { + int struct_type; + int rank; + } array; int nr_vars; union vm_value values[]; }; @@ -120,8 +113,6 @@ int alloc_struct(int no); void init_struct(int no, int slot); void delete_struct(int no, int slot); void create_struct(int no, union vm_value *var); -void struct_register_delegate(int obj, int dg_i); -void struct_unregister_delegate(int obj, int dg_i); // arrays enum ain_data_type array_type(enum ain_data_type type); @@ -140,14 +131,14 @@ int array_find(struct page *page, int start, int end, union vm_value v, int comp void array_reverse(struct page *page); // delegates -void delegate_new_from_method(int dg_i, int obj, int fun); -int delegate_numof(int dg_i); -bool delegate_contains(int dg_i, int obj, int fun); -void delegate_erase(int dg_i, int obj, int fun); -void delegate_append(int dg_i, int obj, int fun); -void delegate_plusa(int dg_i, int add_i); -void delegate_minusa(int dg_i, int minus_i); -void delegate_clear(int dg_i); -void delegate_get(int dg_i, int i, int *obj_out, int *fun_out); +struct page *delegate_new_from_method(int obj, int fun); +int delegate_numof(struct page *page); +bool delegate_contains(struct page *dst, int obj, int fun); +void delegate_erase(struct page *page, int obj, int fun); +struct page *delegate_append(struct page *dst, int obj, int fun); +struct page *delegate_plusa(struct page *dst, struct page *add); +struct page *delegate_minusa(struct page *dst, struct page *minus); +struct page *delegate_clear(struct page *page); +void delegate_get(struct page *page, int i, int *obj_out, int *fun_out); #endif /* SYSTEM4_PAGE_H */ diff --git a/src/heap.c b/src/heap.c index 107c45e..de100b2 100644 --- a/src/heap.c +++ b/src/heap.c @@ -241,14 +241,6 @@ struct string *heap_get_string(int index) return heap[index].s; } -struct page *heap_get_struct_page(int index) -{ - struct page *page = heap_get_page(index); - if (unlikely(!page || page->type != STRUCT_PAGE)) - VM_ERROR("Not a struct page: %d", index); - return page; -} - struct page *heap_get_delegate_page(int index) { struct page *page = heap_get_page(index); diff --git a/src/page.c b/src/page.c index 66b8440..53371a1 100644 --- a/src/page.c +++ b/src/page.c @@ -176,8 +176,13 @@ enum ain_data_type variable_type(struct page *page, int varno, int *struct_type, *array_rank = page->array.rank - 1; return page->array.rank > 1 ? page->a_type : array_type(page->a_type); case DELEGATE_PAGE: - // XXX: we return void here because objects in a delegate page aren't - // reference counted + if (varno % 2 == 0) { + if (struct_type && page->values[varno].i >= 0) + *struct_type = heap_get_page(page->values[varno].i)->index; + if (array_rank) + *array_rank = -1; + return AIN_REF_STRUCT; + } return AIN_VOID; } return AIN_VOID; @@ -196,37 +201,14 @@ void delete_page_vars(struct page *page) } } -static void delegate_delete_object(int dg_i, int obj); - void delete_page(int slot) { struct page *page = heap_get_page(slot); if (!page) return; - if (page->type == STRUCT_PAGE) { - struct ain_struct *s = &ain->structures[page->index]; - if (s->destructor > 0) { - vm_call(s->destructor, slot); - } - - // remove the object from all delegates - for (int i = 0; i < page->struc.nr_delegates; i++) { - delegate_delete_object(page->struc.delegates[i], slot); - } - free(page->struc.delegates); - page->struc.delegates = NULL; - page->struc.nr_delegates = 0; + delete_struct(page->index, slot); } - - if (page->type == DELEGATE_PAGE) { - for (int i = 0; i < page->nr_vars; i += 2) { - if (page->values[i].i < 0) - continue; - struct_unregister_delegate(page->values[i].i, slot); - } - } - delete_page_vars(page); free_page(page); } @@ -239,17 +221,11 @@ struct page *copy_page(struct page *src) if (!src) return NULL; struct page *dst = alloc_page(src->type, src->index, src->nr_vars); - if (src->type == ARRAY_PAGE) { - dst->array = src->array; - } else if (src->type == STRUCT_PAGE) { - dst->struc.delegates = NULL; - dst->struc.nr_delegates = 0; - } + dst->array = src->array; for (int i = 0; i < src->nr_vars; i++) { dst->values[i] = vm_copy(src->values[i], variable_type(src, i, NULL, NULL)); } - return dst; } @@ -257,17 +233,14 @@ int alloc_struct(int no) { struct ain_struct *s = &ain->structures[no]; int slot = heap_alloc_slot(VM_PAGE); - struct page *page = alloc_page(STRUCT_PAGE, no, s->nr_members); - page->struc.delegates = NULL; - page->struc.nr_delegates = 0; + heap_set_page(slot, alloc_page(STRUCT_PAGE, no, s->nr_members)); for (int i = 0; i < s->nr_members; i++) { if (s->members[i].type.data == AIN_STRUCT) { - page->values[i].i = alloc_struct(s->members[i].type.struc); + heap[slot].page->values[i].i = alloc_struct(s->members[i].type.struc); } else { - page->values[i] = variable_initval(s->members[i].type.data); + heap[slot].page->values[i] = variable_initval(s->members[i].type.data); } } - heap_set_page(slot, page); return slot; } @@ -284,6 +257,14 @@ void init_struct(int no, int slot) } } +void delete_struct(int no, int slot) +{ + struct ain_struct *s = &ain->structures[no]; + if (s->destructor > 0) { + vm_call(s->destructor, slot); + } +} + void create_struct(int no, union vm_value *var) { var->i = alloc_struct(no); @@ -646,38 +627,18 @@ void array_reverse(struct page *page) } } -void struct_register_delegate(int obj, int dg_i) +struct page *delegate_new_from_method(int obj, int fun) { - struct page *page = heap_get_struct_page(obj); - - // don't add duplicates - for (int i = 0; i < page->struc.nr_delegates; i++) { - if (page->struc.delegates[i] == obj) - return; + struct page *page = alloc_page(DELEGATE_PAGE, 0, 2); + page->values[0].i = obj; + page->values[1].i = fun; + if (obj >= 0) { + heap_ref(obj); } - - page->struc.delegates = xrealloc(page->struc.delegates, - (page->struc.nr_delegates + 1) * sizeof(int)); - page->struc.delegates[page->struc.nr_delegates++] = dg_i; + return page; } -void struct_unregister_delegate(int obj, int dg_i) -{ - struct page *page = heap_get_struct_page(obj); - for (int i = 0; i < page->struc.nr_delegates; i++) { - if (page->struc.delegates[i] != dg_i) - continue; - if (page->struc.nr_delegates > 1) { - // swap last entry into delegates[i] - page->struc.delegates[i] = page->struc.delegates[page->struc.nr_delegates - 1]; - } - page->struc.nr_delegates--; - return; - } - VM_ERROR("delegate is not registered to object"); -} - -static bool _delegate_contains(struct page *dst, int obj, int fun) +bool delegate_contains(struct page *dst, int obj, int fun) { if (!dst) return false; @@ -688,117 +649,105 @@ static bool _delegate_contains(struct page *dst, int obj, int fun) return false; } -bool delegate_contains(int dg_i, int obj, int fun) +struct page *delegate_append(struct page *dst, int obj, int fun) { - return _delegate_contains(heap_get_delegate_page(dg_i), obj, fun); -} + if (!dst) + return delegate_new_from_method(obj, fun); + if (dst->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + if (delegate_contains(dst, obj, fun)) + return dst; -void delegate_append(int dg_i, int obj, int fun) -{ - struct page *dg = heap_get_delegate_page(dg_i); - if (dg && _delegate_contains(dg, obj, fun)) - return; - if (!dg) { - dg = alloc_page(DELEGATE_PAGE, 0, 2); - dg->values[0].i = obj; - dg->values[1].i = fun; - } else { - dg = xrealloc(dg, sizeof(struct page) + sizeof(union vm_value) * (dg->nr_vars + 2)); - dg->values[dg->nr_vars+0].i = obj; - dg->values[dg->nr_vars+1].i = fun; - dg->nr_vars += 2; - } + dst = xrealloc(dst, sizeof(struct page) + sizeof(union vm_value) * (dst->nr_vars + 2)); + dst->values[dst->nr_vars+0].i = obj; + dst->values[dst->nr_vars+1].i = fun; + dst->nr_vars += 2; if (obj >= 0) - struct_register_delegate(obj, dg_i); - heap_set_page(dg_i, dg); + heap_ref(obj); + return dst; } -int delegate_numof(int dg_i) +int delegate_numof(struct page *page) { - struct page *dg = heap_get_delegate_page(dg_i); - if (!dg) + if (!page) return 0; - return dg->nr_vars / 2; + if (page->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + return page->nr_vars / 2; } -static void delegate_delete_object(int dg_i, int obj) +void delegate_erase(struct page *page, int obj, int fun) { - struct page *dg = heap_get_delegate_page(dg_i); - if (!dg) { - WARNING("Tried to delete object from empty delegate"); - return; - } - for (int i = 0; i < dg->nr_vars; i += 2) { - if (dg->values[i].i != obj) - continue; - for (int j = i+2; j < dg->nr_vars; j += 2) { - dg->values[j-2].i = dg->values[j+0].i; - dg->values[j-1].i = dg->values[j+1].i; - } - dg->nr_vars -= 2; - i -= 2; - } -} - -void delegate_erase(int dg_i, int obj, int fun) -{ - struct page *dg = heap_get_delegate_page(dg_i); - if (!dg) - return; - for (int i = 0; i < dg->nr_vars; i += 2) { - if (dg->values[i].i != obj || dg->values[i+1].i != fun) - continue; - if (dg->values[i].i >= 0) - struct_unregister_delegate(obj, dg_i); - for (int j = i+2; j < dg->nr_vars; j += 2) { - dg->values[j-2].i = dg->values[j+0].i; - dg->values[j-1].i = dg->values[j+1].i; - } - dg->nr_vars -= 2; - break; - } -} - -void delegate_plusa(int dg_i, int add_i) -{ - struct page *add = heap_get_delegate_page(add_i); - if (!add) - return; - for (int i = 0; i < add->nr_vars; i += 2) { - delegate_append(dg_i, add->values[i].i, add->values[i+1].i); - } -} - -void delegate_minusa(int dg_i, int minus_i) -{ - struct page *minus = heap_get_delegate_page(minus_i); - if (!minus) - return; - for (int i = 0; i < minus->nr_vars; i += 2) { - delegate_erase(dg_i, minus->values[i].i, minus->values[i+1].i); - } -} - -void delegate_clear(int dg_i) -{ - struct page *page = heap_get_delegate_page(dg_i); if (!page) return; + if (page->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + for (int i = 0; i < page->nr_vars; i+= 2) { + if (page->values[i].i == obj && page->values[i+1].i == fun) { + if (page->values[i].i >= 0) + heap_unref(page->values[i].i); + for (int j = i+2; j < page->nr_vars; j += 2) { + page->values[j-2].i = page->values[j+0].i; + page->values[j-1].i = page->values[j+1].i; + } + page->nr_vars -= 2; + break; + } + } +} + +struct page *delegate_plusa(struct page *dst, struct page *add) +{ + if (!add) + return dst; + if ((dst && dst->type != DELEGATE_PAGE) || add->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + + for (int i = 0; i < add->nr_vars; i += 2) { + dst = delegate_append(dst, add->values[i].i, add->values[i+1].i); + } + return dst; +} + +struct page *delegate_minusa(struct page *dst, struct page *minus) +{ + if (!dst) + return NULL; + if (!minus) + return dst; + if (dst->type != DELEGATE_PAGE || minus->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + + for (int i = 0; i < minus->nr_vars; i += 2) { + delegate_erase(dst, minus->values[i].i, minus->values[i+1].i); + } + + return dst; +} + +struct page *delegate_clear(struct page *page) +{ + if (!page) + return NULL; + if (page->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); for (int i = 0; i < page->nr_vars; i += 2) { if (page->values[i].i >= 0) - struct_unregister_delegate(page->values[i].i, dg_i); - page->values[i+0].i = -1; + heap_unref(page->values[i].i); + page->values[i].i = -1; page->values[i+1].i = -1; } page->index = 0; page->nr_vars = 0; + return page; } -void delegate_get(int dg_i, int i, int *obj_out, int *fun_out) +void delegate_get(struct page *page, int i, int *obj_out, int *fun_out) { - struct page *dg = heap_get_delegate_page(dg_i); - if (i * 2 >= dg->nr_vars) + if (page->type != DELEGATE_PAGE) + VM_ERROR("Not a delegate"); + if (i*2 >= page->nr_vars) VM_ERROR("Invalid delegate index: %d", i); - *obj_out = dg->values[i*2+0].i; - *fun_out = dg->values[i*2+1].i; + *obj_out = page->values[i*2].i; + *fun_out = page->values[i*2+1].i; } diff --git a/src/resume.c b/src/resume.c index 385abbe..f35a5ef 100644 --- a/src/resume.c +++ b/src/resume.c @@ -500,20 +500,8 @@ static void alloc_heap_slot(int slot) heap_free_ptr++; } -struct delegate_list { - int *slots; - int n; -}; - -void delegate_list_add(struct delegate_list *list, int slot) -{ - list->slots = xrealloc_array(list->slots, list->n, list->n+1, sizeof(int)); - list->slots[list->n++] = slot; -} - static void load_json_heap(cJSON *json) { - struct delegate_list delegates = {0}; delete_heap(); cJSON *item; @@ -533,8 +521,6 @@ static void load_json_heap(cJSON *json) load_json_string(slot, value); } else if (cJSON_IsObject(value)) { load_json_page(slot, value); - if (heap[slot].page->type == DELEGATE_PAGE) - delegate_list_add(&delegates, slot); } else if (cJSON_IsNull(value)) { heap[slot].type = VM_PAGE; heap[slot].page = NULL; @@ -542,13 +528,6 @@ static void load_json_heap(cJSON *json) invalid_save_data("Invalid heap data"); } } - - for (int i = 0; i < delegates.n; i++) { - int slot = delegates.slots[i]; - struct page *page = heap_get_delegate_page(slot); - vm_register_delegate_structs(page, slot); - } - free(delegates.slots); } static int resolve_func_symbol(struct rsave_symbol *sym) @@ -636,8 +615,6 @@ static void load_rsave_struct(int slot, struct rsave_heap_struct *s) { int struct_index = resolve_struct_symbol(&s->struct_type); struct page *page = alloc_page(STRUCT_PAGE, struct_index, s->nr_slots); - page->struc.delegates = NULL; - page->struc.nr_delegates = 0; // type check struct ain_struct *as = &ain->structures[struct_index]; diff --git a/src/vm.c b/src/vm.c index 7f6b6e1..7099907 100644 --- a/src/vm.c +++ b/src/vm.c @@ -241,37 +241,15 @@ int vm_copy_page(struct page *page) return slot; } -void vm_register_delegate_structs(struct page *dg, int dg_i) -{ - for (int i = 0; i < dg->nr_vars; i += 2) { - if (dg->values[i].i < 0) - continue; - struct_register_delegate(dg->values[i].i, dg_i); - } -} - -int vm_copy_delegate_page(int dg_i) -{ - struct page *page = heap_get_page(dg_i); - int slot = vm_copy_page(page); - - if (page) { - vm_register_delegate_structs(page, slot); - } - - return slot; -} - union vm_value vm_copy(union vm_value v, enum ain_data_type type) { switch (type) { case AIN_STRING: return (union vm_value) { .i = vm_string_ref(heap_get_string(v.i)) }; case AIN_STRUCT: + case AIN_DELEGATE: case AIN_ARRAY_TYPE: return (union vm_value) { .i = vm_copy_page(heap_get_page(v.i)) }; - case AIN_DELEGATE: - return (union vm_value) { .i = vm_copy_delegate_page(v.i) }; case AIN_REF_TYPE: heap_ref(v.i); return v; @@ -412,7 +390,7 @@ static void delegate_call(int dg_no, int return_address) int dg_index = stack_peek(0).i; int obj, fun; - delegate_get(dg_page, dg_index, &obj, &fun); + delegate_get(heap_get_delegate_page(dg_page), dg_index, &obj, &fun); int slot = _function_call(fun, return_address); @@ -2162,14 +2140,15 @@ static enum opcode execute_instruction(enum opcode opcode) int obj = stack_pop().i; int dg_i = stack_pop().i; delete_page(dg_i); - delegate_append(dg_i, obj, fun); + heap_set_page(dg_i, delegate_new_from_method(obj, fun)); break; } case DG_SET: { int fun = stack_pop().i; int obj = stack_pop().i; int dg_i = stack_pop().i; - delegate_append(dg_i, obj, fun); + struct page *dg = heap_get_delegate_page(dg_i); + heap_set_page(dg_i, delegate_append(dg, obj, fun)); break; } case DG_CALL: { // DG_TYPE, ADDR @@ -2181,9 +2160,9 @@ static enum opcode execute_instruction(enum opcode opcode) int return_values = (ain->delegates[dg].return_type.data != AIN_VOID) ? 1 : 0; int dg_page = stack_peek(1 + return_values).i; int dg_index = stack_peek(0 + return_values).i; - if (dg_index < delegate_numof(dg_page)) { + if (dg_index < delegate_numof(heap_get_page(dg_page))) { int obj, fun; - delegate_get(dg_page, dg_index, &obj, &fun); + delegate_get(heap_get_delegate_page(dg_page), dg_index, &obj, &fun); // pop previous return value if (ain->delegates[dg].return_type.data != AIN_VOID) { stack_pop(); @@ -2209,32 +2188,32 @@ static enum opcode execute_instruction(enum opcode opcode) } case DG_NUMOF: { int dg = stack_pop().i; - stack_push(delegate_numof(dg)); + stack_push(delegate_numof(heap_get_delegate_page(dg))); break; } case DG_EXIST: { int fun = stack_pop().i; int obj = stack_pop().i; int dg_i = stack_pop().i; - stack_push(delegate_contains(dg_i, obj, fun)); + stack_push(delegate_contains(heap_get_delegate_page(dg_i), obj, fun)); break; } case DG_ERASE: { int fun = stack_pop().i; int obj = stack_pop().i; int dg_i = stack_pop().i; - delegate_erase(dg_i, obj, fun); + delegate_erase(heap_get_delegate_page(dg_i), obj, fun); break; } case DG_CLEAR: { int slot = stack_pop().i; if (!slot) break; - delegate_clear(slot); + heap_set_page(slot, delegate_clear(heap_get_delegate_page(slot))); break; } case DG_COPY: { - stack_push(vm_copy_delegate_page(stack_pop().i)); + stack_push(vm_copy_page(heap_get_delegate_page(stack_pop().i))); break; } case DG_ASSIGN: { @@ -2244,21 +2223,24 @@ static enum opcode execute_instruction(enum opcode opcode) struct page *new_dg = copy_page(set); delete_page(dst_i); heap_set_page(dst_i, new_dg); - vm_register_delegate_structs(new_dg, dst_i); stack_push(set_i); break; } case DG_PLUSA: { int add_i = stack_pop().i; int dst_i = stack_pop().i; - delegate_plusa(dst_i, add_i); + struct page *add = heap_get_delegate_page(add_i); + struct page *dst = heap_get_delegate_page(dst_i); + heap_set_page(dst_i, delegate_plusa(dst, add)); stack_push(add_i); break; } case DG_MINUSA: { int minus_i = stack_pop().i; int dst_i = stack_pop().i; - delegate_minusa(dst_i, minus_i); + struct page *minus = heap_get_delegate_page(minus_i); + struct page *dst = heap_get_delegate_page(dst_i); + heap_set_page(dst_i, delegate_minusa(dst, minus)); stack_push(minus_i); break; } @@ -2269,9 +2251,7 @@ static enum opcode execute_instruction(enum opcode opcode) case DG_NEW_FROM_METHOD: { int fun = stack_pop().i; int obj = stack_pop().i; - int dg_i = heap_alloc_page(NULL); - delegate_append(dg_i, obj, fun); - stack_push(dg_i); + stack_push(heap_alloc_page(delegate_new_from_method(obj, fun))); break; } case DG_CALLBEGIN: { // DG_TYPE From 12ddb585fb419888b85df0fc138f516edc6432ec Mon Sep 17 00:00:00 2001 From: kichikuou Date: Mon, 5 Aug 2024 09:31:19 +0900 Subject: [PATCH 2/3] Rework delegate implementation Delegates "weakly" reference objects, meaning that referenced objects can be deleted. Delegate invocations must not invoke on deleted objects. This is achieved as follows: * Each heap object has a sequential number. * Each delegate object is backed by a page storing (object, function, seq) triples. * Deleted objects can be detected by comparing the sequential number stored in the delegate with the sequential number of the object currently on the heap. This is consistent with the AliceSoft's implementation (presumed from the contents of resume save). This implementation removes dead objects from the delegate in DG_CALL and DG_NUMOF instructions. (We could do that more often, e.g. when an object is added to a delegate.) This breaks existing resume saves that contain delegates. For games that predate delegate support, this does not affect the save format. --- include/vm/heap.h | 4 ++ include/vm/page.h | 6 +-- src/heap.c | 9 +++++ src/page.c | 93 ++++++++++++++++++++++++++++------------------- src/resume.c | 18 ++++++++- src/vm.c | 83 +++++++++++++++++++----------------------- 6 files changed, 127 insertions(+), 86 deletions(-) diff --git a/include/vm/heap.h b/include/vm/heap.h index dbbc30c..9eed1a3 100644 --- a/include/vm/heap.h +++ b/include/vm/heap.h @@ -33,6 +33,7 @@ enum vm_pointer_type { // Heap-backed objects. Reference counted. struct vm_pointer { int ref; + uint32_t seq; enum vm_pointer_type type; union { struct string *s; @@ -60,6 +61,8 @@ void heap_ref(int slot); void heap_unref(int slot); void exit_unref(int slot); +uint32_t heap_get_seq(int slot); + bool heap_index_valid(int index); bool page_index_valid(int index); bool string_index_valid(int index); @@ -86,6 +89,7 @@ void heap_guarantee(unsigned headroom); #ifdef VM_PRIVATE +extern uint32_t heap_next_seq; extern int32_t *heap_free_stack; extern size_t heap_free_ptr; diff --git a/include/vm/page.h b/include/vm/page.h index bde2487..1f289e4 100644 --- a/include/vm/page.h +++ b/include/vm/page.h @@ -50,8 +50,8 @@ enum page_type { * Multi-dimensional arrays are implemented as a tree of pages (meaning the * whole array is NOT contiguous in memory). * - * Delegates: each delegate object is backed by a page storing object/function - * pairs. + * Delegates: each delegate object is backed by a page storing (object, + * function, seq) triples. */ struct page { enum page_type type; @@ -139,6 +139,6 @@ struct page *delegate_append(struct page *dst, int obj, int fun); struct page *delegate_plusa(struct page *dst, struct page *add); struct page *delegate_minusa(struct page *dst, struct page *minus); struct page *delegate_clear(struct page *page); -void delegate_get(struct page *page, int i, int *obj_out, int *fun_out); +bool delegate_get(struct page *page, int i, int *obj_out, int *fun_out); #endif /* SYSTEM4_PAGE_H */ diff --git a/src/heap.c b/src/heap.c index de100b2..5f371ab 100644 --- a/src/heap.c +++ b/src/heap.c @@ -30,6 +30,7 @@ struct vm_pointer *heap = NULL; size_t heap_size = 0; +uint32_t heap_next_seq; // Heap free list // This is a list of unused indices into the 'heap' array. @@ -86,6 +87,7 @@ void heap_init(void) heap_free_stack[i] = i; } heap_free_ptr = 1; // global page at index 0 + heap_next_seq = 1; } int32_t heap_alloc_slot(enum vm_pointer_type type) @@ -96,6 +98,7 @@ int32_t heap_alloc_slot(enum vm_pointer_type type) int32_t slot = heap_free_stack[heap_free_ptr++]; heap[slot].ref = 1; + heap[slot].seq = heap_next_seq++; heap[slot].type = type; #ifdef DEBUG_HEAP heap[slot].alloc_addr = instr_ptr; @@ -110,6 +113,7 @@ int32_t heap_alloc_slot(enum vm_pointer_type type) static void heap_free_slot(int32_t slot) { + heap[slot].seq = 0; heap_free_stack[--heap_free_ptr] = slot; } @@ -212,6 +216,11 @@ void exit_unref(int slot) heap_free_slot(slot); } +uint32_t heap_get_seq(int slot) +{ + return heap_index_valid(slot) ? heap[slot].seq : 0; +} + bool heap_index_valid(int index) { return index >= 0 && (size_t)index < heap_size && heap[index].ref > 0; diff --git a/src/page.c b/src/page.c index 53371a1..746cb47 100644 --- a/src/page.c +++ b/src/page.c @@ -176,13 +176,8 @@ enum ain_data_type variable_type(struct page *page, int varno, int *struct_type, *array_rank = page->array.rank - 1; return page->array.rank > 1 ? page->a_type : array_type(page->a_type); case DELEGATE_PAGE: - if (varno % 2 == 0) { - if (struct_type && page->values[varno].i >= 0) - *struct_type = heap_get_page(page->values[varno].i)->index; - if (array_rank) - *array_rank = -1; - return AIN_REF_STRUCT; - } + // XXX: we return void here because objects in a delegate page aren't + // reference counted return AIN_VOID; } return AIN_VOID; @@ -629,12 +624,10 @@ void array_reverse(struct page *page) struct page *delegate_new_from_method(int obj, int fun) { - struct page *page = alloc_page(DELEGATE_PAGE, 0, 2); + struct page *page = alloc_page(DELEGATE_PAGE, 0, 3); page->values[0].i = obj; page->values[1].i = fun; - if (obj >= 0) { - heap_ref(obj); - } + page->values[2].i = heap_get_seq(obj); return page; } @@ -642,8 +635,10 @@ bool delegate_contains(struct page *dst, int obj, int fun) { if (!dst) return false; - for (int i = 0; i < dst->nr_vars; i += 2) { - if (dst->values[i].i == obj && dst->values[i+1].i == fun) + for (int i = 0; i < dst->nr_vars; i += 3) { + if (dst->values[i].i == obj && + dst->values[i+1].i == fun && + dst->values[i+2].i == heap_get_seq(obj)) return true; } return false; @@ -658,12 +653,11 @@ struct page *delegate_append(struct page *dst, int obj, int fun) if (delegate_contains(dst, obj, fun)) return dst; - dst = xrealloc(dst, sizeof(struct page) + sizeof(union vm_value) * (dst->nr_vars + 2)); + dst = xrealloc(dst, sizeof(struct page) + sizeof(union vm_value) * (dst->nr_vars + 3)); dst->values[dst->nr_vars+0].i = obj; dst->values[dst->nr_vars+1].i = fun; - dst->nr_vars += 2; - if (obj >= 0) - heap_ref(obj); + dst->values[dst->nr_vars+2].i = heap_get_seq(obj); + dst->nr_vars += 3; return dst; } @@ -673,7 +667,20 @@ int delegate_numof(struct page *page) return 0; if (page->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - return page->nr_vars / 2; + + // garbage collection + for (int i = 0; i < page->nr_vars; i += 3) { + if (heap_get_seq(page->values[i].i) != page->values[i+2].i) { + for (int j = i+3; j < page->nr_vars; j += 3) { + page->values[j-3].i = page->values[j+0].i; + page->values[j-2].i = page->values[j+1].i; + page->values[j-1].i = page->values[j+2].i; + } + page->nr_vars -= 3; + i -= 3; + } + } + return page->nr_vars / 3; } void delegate_erase(struct page *page, int obj, int fun) @@ -682,15 +689,14 @@ void delegate_erase(struct page *page, int obj, int fun) return; if (page->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - for (int i = 0; i < page->nr_vars; i+= 2) { + for (int i = 0; i < page->nr_vars; i += 3) { if (page->values[i].i == obj && page->values[i+1].i == fun) { - if (page->values[i].i >= 0) - heap_unref(page->values[i].i); - for (int j = i+2; j < page->nr_vars; j += 2) { - page->values[j-2].i = page->values[j+0].i; - page->values[j-1].i = page->values[j+1].i; + for (int j = i+3; j < page->nr_vars; j += 3) { + page->values[j-3].i = page->values[j+0].i; + page->values[j-2].i = page->values[j+1].i; + page->values[j-1].i = page->values[j+2].i; } - page->nr_vars -= 2; + page->nr_vars -= 3; break; } } @@ -703,8 +709,9 @@ struct page *delegate_plusa(struct page *dst, struct page *add) if ((dst && dst->type != DELEGATE_PAGE) || add->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - for (int i = 0; i < add->nr_vars; i += 2) { - dst = delegate_append(dst, add->values[i].i, add->values[i+1].i); + for (int i = 0; i < add->nr_vars; i += 3) { + if (heap_get_seq(add->values[i].i) == add->values[i+2].i) + dst = delegate_append(dst, add->values[i].i, add->values[i+1].i); } return dst; } @@ -718,8 +725,9 @@ struct page *delegate_minusa(struct page *dst, struct page *minus) if (dst->type != DELEGATE_PAGE || minus->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - for (int i = 0; i < minus->nr_vars; i += 2) { - delegate_erase(dst, minus->values[i].i, minus->values[i+1].i); + for (int i = 0; i < minus->nr_vars; i += 3) { + if (heap_get_seq(minus->values[i].i) == minus->values[i+2].i) + delegate_erase(dst, minus->values[i].i, minus->values[i+1].i); } return dst; @@ -731,23 +739,34 @@ struct page *delegate_clear(struct page *page) return NULL; if (page->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - for (int i = 0; i < page->nr_vars; i += 2) { - if (page->values[i].i >= 0) - heap_unref(page->values[i].i); + for (int i = 0; i < page->nr_vars; i += 3) { page->values[i].i = -1; page->values[i+1].i = -1; + page->values[i+2].i = 0; } page->index = 0; page->nr_vars = 0; return page; } -void delegate_get(struct page *page, int i, int *obj_out, int *fun_out) +bool delegate_get(struct page *page, int i, int *obj_out, int *fun_out) { + if (!page) + return false; if (page->type != DELEGATE_PAGE) VM_ERROR("Not a delegate"); - if (i*2 >= page->nr_vars) - VM_ERROR("Invalid delegate index: %d", i); - *obj_out = page->values[i*2].i; - *fun_out = page->values[i*2+1].i; + while (i*3 < page->nr_vars) { + if (heap_get_seq(page->values[i*3].i) == page->values[i*3+2].i) { + *obj_out = page->values[i*3].i; + *fun_out = page->values[i*3+1].i; + return true; + } + for (int j = (i + 1) * 3; j < page->nr_vars; j += 3) { + page->values[j-3].i = page->values[j+0].i; + page->values[j-2].i = page->values[j+1].i; + page->values[j-1].i = page->values[j+2].i; + } + page->nr_vars -= 3; + } + return false; } diff --git a/src/resume.c b/src/resume.c index f35a5ef..f3d11b5 100644 --- a/src/resume.c +++ b/src/resume.c @@ -101,6 +101,9 @@ static cJSON *heap_item_to_json(int i, possibly_unused void *_) cJSON_AddItemToArray(item, cJSON_CreateString(heap[i].s->text)); break; } + if (ain->nr_delegates > 0) { + cJSON_AddItemToArray(item, cJSON_CreateNumber(heap[i].seq)); + } return item; } @@ -147,6 +150,9 @@ static cJSON *vm_image_to_json(const char *key) cJSON_AddItemToObject(image, "call-stack", call_stack_to_json()); cJSON_AddItemToObject(image, "stack", stack_to_json()); cJSON_AddNumberToObject(image, "ip", instr_ptr); + if (ain->nr_delegates > 0) { + cJSON_AddNumberToObject(image, "next_seq", heap_next_seq); + } return image; } @@ -507,15 +513,19 @@ static void load_json_heap(cJSON *json) cJSON *item; cJSON_ArrayForEach(item, json) { type_check(cJSON_Array, item); - if (cJSON_GetArraySize(item) != 3) + if (cJSON_GetArraySize(item) < 3) invalid_save_data("Invalid heap data"); int slot = type_check(cJSON_Number, cJSON_GetArrayItem(item, 0))->valueint; int ref = type_check(cJSON_Number, cJSON_GetArrayItem(item, 1))->valueint; cJSON *value = cJSON_GetArrayItem(item, 2); + int seq = ain->nr_delegates > 0 + ? type_check(cJSON_Number, cJSON_GetArrayItem(item, 3))->valueint + : slot; alloc_heap_slot(slot); heap[slot].ref = ref; + heap[slot].seq = seq; if (cJSON_IsString(value)) { load_json_string(slot, value); @@ -770,6 +780,12 @@ static void load_json_image(const char *key, const char *path) load_json_call_stack(type_check(cJSON_Array, cJSON_GetObjectItem(save, "call-stack"))); load_json_stack(type_check(cJSON_Array, cJSON_GetObjectItem(save, "stack"))); instr_ptr = ip->valueint; + if (ain->nr_delegates > 0) { + cJSON *next_seq = type_check(cJSON_Number, cJSON_GetObjectItem(save, "next_seq")); + heap_next_seq = next_seq->valueint; + } else { + heap_next_seq = heap_size; + } cJSON_Delete(save); } diff --git a/src/vm.c b/src/vm.c index 7099907..8f3e7cd 100644 --- a/src/vm.c +++ b/src/vm.c @@ -385,24 +385,47 @@ static void vm_execute(void); static void delegate_call(int dg_no, int return_address) { - // stack: [arg0, ..., dg_page, dg_index] - int dg_page = stack_peek(1).i; - int dg_index = stack_peek(0).i; + if (dg_no < 0 || dg_no >= ain->nr_delegates) + VM_ERROR("Invalid delegate index"); + // stack: [arg0, ..., dg_page, dg_index, [return_value]] + int return_values = (ain->delegates[dg_no].return_type.data != AIN_VOID) ? 1 : 0; + int dg_page = stack_peek(1 + return_values).i; + int dg_index = stack_peek(0 + return_values).i; int obj, fun; - delegate_get(heap_get_delegate_page(dg_page), dg_index, &obj, &fun); + if (delegate_get(heap_get_delegate_page(dg_page), dg_index, &obj, &fun)) { + // pop previous return value + if (ain->delegates[dg_no].return_type.data != AIN_VOID) { + stack_pop(); + } - int slot = _function_call(fun, return_address); + int slot = _function_call(fun, instr_ptr + instruction_width(DG_CALL)); - // copy arguments into local page - struct ain_function_type *dg = &ain->delegates[dg_no]; - for (int i = 0; i < dg->nr_arguments; i++) { - union vm_value arg = stack_peek((dg->nr_arguments + 1) - i); - heap[slot].page->values[i] = vm_copy(arg, dg->variables[i].type.data); + // copy arguments into local page + struct ain_function_type *dg = &ain->delegates[dg_no]; + for (int i = 0; i < dg->nr_arguments; i++) { + union vm_value arg = stack_peek((dg->nr_arguments + 1) - i); + heap[slot].page->values[i] = vm_copy(arg, dg->variables[i].type.data); + } + + call_stack[call_stack_ptr-1].struct_page = obj; + call_stack[call_stack_ptr-1].delegate = dg_no; + } else { + // call finished: clean up stack and jump to return address + union vm_value r; + if (return_values) { + r = stack_pop(); + } + stack_pop(); // dg_index + stack_pop(); // dg_page + for (int i = ain->delegates[dg_no].nr_variables - 1; i >= 0; i--) { + variable_fini(stack_pop(), ain->delegates[dg_no].variables[i].type.data); + } + if (return_values) { + stack_push(r); + } + instr_ptr = get_argument(1); } - - call_stack[call_stack_ptr-1].struct_page = obj; - call_stack[call_stack_ptr-1].delegate = dg_no; } void vm_call(int fno, int struct_page) @@ -2152,38 +2175,7 @@ static enum opcode execute_instruction(enum opcode opcode) break; } case DG_CALL: { // DG_TYPE, ADDR - int dg = get_argument(0); - if (dg < 0 || dg >= ain->nr_delegates) - VM_ERROR("Invalid delegate index"); - - // stack: [arg0, ..., dg_page, dg_index, [return_value]] - int return_values = (ain->delegates[dg].return_type.data != AIN_VOID) ? 1 : 0; - int dg_page = stack_peek(1 + return_values).i; - int dg_index = stack_peek(0 + return_values).i; - if (dg_index < delegate_numof(heap_get_page(dg_page))) { - int obj, fun; - delegate_get(heap_get_delegate_page(dg_page), dg_index, &obj, &fun); - // pop previous return value - if (ain->delegates[dg].return_type.data != AIN_VOID) { - stack_pop(); - } - delegate_call(dg, instr_ptr + instruction_width(DG_CALL)); - } else { - // call finished: clean up stack and jump to return address - union vm_value r; - if (return_values) { - r = stack_pop(); - } - stack_pop(); // dg_index - stack_pop(); // dg_page - for (int i = ain->delegates[dg].nr_variables - 1; i >= 0; i--) { - variable_fini(stack_pop(), ain->delegates[dg].variables[i].type.data); - } - if (return_values) { - stack_push(r); - } - instr_ptr = get_argument(1); - } + delegate_call(get_argument(0), get_argument(1)); break; } case DG_NUMOF: { @@ -2350,6 +2342,7 @@ int vm_execute_ain(struct ain *program) // Initialize globals heap[0].ref = 1; + heap[0].seq = heap_next_seq++; heap_set_page(0, alloc_page(GLOBAL_PAGE, 0, ain->nr_globals)); for (int i = 0; i < ain->nr_globals; i++) { if (ain->globals[i].type.data == AIN_STRUCT) { From 2046ad4d065016955a77095476c7661efa5c4fea Mon Sep 17 00:00:00 2001 From: kichikuou Date: Mon, 5 Aug 2024 14:09:52 +0900 Subject: [PATCH 3/3] Remove delegate field from struct function_call Now dg_index in the stack is incremented by delegate_call(). --- include/vm.h | 1 - src/resume.c | 5 ----- src/vm.c | 13 ++----------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/include/vm.h b/include/vm.h index 8d5268a..b156d90 100644 --- a/include/vm.h +++ b/include/vm.h @@ -155,7 +155,6 @@ struct function_call { uint32_t return_address; int32_t page_slot; int32_t struct_page; - int32_t delegate; }; extern struct function_call call_stack[4096]; diff --git a/src/resume.c b/src/resume.c index f3d11b5..6651ad1 100644 --- a/src/resume.c +++ b/src/resume.c @@ -119,8 +119,6 @@ static cJSON *funcall_to_json(struct function_call *call) cJSON_AddNumberToObject(json, "return-address", call->return_address); cJSON_AddNumberToObject(json, "local-page", call->page_slot); cJSON_AddNumberToObject(json, "struct-page", call->struct_page); - if (call->delegate >= 0) - cJSON_AddNumberToObject(json, "delegate", call->delegate); return json; } @@ -674,13 +672,11 @@ static void load_json_call_stack(cJSON *json) cJSON *item; cJSON_ArrayForEach(item, json) { type_check(cJSON_Object, item); - cJSON *delegate = cJSON_GetObjectItem(item, "delegate"); call_stack[call_stack_ptr++] = (struct function_call) { .fno = type_check(cJSON_Number, cJSON_GetObjectItem(item, "function"))->valueint, .return_address = type_check(cJSON_Number, cJSON_GetObjectItem(item, "return-address"))->valueint, .page_slot = type_check(cJSON_Number, cJSON_GetObjectItem(item, "local-page"))->valueint, .struct_page = type_check(cJSON_Number, cJSON_GetObjectItem(item, "struct-page"))->valueint, - .delegate = delegate ? type_check(cJSON_Number, delegate)->valueint : -1 }; } } @@ -710,7 +706,6 @@ static void load_rsave_call_stack(struct rsave *save) .return_address = return_address, .page_slot = save->call_frames[i].local_ptr, .struct_page = save->call_frames[i].struct_ptr, - .delegate = -1, }; // Calculate return address from the function address and offset // to make it robust to ain changes. diff --git a/src/vm.c b/src/vm.c index 8f3e7cd..2c6f7a8 100644 --- a/src/vm.c +++ b/src/vm.c @@ -317,7 +317,6 @@ static void scenario_call(int slot) .return_address = VM_RETURN, .page_slot = slot, .struct_page = -1, - .delegate = -1, }; call_stack_ptr = 1; instr_ptr = ain->functions[fno].address; @@ -342,7 +341,6 @@ static int _function_call(int fno, int return_address) .return_address = return_address, .page_slot = slot, .struct_page = -1, - .delegate = -1, }; // initialize local variables for (int i = f->nr_args; i < f->nr_vars; i++) { @@ -398,6 +396,8 @@ static void delegate_call(int dg_no, int return_address) if (ain->delegates[dg_no].return_type.data != AIN_VOID) { stack_pop(); } + // increment dg_index + stack[stack_ptr - 1].i++; int slot = _function_call(fun, instr_ptr + instruction_width(DG_CALL)); @@ -409,7 +409,6 @@ static void delegate_call(int dg_no, int return_address) } call_stack[call_stack_ptr-1].struct_page = obj; - call_stack[call_stack_ptr-1].delegate = dg_no; } else { // call finished: clean up stack and jump to return address union vm_value r; @@ -445,14 +444,6 @@ static void function_return(void) { heap_unref(call_stack[call_stack_ptr-1].page_slot); instr_ptr = call_stack[call_stack_ptr-1].return_address; - if (call_stack[call_stack_ptr-1].delegate >= 0) { - const int dg = call_stack[call_stack_ptr-1].delegate; - if (ain->delegates[dg].return_type.data != AIN_VOID) { - stack[stack_ptr-2].i++; - } else { - stack[stack_ptr-1].i++; - } - } call_stack_ptr--; }