From dce9dfdae83ded71a945ba2eb1d6a26929011c22 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 00:33:37 +0000 Subject: [PATCH] Cap the direct-borrow scan at max builtin arity (#546) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-call direct-borrow heuristic scanned the ENTIRE argument list comparing item pointers against the builtin's result. For multi-arg calls that list is the small VM-packed temp, but a single list argument is the caller's own list — so `len of xs` cost O(len(xs)) per call and the idiomatic `loop while i < (len of xs):` was quadratic: ~520x slower than a hoisted length at 88k elements (surfaced by DeslanStudios rendering 44.1 kHz audio buffers). A borrowing builtin can only return a child it actually read, and every builtin reads its argument vector at small fixed indices (max arity in the registry is 7), so items past a small cap can never be the borrowed result. The scan is factored into vm_borrow_scan (shared by CASE(CALL), jit_helper_call, and OP_DISPATCH) and capped at 8. The borrow contract the cap must preserve is pinned in tests/test_call_semantics.eigs: coalesce/dict_set returning a direct child of a raw list VARIABLE (not just packed temps), a borrowed child of a dying temporary argument, and a >cap-length raw list with the borrow at index 0. 88k-iteration len-in-condition loop: 5,033ms -> 17.5ms. Full release suite: 2731/2731. Closes #546 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EKawAe121NNznQPXKiyxRf --- CHANGELOG.md | 19 ++++++++ src/vm.c | 84 ++++++++++++++++++---------------- tests/test_call_semantics.eigs | 36 +++++++++++++++ 3 files changed, 100 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5765630..eb6d2ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,25 @@ All notable changes to EigenScript are documented here. ## [Unreleased] +### Fixed +- **Builtin calls with a single list argument were O(len(list)) (#546).** + The direct-borrow heuristic that runs after every builtin call scanned + the entire argument list comparing item pointers against the result. + For multi-arg calls that list is the small VM-packed temp, but for a + single list argument it was the caller's own list — so `len of xs` + cost O(len(xs)) and the idiomatic `loop while i < (len of xs):` was + quadratic (~520x slower than a hoisted length at 88k elements; + surfaced by DeslanStudios audio-buffer rendering). The scan is now + factored into `vm_borrow_scan` (shared by CASE(CALL), + jit_helper_call, and OP_DISPATCH) and capped at the maximum builtin + arity: a borrowing builtin can only return a child it actually read, + and every builtin reads its argument vector at small fixed indices, + so items past the cap can never be the borrowed result. The borrow + contract (coalesce/dict_set returning a child of a raw list variable, + a dying-temporary argument, >cap-length raw lists) is pinned in + tests/test_call_semantics.eigs. 88k-iteration len-in-condition loop: + 5,033ms -> 17.5ms. + ### Added - **Runtime-error carets + token-precise LSP ranges (#407 residual).** Uncaught runtime errors now print the same one-line source excerpt + diff --git a/src/vm.c b/src/vm.c index cce6289..0c73126 100644 --- a/src/vm.c +++ b/src/vm.c @@ -1652,6 +1652,37 @@ void jit_helper_index_get(void) { vm_push(result); } +/* Direct-borrow heuristic, shared by the three builtin call sites + * (CASE(CALL), jit_helper_call, OP_DISPATCH): if a builtin's result is + * one of arg's top-level items (coalesce/append/dict_set return + * arg->data.list.items[0]), incref it so it survives the arg-decref + * that follows. Builtins returning a fresh allocation never match, so + * no spurious ref is added; nested borrows (get_at: items[0][idx]) + * must still incref locally — only direct children are scanned. + * + * #546: the scan is CAPPED. A borrowing builtin can only return a + * child it actually read, and every builtin reads its argument vector + * at small fixed indices (max arity in the registry is 7), so items + * past the cap can never be the borrowed result. Without the cap, a + * single-list-argument call (`len of xs`) scanned the user's entire + * list after every call — O(len) per call, turning the idiomatic + * `loop while i < (len of xs):` quadratic. Raise the cap if a builtin + * with a larger argument vector that RETURNS one of its args is ever + * added (the assert-ish comment in builtins.c's registry is the + * reminder). + */ +#define VM_BORROW_SCAN_CAP 8 +static inline void vm_borrow_scan(Value *arg, Value *result) { + if (arg && arg->type == VAL_LIST) { + int n = arg->data.list.count; + if (n > VM_BORROW_SCAN_CAP) n = VM_BORROW_SCAN_CAP; + Value **items = arg->data.list.items; + for (int i = 0; i < n; i++) { + if (items[i] == result) { val_incref(result); break; } + } + } +} + /* JIT Stage 4r/5f: out-of-line helper for OP_CALL. * * Mirrors the VAL_BUILTIN branch of CASE(CALL), plus (Stage 5f) a @@ -1859,15 +1890,9 @@ int jit_helper_call(EigsChunk *caller_chunk, int argc, int resume_off) { if (!result) { result = make_null(); } else if (!consumes_arg && result != arg) { - /* Direct-borrow heuristic — see CASE(CALL) for rationale and the - * !consumes_arg guard (free_val may have already freed arg). */ - if (arg && arg->type == VAL_LIST) { - int n = arg->data.list.count; - Value **items = arg->data.list.items; - for (int i = 0; i < n; i++) { - if (items[i] == result) { val_incref(result); break; } - } - } + /* Direct-borrow heuristic (capped, #546) — see vm_borrow_scan; + * !consumes_arg guard: free_val may have already freed arg. */ + vm_borrow_scan(arg, result); } if (!consumes_arg && result != arg) val_decref(arg); vm_push(result); @@ -3154,25 +3179,12 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { if (!result) { result = make_null(); } else if (!consumes_arg && result != arg) { - /* Direct-borrow heuristic: if result is one of arg's - * top-level items (e.g., coalesce/append/dict_set return - * arg->data.list.items[0]), incref it so it survives the - * arg-decref below. Builtins returning a fresh allocation - * (range, make_*) do not match this scan, so no spurious - * +1 ref is added. Nested borrows (get_at: items[0][idx]) - * must still incref locally — only direct children are - * scanned here. - * - * Guarded by !consumes_arg: a consuming builtin like - * free_val may have already freed arg, so reading arg - * here would be a use-after-free. */ - if (arg && arg->type == VAL_LIST) { - int n = arg->data.list.count; - Value **items = arg->data.list.items; - for (int i = 0; i < n; i++) { - if (items[i] == result) { val_incref(result); break; } - } - } + /* Direct-borrow heuristic (capped, #546) — see + * vm_borrow_scan for the full rationale. Guarded by + * !consumes_arg: a consuming builtin like free_val may + * have already freed arg, so reading arg here would be + * a use-after-free. */ + vm_borrow_scan(arg, result); } if (!consumes_arg && result != arg) val_decref(arg); /* If result == arg, the arg's refcount transfers to the result */ @@ -5030,17 +5042,11 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { if (!result) { result = make_null(); } else if (!consumes_arg && result != arg) { - /* Direct-borrow heuristic — see CASE(CALL) for rationale. - * Must run before val_decref(arg) so the items array is - * still valid, and skipped when the builtin already - * consumed arg (free_val), since arg would be freed. */ - if (arg && arg->type == VAL_LIST) { - int n = arg->data.list.count; - Value **items = arg->data.list.items; - for (int i = 0; i < n; i++) { - if (items[i] == result) { val_incref(result); break; } - } - } + /* Direct-borrow heuristic (capped, #546) — see + * vm_borrow_scan. Must run before val_decref(arg) so + * the items array is still valid, and skipped when the + * builtin already consumed arg (free_val). */ + vm_borrow_scan(arg, result); } if (!consumes_arg && result != arg) val_decref(arg); slot_decref(table_s); diff --git a/tests/test_call_semantics.eigs b/tests/test_call_semantics.eigs index ba26a48..73f8189 100644 --- a/tests/test_call_semantics.eigs +++ b/tests/test_call_semantics.eigs @@ -86,4 +86,40 @@ y is x y is y + 1 assert of [x == 5, "scalar reassignment does not affect original"] +# #546: the direct-borrow scan is capped at the max builtin arity, so a +# builtin call with a single LIST argument is O(1), not O(len). These pin +# the borrow contract the cap must preserve: a builtin returning a direct +# child of its argument vector keeps that child alive past the arg decref. + +# Borrowed child from a raw list VARIABLE argument (not a packed temp) +pair is [[1, 2, 3], "fallback"] +borrowed is coalesce of pair +assert of [borrowed == [1, 2, 3], "coalesce of raw list var borrows items[0]"] +pair is null +assert of [borrowed == [1, 2, 3], "borrowed child survives the source list dying"] + +# Borrowed child at index 1 (coalesce falls through empty first slot) +pair2 is ["", [4, 5]] +b2 is coalesce of pair2 +assert of [b2 == [4, 5], "coalesce borrows items[1] on empty first"] + +# Borrowed child of a DYING TEMPORARY argument (refcount hits 0 at the +# call site's arg decref -- the exact case the scan protects) +b3 is coalesce of (concat of [[[9, 8]], [["d"]]]) +assert of [b3 == [9, 8], "borrow from freshly built temp argument survives"] + +# dict_set returns its container (items[0]) borrowed from a raw arg list +d546 is {"k": 1} +args546 is [d546, "k2", 2] +returned is dict_set of args546 +assert of [returned.k2 == 2, "dict_set of raw arg-list var returns the dict"] + +# Raw list longer than the scan cap: the borrowed child sits at index 0, +# well under the cap, and must still be protected +big is [[7, 7], 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] +b4 is coalesce of big +assert of [b4 == [7, 7], "borrow from >cap-length raw list still protected"] +big is null +assert of [b4 == [7, 7], "child of dropped >cap-length list survives"] + print of "All tests passed"