From 94f1645be1337e3fe5a0031cab7c1ac855d020bd Mon Sep 17 00:00:00 2001 From: John Alanbrook Date: Tue, 3 Feb 2026 19:04:38 -0600 Subject: [PATCH] new closure model --- source/quickjs.c | 210 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 151 insertions(+), 59 deletions(-) diff --git a/source/quickjs.c b/source/quickjs.c index 728dc2e1..b71405a7 100644 --- a/source/quickjs.c +++ b/source/quickjs.c @@ -451,13 +451,13 @@ struct VMFrame { int stack_size_allocated; /* total size allocated for this frame */ }; -/* GC-managed frame for closures - lives on GC heap with OBJ_FRAME type - Used by the link-time relocation model where closures reference - outer frames via (depth, slot) addressing */ +/* Frame for closures - used by link-time relocation model where closures + reference outer frames via (depth, slot) addressing. + Stores function as JSValue to survive GC movements. */ typedef struct JSFrame { objhdr_t header; /* OBJ_FRAME, cap56 = slot count */ - struct JSFunction *function; - struct JSFrame *caller; + JSValue function; /* JSValue for GC safety (use JS_VALUE_GET_FUNCTION) */ + JSValue caller; /* JSValue for GC safety (unused currently) */ uint32_t return_pc; JSValue slots[]; /* args, captured, locals, temps */ } JSFrame; @@ -2527,17 +2527,9 @@ static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *from_base, uint8 case OBJ_FRAME: { /* JSFrame - scan function, caller, and slots */ JSFrame *frame = (JSFrame *)ptr; - /* function and caller are pointers - need to copy them as values */ - if (frame->function) { - JSValue fn_val = JS_MKPTR (frame->function); - fn_val = gc_copy_value (ctx, fn_val, from_base, from_end, to_base, to_free, to_end); - frame->function = (JSFunction *)JS_VALUE_GET_PTR (fn_val); - } - if (frame->caller) { - JSValue caller_val = JS_MKPTR (frame->caller); - caller_val = gc_copy_value (ctx, caller_val, from_base, from_end, to_base, to_free, to_end); - frame->caller = (JSFrame *)JS_VALUE_GET_PTR (caller_val); - } + /* function and caller are now JSValues - copy them directly */ + frame->function = gc_copy_value (ctx, frame->function, from_base, from_end, to_base, to_free, to_end); + frame->caller = gc_copy_value (ctx, frame->caller, from_base, from_end, to_base, to_free, to_end); /* Scan all slots */ uint64_t slot_count = objhdr_cap56 (frame->header); for (uint64_t i = 0; i < slot_count; i++) { @@ -2707,8 +2699,14 @@ static int ctx_gc (JSContext *ctx, int allow_grow) { sf->var_buf[i] = gc_copy_value(ctx, sf->var_buf[i], from_base, from_end, to_base, &to_free, to_end); } } - /* Scan js_frame if present */ + /* Scan js_frame if present - it's on regular heap but contains JSValues pointing to GC heap */ sf->js_frame = gc_copy_value(ctx, sf->js_frame, from_base, from_end, to_base, &to_free, to_end); + if (!JS_IsNull(sf->js_frame)) { + JSFrame *jf = JS_VALUE_GET_FRAME(sf->js_frame); + jf->function = gc_copy_value(ctx, jf->function, from_base, from_end, to_base, &to_free, to_end); + jf->caller = gc_copy_value(ctx, jf->caller, from_base, from_end, to_base, &to_free, to_end); + /* Note: jf->slots are same as arg_buf/var_buf, already scanned above */ + } /* Scan operand stack */ if (sf->stack_buf && sf->p_sp) { JSValue *sp = *sf->p_sp; @@ -4035,8 +4033,8 @@ static JSFrame *js_new_frame (JSContext *ctx, JSFunction *func, if (!f) return NULL; f->header = objhdr_make (slot_count, OBJ_FRAME, false, false, false, false); - f->function = func; - f->caller = caller; + f->function = JS_MKPTR(func); + f->caller = caller ? JS_MKPTR(caller) : JS_NULL; f->return_pc = ret_pc; /* Initialize all slots to JS_NULL */ @@ -4054,7 +4052,7 @@ static inline JSValue *get_upvalue_ptr (JSValue frame_val, int depth, int slot) if (JS_IsNull(frame_val)) return NULL; JSFrame *frame = JS_VALUE_GET_FRAME(frame_val); while (depth > 0) { - JSFunction *fn = frame->function; + JSFunction *fn = JS_VALUE_GET_FUNCTION(frame->function); frame_val = fn->u.func.outer_frame; if (JS_IsNull(frame_val)) return NULL; frame = JS_VALUE_GET_FRAME(frame_val); @@ -6831,8 +6829,10 @@ static JSValue js_closure2 (JSContext *ctx, JSValue func_obj, JSFunctionBytecode f = JS_VALUE_GET_FUNCTION (func_obj); f->u.func.function_bytecode = b; f->u.func.var_refs = NULL; - f->u.func.outer_frame = JS_NULL; /* Not used in var_refs model */ + f->u.func.outer_frame = JS_NULL; /* Will be set after GC-safe operations */ + /* Also populate var_refs for backward compatibility with existing bytecode + that uses OP_get_var_ref/OP_put_var_ref opcodes. */ if (b->closure_var_count) { /* Use pjs_mallocz for var_refs - regular heap, not GC-managed */ var_refs = pjs_mallocz(sizeof(var_refs[0]) * b->closure_var_count); @@ -6853,14 +6853,18 @@ static JSValue js_closure2 (JSContext *ctx, JSValue func_obj, JSFunctionBytecode var_refs = f->u.func.var_refs; } else { var_ref = cur_var_refs[cv->var_idx]; - /* No ref_count increment needed with copying GC */ } var_refs[i] = var_ref; } } + + /* Set outer_frame AFTER all GC-triggering operations. + sf->js_frame may have been updated by GC, so we read it here. */ + f = JS_VALUE_GET_FUNCTION (func_obj); + f->u.func.outer_frame = sf ? sf->js_frame : JS_NULL; + return func_obj; fail: - /* bfunc is freed when func_obj is freed */ return JS_EXCEPTION; } @@ -7151,31 +7155,47 @@ static JSValue JS_CallInternal (JSContext *caller_ctx, JSValue func_obj, JSValue return JS_ThrowStackOverflow (caller_ctx); sf->js_mode = b->js_mode; - arg_buf = argv; sf->arg_count = argc; sf->cur_func = (JSValue)func_obj; init_list_head (&sf->var_ref_list); var_refs = f->u.func.var_refs; - sf->js_frame = JS_NULL; /* Will be created lazily if needed for closures */ - local_buf = alloca(alloca_size); - if (unlikely(arg_allocated_size)) { + /* Allocate JSFrame for args + vars on regular heap (stable pointers). + This enables the outer_frame closure model. */ + { + int frame_slot_count = b->arg_count + b->var_count; + size_t frame_size = sizeof(JSFrame) + frame_slot_count * sizeof(JSValue); + JSFrame *jf = pjs_mallocz(frame_size); + if (!jf) + return JS_ThrowOutOfMemory(caller_ctx); + jf->header = objhdr_make(frame_slot_count, OBJ_FRAME, false, false, false, false); + jf->function = JS_MKPTR(f); + jf->caller = JS_NULL; + jf->return_pc = 0; + sf->js_frame = JS_MKPTR(jf); + + /* Point arg_buf and var_buf into JSFrame slots */ + arg_buf = jf->slots; + var_buf = jf->slots + b->arg_count; + sf->arg_buf = arg_buf; + sf->var_buf = var_buf; + + /* Copy arguments into frame */ int n = min_int(argc, b->arg_count); - arg_buf = local_buf; for (i = 0; i < n; i++) arg_buf[i] = argv[i]; for (; i < b->arg_count; i++) arg_buf[i] = JS_NULL; - sf->arg_count = b->arg_count; + if (argc < b->arg_count) + sf->arg_count = b->arg_count; + + /* Initialize local variables */ + for (i = 0; i < b->var_count; i++) + var_buf[i] = JS_NULL; } - var_buf = local_buf + arg_allocated_size; - sf->var_buf = var_buf; - sf->arg_buf = arg_buf; - for (i = 0; i < b->var_count; i++) - var_buf[i] = JS_NULL; - - stack_buf = var_buf + b->var_count; + /* Stack is still on C stack (transient) */ + stack_buf = alloca(sizeof(JSValue) * b->stack_size); sp = stack_buf; pc = b->byte_code_buf; sf->stack_buf = stack_buf; @@ -15009,6 +15029,33 @@ static int get_closure_var (JSContext *ctx, JSFunctionDef *s, JSFunctionDef *fd, return get_closure_var2 (ctx, s, fd, TRUE, is_arg, var_idx, var_name, is_const, is_lexical, var_kind); } +/* Compute closure depth and slot for OP_get_up/OP_set_up opcodes. + Follows the closure_var chain to find the actual variable location. + Returns depth (1 = immediate parent) and slot (index in JSFrame.slots). + Returns -1 on error. */ +static int compute_closure_depth_slot(JSFunctionDef *s, int closure_idx, int *out_slot) { + int depth = 1; + JSClosureVar *cv = &s->closure_var[closure_idx]; + JSFunctionDef *fd = s->parent; + + /* Follow chain until we find the actual local variable */ + while (!cv->is_local) { + if (!fd || !fd->parent) return -1; /* Invalid chain */ + cv = &fd->closure_var[cv->var_idx]; + fd = fd->parent; + depth++; + } + + /* Now cv->var_idx is the index in fd's variables/arguments. + Compute slot in JSFrame.slots layout: [args..., vars...] */ + if (cv->is_arg) { + *out_slot = cv->var_idx; + } else { + *out_slot = fd->arg_count + cv->var_idx; + } + return depth; +} + static BOOL can_opt_put_ref_value (const uint8_t *bc_buf, int pos) { int opcode = bc_buf[pos]; return (bc_buf[pos + 1] == OP_put_ref_value @@ -15442,12 +15489,41 @@ static int resolve_scope_var (JSContext *ctx, JSFunctionDef *s, JSValue var_name dbuf_put_u32 (bc, cpool_idx); } else if (label_done == -1 && can_opt_put_ref_value (bc_buf, ls->pos)) { - int get_op; - if (s->closure_var[idx].is_lexical) - get_op = OP_get_var_ref_check; - else - get_op = OP_get_var_ref; - pos_next = optimize_scope_make_ref (ctx, s, bc, bc_buf, ls, pos_next, get_op, idx); + /* Check if we can use new OP_set_up model for closure variables */ + int slot; + int depth = compute_closure_depth_slot(s, idx, &slot); + if (depth > 0 && depth <= 255 && slot <= 65535) { + /* Use OP_get_up/OP_set_up for the make_ref optimization */ + int label_pos = ls->pos; + int pos = label_pos - 5; + int end_pos = label_pos + 2; + if (bc_buf[pos_next] == OP_get_ref_value) { + /* Get part - emit to bc output buffer */ + dbuf_putc (bc, OP_get_up); + dbuf_putc (bc, (uint8_t)depth); + dbuf_put_u16 (bc, (uint16_t)slot); + pos_next++; + } + /* Put part - patch in-place in bc_buf */ + if (bc_buf[label_pos] == OP_insert3) { + bc_buf[pos++] = OP_dup; + } + bc_buf[pos] = OP_set_up; + bc_buf[pos + 1] = (uint8_t)depth; + put_u16(bc_buf + pos + 2, (uint16_t)slot); + pos += 4; + /* Pad with nops */ + while (pos < end_pos) + bc_buf[pos++] = OP_nop; + } else { + /* Fall back to old var_ref model */ + int get_op; + if (s->closure_var[idx].is_lexical) + get_op = OP_get_var_ref_check; + else + get_op = OP_get_var_ref; + pos_next = optimize_scope_make_ref (ctx, s, bc, bc_buf, ls, pos_next, get_op, idx); + } } else { /* Create a dummy object with a named slot that is a reference to the closure variable */ @@ -15467,28 +15543,44 @@ static int resolve_scope_var (JSContext *ctx, JSFunctionDef *s, JSValue var_name case OP_scope_put_var: case OP_scope_put_var_init: is_put = (op == OP_scope_put_var || op == OP_scope_put_var_init); - if (is_put) { - if (s->closure_var[idx].is_lexical) { - if (op == OP_scope_put_var_init) { - /* 'this' can only be initialized once */ - if (js_key_equal (var_name, JS_KEY_this)) - dbuf_putc (bc, OP_put_var_ref_check_init); - else - dbuf_putc (bc, OP_put_var_ref); + { + /* Use OP_get_up/OP_set_up with (depth, slot) encoding */ + int slot; + int depth = compute_closure_depth_slot(s, idx, &slot); + if (depth > 0 && depth <= 255 && slot <= 65535) { + /* Can use the new outer_frame model */ + if (is_put) { + dbuf_putc(bc, OP_set_up); } else { - dbuf_putc (bc, OP_put_var_ref_check); + dbuf_putc(bc, OP_get_up); } + dbuf_putc(bc, (uint8_t)depth); + dbuf_put_u16(bc, (uint16_t)slot); } else { - dbuf_putc (bc, OP_put_var_ref); - } - } else { - if (s->closure_var[idx].is_lexical) { - dbuf_putc (bc, OP_get_var_ref_check); - } else { - dbuf_putc (bc, OP_get_var_ref); + /* Fall back to old var_ref model */ + if (is_put) { + if (s->closure_var[idx].is_lexical) { + if (op == OP_scope_put_var_init) { + if (js_key_equal (var_name, JS_KEY_this)) + dbuf_putc (bc, OP_put_var_ref_check_init); + else + dbuf_putc (bc, OP_put_var_ref); + } else { + dbuf_putc (bc, OP_put_var_ref_check); + } + } else { + dbuf_putc (bc, OP_put_var_ref); + } + } else { + if (s->closure_var[idx].is_lexical) { + dbuf_putc (bc, OP_get_var_ref_check); + } else { + dbuf_putc (bc, OP_get_var_ref); + } + } + dbuf_put_u16 (bc, idx); } } - dbuf_put_u16 (bc, idx); break; case OP_scope_delete_var: dbuf_putc (bc, OP_push_false);