From 05f8ac525fbf8fbed492a1783c948a0d4e815c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 20 Sep 2026 15:41:00 +0200 Subject: [PATCH 1/2] perf(runtime): skip the arg-combine Vec allocation for no-partial-args bound functions dispatch_bound_function unconditionally built a Vec, copied the call-time args into it, and freed it before every Function.prototype.bind call -- even for the overwhelmingly common .bind(thisArg) method-reference shape with no partial-applied arguments, where js_function_bind already leaves the bound-args capture null. That shape is exactly what direct.rs's per-loop dispatch hoisting excludes (BoundFunction is deliberately not resolved by resolve_direct_func_ptr), so a bound method used as an arr.forEach callback paid this allocation on every element. --- .../callback-bound-dispatch-skip-vec-alloc.md | 76 ++++++ .../src/closure/dispatch/bound.rs | 37 ++- .../test_gap_callback_dispatch_shapes.ts | 227 ++++++++++++++++++ 3 files changed, 331 insertions(+), 9 deletions(-) create mode 100644 changelog.d/callback-bound-dispatch-skip-vec-alloc.md create mode 100644 test-files/test_gap_callback_dispatch_shapes.ts diff --git a/changelog.d/callback-bound-dispatch-skip-vec-alloc.md b/changelog.d/callback-bound-dispatch-skip-vec-alloc.md new file mode 100644 index 0000000000..9fc376b138 --- /dev/null +++ b/changelog.d/callback-bound-dispatch-skip-vec-alloc.md @@ -0,0 +1,76 @@ +Removed a per-call heap allocation from `dispatch_bound_function` +(`crates/perry-runtime/src/closure/dispatch/bound.rs`), the runtime entry +every `js_closure_call` routes a `Function.prototype.bind` result through. + +The function unconditionally built a `Vec` (`Vec::with_capacity(args.len() ++ 4)`, a push loop over any bound args, then `extend_from_slice(args)`) before +calling the bound target, even for the overwhelmingly common `.bind(thisArg)` +shape with **no** partial-applied arguments — a plain method reference +(`arr.forEach(obj.method.bind(obj))`). `js_function_bind` already leaves +capture slot 2 (`bound_args_ptr`) null whenever no extra args were bound, so +that case now skips the allocate/copy/free cycle entirely and passes the +caller's own `args` slice straight through to `js_native_call_value`. The +actual partial-application shape (`.bind(obj, extra)`) is untouched other than +a tighter `Vec` capacity hint (`n + args.len()` instead of the old +`args.len() + 4`). + +This targets the "method reference passed as callback" shape specifically: +unlike a plain closure held in a local, a `.bind()`-created function is +excluded from `direct.rs`'s per-loop dispatch hoisting (`BoundFunction` is +deliberately not resolved by `resolve_direct_func_ptr` — +`crates/perry-runtime/src/closure/dispatch/direct.rs:70`, inside the +`func_ptr.is_null() || func_ptr == BOUND_METHOD_FUNC_PTR || func_ptr == +BOUND_FUNCTION_FUNC_PTR` early-return at lines 68-73), so `arr.forEach(fn)` +over a bound method re-runs this allocation on every element. + +**Aliasing / rooting**: traced every one of `dispatch_bound_function`'s 9 +call sites (exhaustive repo grep, not just perry-runtime). Every one hands it +either a `[f64; N]` literal array of by-value `f64` parameters living on the +Rust native stack (`closure/dispatch/calln.rs`'s per-arity entry points, +lines 38/83/151/181/215/255/299, and `dispatch_registered_call`'s 8 callers +at lines 382/415/449/496/549/606/665/727, each building `let args = [arg0, +arg1, ...]`) or a freshly Rust-`Vec`-allocated copy (`value_call.rs`'s +`full`, built by a `push` loop over `a(i)` before dispatch, for the >16-arg / +dynamic-call path). Neither shape is ever GC-managed memory — the collector +only owns memory it allocates itself (the `js_*_alloc` family into the +arena/nursery/old-gen) and has no knowledge of the Rust stack or +`Vec`/`Box` heap. So `args`'s *backing storage* can't be moved or freed by a +collection inside this call on any path, before or after this change, and +handing `js_native_call_value` the caller's slice directly instead of a +byte-copy of it is safe. + +That is a narrower claim than "GC-safe" and worth stating precisely: a stack +array of NaN-boxed values is not a GC root, and neither was the old `Vec` +copy. If a collection *moves* an object referenced by one of the argument +*values* during `js_native_call_value` (e.g. inside `rebind_explicit_this`, +which can allocate), neither the old buffer nor the new one gets its bits +rewritten — copying bytes into a fresh `Vec` is not registering a root, so it +never protected against that. This change neither introduces nor fixes that +pre-existing exposure; it is identical before and after (and the conservative +native-stack scan that could theoretically cover it is diagnostic-only by +default, `Auto` → `SkipDisabled`). + +Measured with the repo's differential probe technique (marginal cost isolated +from loop overhead, instructions retired plus wall/CPU time under a +measurement mutex, best-of-N): the `.bind(obj)`-with-no-extra-args shape drops +from ~2795 to ~2676 instructions per call (~4.3%), reproduced across two +independent runs (N=50000 and N=250000, 7 and 15 reps). Wall-clock time on the +measurement host was noisy under heavy unrelated contention (system load +averaged 60-117 on a 10-core box) and did not resolve a stable direction; the +more contention-robust process CPU-time metric (user+sys) showed no +regression (flat-to-favorable across both runs). The partially-applied-bind +shape (`.bind(obj, extra)`), the already-optimized loop-local-closure shape, +and a bare-loop control all measured ~0 delta, confirming the change is +isolated to its target shape. + +Added `test-files/test_gap_callback_dispatch_shapes.ts`, byte-identical +against node 26.5.1, covering: direct arrow inline to a builtin array method, +a closure held in a local (once and in a loop), a callback threaded through a +second function frame, a callback parameter called directly in a loop, a +`.bind()` method reference with and without partial args, `this` binding +across arrow/ordinary/bound call shapes (including the receiverless-call +`this === undefined` case), `arguments`/extra/missing-argument/`.length` +handling, a callback that throws through one and two frames, recursion +through a plain and a bound callback reference, closures capturing a loop +variable (`let` and the classic `var` + IIFE idiom), and a bound method used +as a hot per-element `forEach` callback. diff --git a/crates/perry-runtime/src/closure/dispatch/bound.rs b/crates/perry-runtime/src/closure/dispatch/bound.rs index c73f17a5bb..60b7c356ff 100644 --- a/crates/perry-runtime/src/closure/dispatch/bound.rs +++ b/crates/perry-runtime/src/closure/dispatch/bound.rs @@ -306,14 +306,38 @@ pub unsafe fn dispatch_bound_function(closure: *const ClosureHeader, args: &[f64 // Collect the partial-applied (bound) leading args, then append the // call-time args. `g = f.bind(obj, 2); g(3)` calls `f` with `(2, 3)`. - let mut combined: Vec = Vec::with_capacity(args.len() + 4); - if !bound_args_ptr.is_null() { + // + // The overwhelmingly common shape is `.bind(thisArg)` with NO partial + // args at all — a plain method reference (`arr.forEach(obj.method.bind( + // obj))`), the callback shape `direct.rs` cannot hoist out of a loop + // (BoundFunction is deliberately excluded from `resolve_direct_func_ptr` + // — see that module's doc), so this function runs on every element. + // `js_function_bind` leaves capture slot 2 (`bound_args_ptr`) null + // whenever `bound_arg_count == 0`, so that's exactly the free-to-detect + // case: skip the allocate-copy-free `Vec` and hand `js_native_call_value` + // the caller's own `args` slice directly. Only the actual + // partial-application shape (`.bind(obj, extra)`) still needs a combined + // buffer. + let mut combined: Vec; + let (call_ptr, call_len): (*const f64, usize) = if bound_args_ptr.is_null() { + if args.is_empty() { + (std::ptr::null(), 0) + } else { + (args.as_ptr(), args.len()) + } + } else { let n = crate::array::js_array_length(bound_args_ptr) as usize; + combined = Vec::with_capacity(n + args.len()); for i in 0..n { combined.push(crate::array::js_array_get_f64(bound_args_ptr, i as u32)); } - } - combined.extend_from_slice(args); + combined.extend_from_slice(args); + if combined.is_empty() { + (std::ptr::null(), 0) + } else { + (combined.as_ptr(), combined.len()) + } + }; // A bound concise/object-literal method reads `this` from its baked capture // slot, not IMPLICIT_THIS — rebind it to the bound receiver so the bound @@ -321,11 +345,6 @@ pub unsafe fn dispatch_bound_function(closure: *const ClosureHeader, args: &[f64 let target = rebind_explicit_this(target, bound_this); let this_scope = crate::gc::RuntimeHandleScope::new(); // #9445 let prev_this = this_scope.root_nanbox_f64(crate::object::js_implicit_this_set(bound_this)); - let (call_ptr, call_len) = if combined.is_empty() { - (std::ptr::null::(), 0usize) - } else { - (combined.as_ptr(), combined.len()) - }; let result = js_native_call_value(target, call_ptr, call_len); crate::object::js_implicit_this_set(prev_this.get_nanbox_f64()); result diff --git a/test-files/test_gap_callback_dispatch_shapes.ts b/test-files/test_gap_callback_dispatch_shapes.ts new file mode 100644 index 0000000000..07c35589e4 --- /dev/null +++ b/test-files/test_gap_callback_dispatch_shapes.ts @@ -0,0 +1,227 @@ +// Callback-dispatch shapes audited for perf/callback-dispatch (calling a +// JSValue that holds a closure through the various paths codegen/runtime +// recognize: direct arrow-inline array-method callback, a closure held in a +// local, a closure threaded through a second function frame, a +// `Function.prototype.bind` method reference with and without partial args, +// and a hand-written tight loop over a callback parameter). +// +// This exercises exactly the shape `dispatch_bound_function` +// (crates/perry-runtime/src/closure/dispatch/bound.rs) treats specially: +// `.bind(thisArg)` with NO extra bound args skips the old per-call `Vec` +// combine-copy and passes the call-time args straight through. What is easy +// to break doing that: `this` binding (arrow vs ordinary function vs bound), +// `arguments`, extra/missing arguments and `.length`, a callback that +// throws (stack must stay correct), recursion through a callback reference, +// and a callback closing over a loop variable. + +function log(label: string, value: unknown): void { + console.log(label + ": " + JSON.stringify(value)); +} + +// --- shape: direct arrow inline to a builtin array-iteration method ------- +{ + const arr = [1, 2, 3, 4, 5]; + let s = 0; + arr.forEach((x) => { s += x; }); + log("forEach_arrow_inline", s); +} + +// --- shape: callback held in a local, called once ------------------------- +{ + const cb = (x: number) => x + 1; + log("local_once", cb(5)); +} + +// --- shape: callback held in a local, called in a tight loop -------------- +{ + const cb = (x: number) => x * 2; + let s = 0; + for (let i = 0; i < 20; i++) s += cb(i); + log("local_loop", s); +} + +// --- shape: callback threaded through a second function frame ------------- +function invokeOnce(cb: (x: number) => number, x: number): number { + return cb(x); +} +function twoFrames(cb: (x: number) => number, n: number): number { + let s = 0; + for (let i = 0; i < n; i++) s += invokeOnce(cb, i); + return s; +} +log("two_frames", twoFrames((x) => x + 3, 15)); + +// --- shape: callback parameter called directly in a tight loop ------------ +function directLoop(cb: (i: number) => number, n: number): number { + let s = 0; + for (let i = 0; i < n; i++) s += cb(i); + return s; +} +log("direct_loop_param", directLoop((x) => x - 1, 12)); + +// --- shape: method reference via .bind(), NO extra bound args ------------- +class Adder { + base: number; + constructor(base: number) { this.base = base; } + add(x: number): number { return this.base + x; } + // reads `this` explicitly, so a wrong receiver after bind() is observable. + describe(): string { return "Adder(" + this.base + ")"; } +} +{ + const a = new Adder(10); + const fn = a.add.bind(a); + const arr = [1, 2, 3, 4, 5]; + let s = 0; + arr.forEach((x) => { s += fn(x); }); + log("bound_no_extra_args_forEach", s); + log("bound_no_extra_args_once", fn(7)); + + const describeFn = a.describe.bind(a); + log("bound_this_reads_correctly", describeFn()); +} + +// --- shape: method reference via .bind(), WITH partial-applied args ------- +{ + const a = new Adder(100); + const fn2 = a.add.bind(a, 5); // bound arg `5` prepended... but `add` takes + // only ONE param, so the extra bound arg is simply ignored per spec (bound + // length caps at 0, extra bound args beyond declared arity are dropped by + // the underlying call, not by bind itself -- Node and Perry must agree). + log("bound_with_extra_args", fn2(1)); + + function sum3(a: number, b: number, c: number): number { return a + b + c; } + const boundSum = sum3.bind(null, 1, 2); + log("bound_plain_fn_partial", boundSum(3)); +} + +// --- shape: this binding across arrow / ordinary function / bound --------- +{ + const obj = { + v: 42, + arrowGet(this: any) { return (() => this.v)(); }, + ordinary(this: any) { return this.v; }, + }; + function grabThis(this: any): unknown { return this; } + const boundGrab = grabThis.bind(obj); + log("this_arrow_capture", obj.arrowGet()); + log("this_ordinary_direct", obj.ordinary()); + log("this_bound_grab", (boundGrab() as { v: number }).v); + + // A receiverless call of an ordinary function callback observes + // `this === undefined` (strict-mode-like OrdinaryCallBindThis) even when + // an enclosing method call left an IMPLICIT_THIS around. + function receiverless(this: unknown): string { + return this === undefined ? "undefined" : "leaked:" + JSON.stringify(this); + } + function callIt(cb: () => string): string { return cb(); } + const holder = { + m(): string { return callIt(receiverless); }, + }; + log("this_receiverless_no_leak", holder.m()); +} + +// --- shape: arguments object + extra/missing args + .length --------------- +{ + function variadic(): string { + // eslint-disable-next-line prefer-rest-params + const args = arguments as unknown as ArgumentsLike; + const parts: string[] = []; + for (let i = 0; i < args.length; i++) parts.push(String(args[i])); + return parts.join(","); + } + interface ArgumentsLike { length: number; [i: number]: unknown; } + function callWithN(cb: (...a: unknown[]) => string, ...a: unknown[]): string { + return cb(...a); + } + log("arguments_object_extra", callWithN(variadic as any, 1, 2, 3, 4)); + log("arguments_object_missing", callWithN(variadic as any)); + log("function_length_declared", ((a: number, b: number, c: number) => a + b + c).length); + + function needsThree(a: number, b: number, c: number): string { + return `${a},${b},${c}`; + } + log("missing_args_become_undefined", (needsThree as any)(1)); + log("extra_args_ignored", (needsThree as any)(1, 2, 3, 4, 5)); +} + +// --- shape: callback that throws, stack must stay correct ----------------- +{ + function boom(): never { throw new Error("boom"); } + function callThrow(cb: () => never): string { + try { + cb(); + return "no-throw"; + } catch (e) { + return "caught:" + (e as Error).message; + } + } + log("callback_throws_caught", callThrow(boom)); + + const boundBoom = boom.bind(null); + let threwFromBound = "no"; + try { + boundBoom(); + } catch (e) { + threwFromBound = "caught:" + (e as Error).message; + } + log("bound_callback_throws", threwFromBound); + + function outer(): string { + function inner(cb: () => never): string { + try { + cb(); + return "unreachable"; + } catch (e) { + return (e as Error).message; + } + } + return inner(boom); + } + log("callback_throws_through_two_frames", outer()); +} + +// --- shape: recursion through a callback reference ------------------------- +{ + function makeCountdown(): (n: number) => number { + const step = (n: number): number => (n <= 0 ? 0 : n + step(n - 1)); + return step; + } + const countdown = makeCountdown(); + log("recursive_callback", countdown(10)); + + // Recursion through a bound reference to itself. + let fact: (n: number) => number; + fact = (n: number): number => (n <= 1 ? 1 : n * fact(n - 1)); + const boundFact = fact.bind(null); + log("recursive_bound_callback", boundFact(6)); +} + +// --- shape: closure captured in a loop variable ---------------------------- +{ + const callbacks: Array<() => number> = []; + for (let i = 0; i < 5; i++) { + callbacks.push(() => i * i); + } + log("loop_var_capture_let", callbacks.map((f) => f())); + + const callbacksVar: Array<() => number> = []; + for (var j = 0; j < 5; j++) { + // eslint-disable-next-line no-loop-func + callbacksVar.push((function (captured) { return () => captured; })(j)); + } + log("loop_var_capture_var_iife", callbacksVar.map((f) => f())); +} + +// --- shape: bound method used as a hot forEach callback across many calls - +{ + class Acc { + total: number = 0; + add(x: number): number { this.total += x; return this.total; } + } + const acc = new Acc(); + const boundAdd = acc.add.bind(acc); + const many: number[] = []; + for (let i = 0; i < 200; i++) many.push(i); + many.forEach((x) => boundAdd(x)); + log("bound_hot_loop_total", acc.total); +} From 301aca4fda9d331f5ec2c1bacafcaca5df34ac93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 20 Sep 2026 15:48:15 +0200 Subject: [PATCH 2/2] changelog: key fragment to #10818 --- ...c-alloc.md => 10818-callback-bound-dispatch-skip-vec-alloc.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{callback-bound-dispatch-skip-vec-alloc.md => 10818-callback-bound-dispatch-skip-vec-alloc.md} (100%) diff --git a/changelog.d/callback-bound-dispatch-skip-vec-alloc.md b/changelog.d/10818-callback-bound-dispatch-skip-vec-alloc.md similarity index 100% rename from changelog.d/callback-bound-dispatch-skip-vec-alloc.md rename to changelog.d/10818-callback-bound-dispatch-skip-vec-alloc.md