Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog.d/10127-call-apply-dispatch-cache.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
### Performance

- **`Function.prototype.call`, `Function.prototype.apply`, and direct spread
calls no longer hash-probe the closure-body registry on every invocation.**
Their rest-parameter check now shares the existing four-entry dispatch memo,
retaining late-registration invalidation and all 0–16 argument, receiver,
bound-function, rest-array, and imported-class semantics. Refs #10085.
120 changes: 119 additions & 1 deletion crates/perry-runtime/src/closure/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ crate::perry_thread_local! {
/// The record for `func_ptr`, if module init registered anything about it.
#[inline(always)]
fn body_record(func_ptr: *const u8) -> Option<ClosureBodyRecord> {
#[cfg(test)]
BODY_RECORD_LOOKUPS.with(|lookups| lookups.set(lookups.get() + 1));
CLOSURE_BODY_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied())
}

Expand Down Expand Up @@ -307,6 +309,7 @@ crate::perry_thread_local! {
#[cfg(test)]
std::thread_local! {
static RESOLVE_STRATEGY_SLOW_CALLS: std::cell::Cell<u32> = const { std::cell::Cell::new(0) };
static BODY_RECORD_LOOKUPS: std::cell::Cell<u32> = const { std::cell::Cell::new(0) };
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -437,6 +440,27 @@ mod dispatch_recent_tests {
4.0
}

extern "C" fn add_two(_: *const ClosureHeader, left: f64, right: f64) -> f64 {
left + right
}

extern "C" fn identify_rest_array(_: *const ClosureHeader, value: f64) -> f64 {
let is_pointer = value.to_bits() >> 48 == crate::value::POINTER_TAG >> 48;
if is_pointer {
1.0
} else {
0.0
}
}

fn stack_closure(func_ptr: *const u8) -> ClosureHeader {
ClosureHeader {
func_ptr,
capture_count: 0,
type_tag: CLOSURE_MAGIC,
}
}

#[test]
fn four_alternating_bodies_stay_out_of_the_hash_lookup() {
let bodies = [
Expand Down Expand Up @@ -486,6 +510,90 @@ mod dispatch_recent_tests {
"late registration must evict a body from every recent-cache slot"
);
}

#[test]
fn repeated_array_calls_probe_the_body_registry_only_once() {
let body = add_two as *const u8;
let closure = stack_closure(body);
let args = [20.0, 22.0];
invalidate_dispatch_strategy(body);
RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.set(0));
BODY_RECORD_LOOKUPS.with(|lookups| lookups.set(0));

for _ in 0..32 {
assert_eq!(
unsafe {
crate::closure::js_closure_call_array(
&closure as *const ClosureHeader as i64,
args.as_ptr(),
args.len() as i64,
)
},
42.0
);
}

assert_eq!(
RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.get()),
1,
"a warm non-rest call-array path must not hash-probe the registry per call"
);
assert_eq!(
BODY_RECORD_LOOKUPS.with(|lookups| lookups.get()),
1,
"32 repeated calls must perform one total closure-body hash lookup"
);
}

#[test]
fn late_rest_registration_invalidates_the_call_array_memo() {
let body = identify_rest_array as *const u8;
let closure = stack_closure(body);
let direct_arg = [0.0];
invalidate_dispatch_strategy(body);
RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.set(0));
BODY_RECORD_LOOKUPS.with(|lookups| lookups.set(0));

assert_eq!(
unsafe {
crate::closure::js_closure_call_array(
&closure as *const ClosureHeader as i64,
direct_arg.as_ptr(),
direct_arg.len() as i64,
)
},
0.0,
"the first unregistered call must use direct dispatch"
);
assert_eq!(RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.get()), 1);
assert_eq!(BODY_RECORD_LOOKUPS.with(|lookups| lookups.get()), 1);

js_register_closure_rest(body, 0);
let rest_args = [1.0, 2.0, 3.0];
for _ in 0..2 {
assert_eq!(
unsafe {
crate::closure::js_closure_call_array(
&closure as *const ClosureHeader as i64,
rest_args.as_ptr(),
rest_args.len() as i64,
)
},
1.0,
"late registration must switch the cached body to rest dispatch"
);
}
assert_eq!(
RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.get()),
2,
"registration should cause one fresh registry probe, then remain cached"
);
assert_eq!(
BODY_RECORD_LOOKUPS.with(|lookups| lookups.get()),
2,
"the initial call and post-registration miss should be the only hash lookups"
);
}
}

#[no_mangle]
Expand Down Expand Up @@ -557,7 +665,17 @@ pub fn lookup_closure_rest(func_ptr: *const u8) -> Option<u32> {

#[inline(always)]
pub fn lookup_closure_rest_full(func_ptr: *const u8) -> Option<(u32, RestDispatchKind)> {
body_record(func_ptr)?.rest()
// Rest-ness is part of the same immutable-at-dispatch-time answer as
// bound routing and declared arity. In particular, do not bypass
// `DISPATCH_RECENT` here: `js_closure_call_array` probes this on every
// Function.prototype.call/apply invocation, and a direct `body_record`
// read would pay a TLS RefCell borrow plus a pointer hash lookup every
// time through that hot path. Registration invalidates the memo, so the
// #6475 call-before-registration case still observes a later rest entry.
match resolve_strategy(func_ptr).kind() {
DispatchKind::Rest(fixed_arity, kind) => Some((fixed_arity, kind)),
_ => None,
}
}

/// Register a closure body's declared param count (for closures WITHOUT a rest
Expand Down
5 changes: 5 additions & 0 deletions test-files/fixtures/issue_10085_dispatch/imported.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class ImportedClass {
static describe(value: number): string {
return `imported:${value}`;
}
}
131 changes: 131 additions & 0 deletions test-files/test_gap_10085_call_apply_dispatch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// #10085: Function.prototype.call/apply and direct spread calls all reach
// js_closure_call_array. Keep its cached non-rest path and its rest-bundling
// detour semantically identical across every supported dispatch arity.

import { ImportedClass } from "./fixtures/issue_10085_dispatch/imported.ts";

function check(condition: boolean, message: string): void {
if (!condition) throw new Error(message);
}

function restShape(first: number, ...rest: number[]): string {
return `${first}|${rest.length}|${rest.length === 0 ? -1 : rest[rest.length - 1]}`;
}

const restFunctions: any[] = [];
restFunctions.push(restShape);
const restValue: any = restFunctions[0];

// Rest closures must still bundle through .call, .apply, and direct spread at
// arities 1..=16. This pins #653's high-arity fix while the metadata probe is
// switched from a registry lookup to the dispatch-strategy memo.
for (let n = 1; n <= 16; n++) {
const args: number[] = [];
for (let i = 1; i <= n; i++) args.push(i);
const expected = `1|${n - 1}|${n === 1 ? -1 : n}`;
const direct = restValue(...args);
const viaCall = restValue.call({ ignored: true }, ...args);
const viaApply = restValue.apply({ ignored: true }, args);
check(direct === expected, `rest direct arity ${n}: ${direct}`);
check(viaCall === expected, `rest call arity ${n}: ${viaCall}`);
check(viaApply === expected, `rest apply arity ${n}: ${viaApply}`);
}

// A 16-parameter non-rest closure called with 0..=16 values exercises every
// js_closure_callN arm. Missing values must still be padded with undefined.
function arity16(
a0: any,
a1: any,
a2: any,
a3: any,
a4: any,
a5: any,
a6: any,
a7: any,
a8: any,
a9: any,
a10: any,
a11: any,
a12: any,
a13: any,
a14: any,
a15: any,
): number {
const values = [
a0,
a1,
a2,
a3,
a4,
a5,
a6,
a7,
a8,
a9,
a10,
a11,
a12,
a13,
a14,
a15,
];
let count = 0;
for (const value of values) if (value !== undefined) count++;
return count;
}

for (let n = 0; n <= 16; n++) {
const args: number[] = [];
for (let i = 0; i < n; i++) args.push(i + 1);
const direct = (arity16 as any)(...args);
const viaCall = (arity16 as any).call(null, ...args);
const viaApply = (arity16 as any).apply(null, args);
check(direct === n, `arity16 direct ${n}: ${direct}`);
check(viaCall === n, `arity16 call ${n}: ${viaCall}`);
check(viaApply === n, `arity16 apply ${n}: ${viaApply}`);
}

function ordinary(this: { bias: number }, left: number, right: number): number {
return this.bias + left + right;
}

const receiver = {
bias: 40,
ordinary,
makeArrow() {
return (value: number) => this.bias + value;
},
};
check(receiver.ordinary(1, 1) === 42, "ordinary direct receiver");
check(receiver.ordinary.call(receiver, 1, 1) === 42, "ordinary call receiver");
check(receiver.ordinary.apply(receiver, [1, 1]) === 42, "ordinary apply receiver");

const arrow = receiver.makeArrow();
check(arrow(2) === 42, "arrow direct receiver");
check(arrow.call({ bias: 100 }, 2) === 42, "arrow call keeps lexical receiver");
check(arrow.apply({ bias: 100 }, [2]) === 42, "arrow apply keeps lexical receiver");

const bound = ordinary.bind(receiver, 1);
check(bound(1) === 42, "bound direct receiver");
check(bound.call({ bias: 100 }, 1) === 42, "bound call keeps receiver");
check(bound.apply({ bias: 100 }, [1]) === 42, "bound apply keeps receiver");

function invokeImportedClass(value: any, input: number): string {
return `${typeof value}:${value.describe(input)}`;
}

// Imported class refs share the 0x7FFE tag with bridged int32 values. They
// must remain callable objects while call-array unboxes genuine integers.
const classArgs: any[] = [ImportedClass, 7];
const classExpected = "function:imported:7";
check((invokeImportedClass as any)(...classArgs) === classExpected, "class direct spread");
check(
(invokeImportedClass as any).call(null, ...classArgs) === classExpected,
"class call spread",
);
check(
(invokeImportedClass as any).apply(null, classArgs) === classExpected,
"class apply",
);

console.log("ok");
Loading