diff --git a/TYPE_LOWERING.md b/TYPE_LOWERING.md index 23840a7105..304163d494 100644 --- a/TYPE_LOWERING.md +++ b/TYPE_LOWERING.md @@ -654,7 +654,7 @@ Every allocation is preceded by an 8-byte `GcHeader`: `obj_type` (u8), `gc_flags ### Closures -`ClosureHeader`: `func_ptr` (usize), `capture_count` (u32, high bit = `CAPTURES_THIS_FLAG`), `type_tag` (`CLOSURE_MAGIC 0x434C_4F53`), variadic `captures[]` (u64 slots). Mutable captures are heap-boxed. Side-tables: `CLOSURE_REST_REGISTRY`, `CLOSURE_ARITY_REGISTRY`, `DISPATCH_CACHE`. Public closure dispatch still uses the generic closure pointer plus boxed `double` argument/return model. Eligible typed closure clones now use an internal `i64 this_closure, typed args...` ABI so immutable f64/i1 captures can be loaded as native values before the body is lowered. +`ClosureHeader`: `func_ptr` (usize), `capture_count` (u32, high bit = `CAPTURES_THIS_FLAG`), `type_tag` (`CLOSURE_MAGIC 0x434C_4F53`), variadic `captures[]` (u64 slots). Mutable captures are heap-boxed. Side-table: `CLOSURE_BODY_REGISTRY` (one packed record per body: rest/arity/length/flags, #9707) plus the `DISPATCH_RECENT` four-entry cache. Public closure dispatch still uses the generic closure pointer plus boxed `double` argument/return model. Eligible typed closure clones now use an internal `i64 this_closure, typed args...` ABI so immutable f64/i1 captures can be loaded as native values before the body is lowered. ### Async/Await diff --git a/changelog.d/9720-initializer-self-binding.md b/changelog.d/9720-initializer-self-binding.md new file mode 100644 index 0000000000..7bdea7e1d0 --- /dev/null +++ b/changelog.d/9720-initializer-self-binding.md @@ -0,0 +1,39 @@ +**A closure in a `let`/`const` declarator's own initializer can now see the +binding that declarator introduces** — `const off = ev.on(() => off())`, +`const { unmount } = await render({ onDone: () => unmount() })`. It previously +threw `ReferenceError: is not defined` (#9718). + +The function-body forward-capture pre-pass (`pre_register_forward_captured_lets`) +decides whether a `let`/`const` binding needs a boxed, TDZ-seeded forward +declaration by asking whether any closure *seen so far* references its name. It +recorded a declarator's own initializer into that set only **after** making that +decision — which is all the later-declarator case needs (`let z = (w) => { … O … }, +O = setTimeout(z, K)`, the minified `new Promise` executor shape), but left the +self-referential shape unregistered. The reference then fell through to +`js_global_get_or_throw_unresolved`. Recording the initializer's closure refs +before the decision is a strict superset: later declarators still see them. + +The hole was reachable from every declarator form, because the pre-pass is what +the destructuring path relies on — `destructuring/var_decl.rs`'s own +`is_function_expr_init` pre-registration covers only simple `Pat::Ident` +bindings, and its `ast_expr_contains_function_expr` scan does not descend +through `await`. So `const O = await mk({ cb: () => O() })` (plain binding +behind an `await`), every object/array/nested pattern, and `{ key }` shorthand +all failed; only a closure created *after* the declaration worked. + +Found in claude-code: the `install` subcommand is + +```js +let { unmount: O } = await eB(el(wMA, { onDone: (w, $) => { O(), q(w, $) }, … })) +``` + +so `claude install ` threw an uncaught `ReferenceError: O is not +defined` mid-teardown, losing the final newline and the exit code — perry exited +0 where node exits 1 (the `B60_install_badtarget` divergence carried in #9575). + +`test-files/test_gap_9718_initializer_self_binding.ts` pins all of it against +node: plain binding, object pattern, `{ key }` shorthand, array pattern, nested +pattern, `let` and `const`, with and without `await`, plus a multi-declarator +statement so the reordered scan's pre-existing earlier-refs-later behavior stays +covered. Unpatched, 7 of its 12 lines are `ReferenceError`; patched, the output +is byte-identical to node 26.5.1. diff --git a/changelog.d/9722-closure-body-registry-record.md b/changelog.d/9722-closure-body-registry-record.md new file mode 100644 index 0000000000..5d1c75ffbf --- /dev/null +++ b/changelog.d/9722-closure-body-registry-record.md @@ -0,0 +1,50 @@ +**Closure body registries: one packed record per function instead of ten +maps keyed by the same pointer (#9707).** + +Module init used to record what it knows about each closure body — rest +arity + kind, declared ABI arity, ECMAScript `.length`, arrow / strict / +async / generator / async-generator flags, the two compiler-private +direct-call bodies of an eligible arrow — into ten separate thread-local +`PtrHashMap`s, every one keyed by the same `func_ptr`, plus an eleventh map +memoizing the dispatch strategy those answers imply. Ten key copies, ten +tables' worth of hashbrown load-factor slack, and a dispatch-strategy miss +that probed up to four of them in sequence. On cc's validated heap census +(~59k functions) the family summed to 7.24 MB with the current estimator +(the 11.8 MB the issue quotes was the earlier one, which double-counted +exactly-sized tables). + +`CLOSURE_BODY_REGISTRY` is now the single table: `func_ptr → +ClosureBodyRecord`, a 16-byte record holding `.length`, the declared arity +and the rest arity as integers, and every boolean attribute plus the 2-bit +rest kind as flag bits; `(usize, ClosureBodyRecord)` is a 24-byte bucket, +pinned by a size test. The rare `TrustedDirectTarget` pair (direct-call body +and versioned-loop body) moves to a dense append-only side array +(`TRUSTED_TARGETS`) that only eligible arrows index into, so a body without +them pays four bytes, not two `Option` maps. The +dispatch-strategy cache is deleted outright: a miss now does ONE probe of +the record and derives rest/arity/arrow-ness from its bits, which is cheaper +than the second hash probe the cache cost — and cannot go stale, so the +#6475 late-registration invalidation shrinks to the four-entry +`DISPATCH_RECENT` eviction. Every `js_register_closure_*` entry point and +every `lookup_*` / `is_registered_*` reader keeps its signature; rest still +wins over arity for dispatch and `closure_arity`, and `closure_length` still +prefers the explicit length, then rest, then arity. + +Measured with `PERRY_GC_CENSUS` on a generated 20k-function fixture (5k +each of default-param arrows, rest functions, async functions and +generators; 35,051 registered bodies including the runtime's own): the +closure registry rows go from 2,916,564 bytes across seven populated maps to +1,638,416 bytes in one (`closure.body_registry`), −44 %, with byte-identical +program output. Projected onto cc's recorded census counts (59,384 distinct +bodies, 58.5k of them strict, 27k arrows, 6.8k dispatch-cache entries) the +same estimator gives 3.28 MB against the 7.24 MB before, −55 %; the +remaining floor is hashbrown's power-of-two bucket count at that size. The +census now prints `closure.body_registry` and `closure.trusted_targets` in +place of the ten per-attribute rows. + +Not in this change: the `fn.name_registry` / `fn.source_registry` tables the +issue mentions (5.4 MB on the same census) keep their own `func_ptr` keying — +folding them in wants the dense function-id scheme, which this does not +introduce. `scripts/gc_runtime_root_holders.json` records the two new +statics under the same `not_a_gc_pointer` verdict the deleted maps carried +(code pointers and plain integers only) and drops the eight stale entries. diff --git a/changelog.d/9724-stale-tls-allowlist-entry.md b/changelog.d/9724-stale-tls-allowlist-entry.md new file mode 100644 index 0000000000..8ced0f2eba --- /dev/null +++ b/changelog.d/9724-stale-tls-allowlist-entry.md @@ -0,0 +1,6 @@ +**Drop the stale `readline_helpers.rs` entry from the cold-`thread_local!` +allowlist.** #9697 removed that file's only raw `thread_local!` block but left +its justification behind, and `scripts/check_thread_locals.py` fails a recorded +entry that no longer matches the tree — "a stale entry is one nobody has to +justify". The check runs in `tls-budget.yml`, a satellite workflow outside the +64-gate `lint` set, so it was not caught at merge time. diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index 7ae97d29c5..96243477a1 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -169,7 +169,7 @@ pub(super) fn emit_string_pool( user_fn_wrapper_rest_and_arguments: &std::collections::HashSet, // ABI param count for every top-level user-function wrapper // (`__perry_wrap_`) — used to register the wrapper's - // declared arity in the runtime's `CLOSURE_ARITY_REGISTRY` so dynamic + // declared arity in the runtime's closure body registry so dynamic // dispatch can pad missing trailing args before invoking the wrapper. // Entries for wrappers also present in `user_fn_wrapper_rest` are skipped // (those go through the rest registry which already controls dispatch). @@ -769,7 +769,7 @@ pub(super) fn emit_string_pool( // length, which mis-split dynamic-parent (capless-sig-with-snapshot) ctors. let mut ctor_triples: Vec<(u32, String, u32, u32)> = Vec::new(); // #wall3: class ctors with a rest param (`constructor(...args)`) need their - // standalone `_constructor` func_ptr registered in CLOSURE_REST_REGISTRY so + // standalone `_constructor` func_ptr registered as rest-bearing in the closure body registry so // a member-new (`new ns.Sub(opts)` → js_new_function_construct → // js_native_call_value) BUNDLES trailing args into the rest array. Without // this the rest param binds to the first arg as a scalar (a=opts, not diff --git a/crates/perry-hir/src/lower_decl/block.rs b/crates/perry-hir/src/lower_decl/block.rs index c2315806a2..b20d746865 100644 --- a/crates/perry-hir/src/lower_decl/block.rs +++ b/crates/perry-hir/src/lower_decl/block.rs @@ -178,6 +178,32 @@ pub(crate) fn pre_register_forward_captured_lets( ast::VarDeclKind::Let | ast::VarDeclKind::Const ) { for decl in &var_decl.decls { + // A closure in a declarator's OWN initializer can + // reference the binding that declarator introduces: + // `const off = ev.on(() => off())` + // `const { unmount: O } = await render({ onDone: () => O() })` + // (claude-code's `install` subcommand, #9718). The + // closure body does not run until after initialization, + // so this is legal — node only has a TDZ window here. + // Record this declarator's closure refs BEFORE deciding + // whether its own bindings are forward-captured; doing + // it only afterwards (which is all the LATER-declarator + // case below needs) left the self-referential shape + // unregistered, so the reference fell through to + // `js_global_get_or_throw_unresolved` and threw + // `ReferenceError: is not defined`. + // + // Recording early is a superset of recording late: later + // declarators of the same declaration still see these + // refs, which is what the intra-declaration case wants: + // `let z = (w) => { … O … }, O = setTimeout(z, K);` + // (the minified `new Promise` executor shape — without + // it the `resolve` never fires and the awaiting caller + // hangs). Cross-statement forward-refs are handled by + // the trailing `cic_stmt`. + if let Some(init) = &decl.init { + cic_expr(init, false, &mut seen_closure_refs); + } let mut binding_idents: Vec<(String, u32)> = Vec::new(); collect_pat_forward_idents(&decl.name, &mut binding_idents); for (name, span_lo) in binding_idents { @@ -213,27 +239,6 @@ pub(crate) fn pre_register_forward_captured_lets( } } } - // A closure in an EARLIER declarator of THIS same - // `let`/`const` can forward-reference a name bound by a LATER - // declarator in the SAME declaration: - // `let z = (w) => { … O … Y … A … }, - // Y = () => z(false), - // A = () => clearTimeout(O), - // O = setTimeout(z, K);` - // (the minified `new Promise` executor shape). Record this - // declarator's closure refs NOW so the later declarators are - // seen as forward-captured too — `seen_closure_refs` is - // otherwise only updated by the trailing `cic_stmt` AFTER the - // whole declaration, so intra-declaration forward-refs were - // missed: the later names never got pre-registered, so the - // ref fell through to `js_global_get_or_throw_unresolved` and - // the closure captured a global instead of the local box — - // e.g. a `new Promise` `resolve` that never fires, hanging - // the awaiting caller. Cross-statement forward-refs were - // already handled by the trailing `cic_stmt`. - if let Some(init) = &decl.init { - cic_expr(init, false, &mut seen_closure_refs); - } } } else { // `var` bindings are already predefined + boxed by diff --git a/crates/perry-runtime/src/async_hooks.rs b/crates/perry-runtime/src/async_hooks.rs index 05490c2f1d..16a41515dd 100644 --- a/crates/perry-runtime/src/async_hooks.rs +++ b/crates/perry-runtime/src/async_hooks.rs @@ -1602,7 +1602,7 @@ extern "C" fn async_resource_bind_trampoline(closure: *const ClosureHeader, rest fn register_bind_trampoline_once() { thread_local! { - // CLOSURE_REST_REGISTRY is thread-local, so each thread that + // The closure body registry is thread-local, so each thread that // synthesizes a bind() trampoline must register the func_ptr once. static REGISTERED: Cell = const { Cell::new(false) }; } diff --git a/crates/perry-runtime/src/closure/registry.rs b/crates/perry-runtime/src/closure/registry.rs index 615ff2cd4d..f77ba5dcc3 100644 --- a/crates/perry-runtime/src/closure/registry.rs +++ b/crates/perry-runtime/src/closure/registry.rs @@ -11,36 +11,40 @@ pub(crate) struct TrustedDirectTarget { pub boxed_capture_mask: u64, } -// Side-table mapping closure body `func_ptr` -> fixed_arity (number of fixed -// params declared BEFORE the rest param). Populated at module init by -// `js_register_closure_rest` for every closure body whose HIR signature ends -// in `...rest`. Looked up by `js_closure_callN` so that calls through dynamic -// dispatch (e.g. `obj.cb(a, b, c)` where `cb` is a class field holding an -// arrow) bundle trailing args into the rest array — the previous behavior -// passed unbundled args, leaving the rest param bound to the first trailing -// arg as a scalar (issue #493 / #421-rest fix). Static call sites (named -// functions, `Expr::FuncRef`, local closure-bound `let`) keep their existing -// bundling at the call site, which is faster — the registry is consulted only -// when needed. -// -// Stored as a thread-local rather than a global RwLock because closures are -// thread-local in perry's runtime model (each thread has its own arena + -// GC), so a per-thread copy avoids the per-call lock acquisition. Module -// init runs on the main thread and populates one entry per -// rest-param-bearing closure body in the program; worker threads (issue -// #29 `perry/thread`) currently don't see the table because they aren't -// supposed to invoke arbitrary user closures across the boundary anyway. -crate::perry_thread_local! { - /// (fixed_arity, kind) — kind describes whether the function has an - /// ordinary user rest param, a synthesized `arguments` rest param, or - /// both a user rest param plus a hidden raw-arguments slot. - static CLOSURE_REST_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); - /// Side-table mapping closure body `func_ptr` -> declared param count - /// (for closures WITHOUT a rest param — those use CLOSURE_REST_REGISTRY). - /// Populated at module init by `js_register_closure_arity`. Looked up by - /// `js_native_call_value` (the dynamic dispatch path used when a closure is - /// stored as a class field and called method-style on an any-typed +/// Everything module init records about one closure body, keyed by the +/// body's `func_ptr` in `CLOSURE_BODY_REGISTRY`. +/// +/// #9707: these attributes used to live in TEN separate thread-local maps +/// (rest, arity, length, arrow, versioned-loop, strict, async, generator, +/// async-generator, dispatch cache), every one keyed by the same code +/// pointer. A ~59k-function program paid ~200 B per function for that — +/// ten key copies, ten buckets' worth of load-factor slack, and up to four +/// hash probes on a dispatch miss. One 16-byte record per body holds all of +/// it: the two arities and `.length` inline, the boolean attributes as flag +/// bits, and the rare compiler-private direct-call bodies as an index into +/// `TRUSTED_TARGETS` (only the eligible-arrow subset pays for those 64 +/// bytes). A dispatch-strategy miss is now ONE probe that yields every +/// input the strategy needs, so the separate dispatch cache is gone too. +/// +/// Stored as a thread-local rather than a global RwLock because closures are +/// thread-local in perry's runtime model (each thread has its own arena + +/// GC), so a per-thread copy avoids the per-call lock acquisition. Module +/// init runs on the main thread and populates one record per registered +/// body in the program; worker threads (issue #29 `perry/thread`) currently +/// don't see the table because they aren't supposed to invoke arbitrary user +/// closures across the boundary anyway. +#[derive(Clone, Copy, Default)] +pub(crate) struct ClosureBodyRecord { + /// ECMAScript-visible `.length` (`js_register_closure_length`), valid + /// when `HAS_LENGTH` is set. Unlike declared arity, function length stops + /// at the first parameter with a default and before rest. Kept separate + /// from `arity` because dispatch padding needs the body's real ABI arity + /// while `fn.length` needs the ECMAScript-visible length. + length: u32, + /// Declared ABI param count (`js_register_closure_arity`), valid when + /// `HAS_ARITY` is set — for closures WITHOUT a rest param. Looked up by + /// `js_native_call_value` (the dynamic dispatch path used when a closure + /// is stored as a class field and called method-style on an any-typed /// receiver) so the runtime can pad missing args with TAG_UNDEFINED to /// match the closure body's declared arity. Without this, calling a /// 3-param arrow with 1 arg through `js_native_call_method` → @@ -50,82 +54,190 @@ crate::perry_thread_local! { /// hit exactly this; uninit `headers` slot evaluated to a small denormal /// float, slow-path runs, `setDefaultContentType` returns a header object, /// `responseHeaders.set(k, v)` then fails with a #510-class TypeError). - static CLOSURE_ARITY_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + /// + /// `u16`: closure dispatch tops out at 16 args (`js_closure_callN`), so + /// the narrow field costs nothing; a registration above `u16::MAX` + /// saturates rather than wraps. + arity: u16, + /// Number of fixed params declared BEFORE the rest param, valid when + /// `HAS_REST` is set (the rest KIND rides in `REST_KIND_MASK`). Populated + /// at module init by `js_register_closure_rest` for every closure body + /// whose HIR signature ends in `...rest`. Looked up by `js_closure_callN` + /// so that calls through dynamic dispatch (e.g. `obj.cb(a, b, c)` where + /// `cb` is a class field holding an arrow) bundle trailing args into the + /// rest array — the previous behavior passed unbundled args, leaving the + /// rest param bound to the first trailing arg as a scalar (issue #493 / + /// #421-rest fix). Static call sites (named functions, `Expr::FuncRef`, + /// local closure-bound `let`) keep their existing bundling at the call + /// site, which is faster — the registry is consulted only when needed. + rest_arity: u16, + /// `HAS_*` presence bits plus the boolean attributes (`ARROW`, `STRICT`, + /// `ASYNC`, `GENERATOR`, `ASYNC_GENERATOR`) and the 2-bit rest kind. + flags: u16, + /// 1-based index into `TRUSTED_TARGETS`; 0 = this body has no + /// compiler-private direct-call bodies. Only ever non-zero on an arrow. + targets: u32, +} - /// Side-table mapping closure body `func_ptr` -> spec `.length`. - /// Unlike declared arity, function length stops at the first parameter - /// with a default and before rest. Keep this separate from - /// CLOSURE_ARITY_REGISTRY because dispatch padding needs the body's real - /// ABI arity while `fn.length` needs the ECMAScript-visible length. - static CLOSURE_LENGTH_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); +/// `ClosureBodyRecord::flags` bits. +mod body_flags { + /// `arity` is valid (`js_register_closure_arity` ran for this body). + pub(super) const HAS_ARITY: u16 = 1 << 0; + /// `rest_arity` + `REST_KIND_MASK` are valid (one of the three + /// `js_register_closure_rest*` entry points ran for this body). + pub(super) const HAS_REST: u16 = 1 << 1; + /// Two-bit `RestDispatchKind` discriminant, valid iff `HAS_REST`. + pub(super) const REST_KIND_SHIFT: u32 = 2; + pub(super) const REST_KIND_MASK: u16 = 0b11 << REST_KIND_SHIFT; + /// `length` is valid (`js_register_closure_length` ran for this body). + pub(super) const HAS_LENGTH: u16 = 1 << 4; + /// The body came from an arrow function. Arrows are callable but not + /// constructable and inherit the restricted Function.prototype + /// caller/arguments accessors. A small eligible subset also records an + /// internal direct-call body that may use compiler-proven capture + /// invariants (see `targets`). Dynamic callers retain the public body; + /// only the exact-arrow resolver reads that optional target. + pub(super) const ARROW: u16 = 1 << 5; + /// The body is strict-mode code (file-level `"use strict"` or a body + /// directive). Drives OrdinaryCallBindThis in `call`/`apply`/`bind`: a + /// strict callee observes the raw primitive `thisArg`; a sloppy user + /// callee gets it boxed once. + pub(super) const STRICT: u16 = 1 << 6; + /// The body came from an async function. `util.types.isAsyncFunction` + /// uses this when the predicate sees a runtime closure value instead of + /// a statically-known HIR node. Async generators set it too. + pub(super) const ASYNC: u16 = 1 << 7; + /// The source function was a generator function. The generator transform + /// clears HIR's `is_generator` flag after lowering to a state machine, so + /// codegen registers the wrapper/closure symbols here for util.types + /// identity tests. + pub(super) const GENERATOR: u16 = 1 << 8; + /// #3664: the body came from an `async function*`. Async generators also + /// carry `GENERATOR` (they lower to the same `{next,return,throw}` + /// wrapper as sync generators), so this bit is what distinguishes the + /// two — it drives `%AsyncGeneratorFunction%` vs `%GeneratorFunction%` + /// intrinsic resolution. + pub(super) const ASYNC_GENERATOR: u16 = 1 << 9; +} - /// Side-table marking closure body `func_ptr`s that came from arrow - /// functions. Arrows are callable but not constructable and inherit the - /// restricted Function.prototype caller/arguments accessors. A small - /// eligible subset also records an internal direct-call body that may use - /// compiler-proven capture invariants. Dynamic callers retain the public - /// body; only the exact-arrow resolver reads that optional target. - static CLOSURE_ARROW_FUNCTION_REGISTRY: - RefCell>> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); +impl ClosureBodyRecord { + #[inline(always)] + fn has(self, bit: u16) -> bool { + self.flags & bit != 0 + } - /// Exact arrows with a compiler-private callback body whose cold arms - /// poison the caller's versioned loop before observable fallback code. - static CLOSURE_VERSIONED_LOOP_REGISTRY: - RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + #[inline(always)] + fn arity(self) -> Option { + self.has(body_flags::HAS_ARITY) + .then_some(u32::from(self.arity)) + } - /// Side-table marking closure body `func_ptr`s whose body is strict-mode - /// code (file-level `"use strict"` or a body directive). Drives - /// OrdinaryCallBindThis in `call`/`apply`/`bind`: a strict callee - /// observes the raw primitive `thisArg`; a sloppy user callee gets it - /// boxed once. - static CLOSURE_STRICT_FUNCTION_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + #[inline(always)] + fn rest(self) -> Option<(u32, RestDispatchKind)> { + if !self.has(body_flags::HAS_REST) { + return None; + } + let kind = match (self.flags & body_flags::REST_KIND_MASK) >> body_flags::REST_KIND_SHIFT { + 0 => RestDispatchKind::UserRest, + 1 => RestDispatchKind::SyntheticArguments, + _ => RestDispatchKind::UserRestAndArguments, + }; + Some((u32::from(self.rest_arity), kind)) + } - /// Side-table marking closure body `func_ptr`s that came from async - /// functions. `util.types.isAsyncFunction` uses this when the predicate - /// sees a runtime closure value instead of a statically-known HIR node. - static CLOSURE_ASYNC_FUNCTION_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + #[inline(always)] + fn length(self) -> Option { + self.has(body_flags::HAS_LENGTH).then_some(self.length) + } - /// Side-table mapping closure body `func_ptr` -> true when the source - /// function was a plain generator function. The generator transform clears - /// HIR's `is_generator` flag after lowering to a state machine, so codegen - /// registers the wrapper/closure symbols here for util.types identity tests. - static CLOSURE_GENERATOR_FUNCTION_REGISTRY: RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + fn set_rest(&mut self, fixed_arity: u32, kind: RestDispatchKind) { + let kind_bits: u16 = match kind { + RestDispatchKind::UserRest => 0, + RestDispatchKind::SyntheticArguments => 1, + RestDispatchKind::UserRestAndArguments => 2, + }; + self.rest_arity = saturate_u16(fixed_arity); + self.flags = (self.flags & !body_flags::REST_KIND_MASK) + | body_flags::HAS_REST + | (kind_bits << body_flags::REST_KIND_SHIFT); + } - /// #3664: side-table marking closure body `func_ptr`s that came from an - /// `async function*`. Async generators also live in - /// `CLOSURE_GENERATOR_FUNCTION_REGISTRY` (they lower to the same - /// `{next,return,throw}` wrapper as sync generators), so this registry is - /// what distinguishes the two — it drives `%AsyncGeneratorFunction%` vs - /// `%GeneratorFunction%` intrinsic resolution. - static CLOSURE_ASYNC_GENERATOR_FUNCTION_REGISTRY: - RefCell> = - RefCell::new(crate::fast_hash::new_ptr_hash_map()); + /// The dispatch strategy this body's attributes imply. Pure bit tests on + /// an already-fetched record, so a strategy miss costs exactly one hash + /// probe (the record's) instead of the old cache + rest + arity + arrow + /// chain. + #[inline(always)] + fn dispatch_strategy(self) -> DispatchStrategy { + let kind = if let Some((fixed_arity, rest_kind)) = self.rest() { + DispatchKind::Rest(fixed_arity, rest_kind) + } else if let Some(declared) = self.arity() { + DispatchKind::Arity(declared) + } else { + DispatchKind::Direct + }; + DispatchStrategy { + kind, + is_arrow: self.has(body_flags::ARROW), + } + } +} + +#[inline(always)] +fn saturate_u16(value: u32) -> u16 { + debug_assert!( + value <= u32::from(u16::MAX), + "closure arity {value} exceeds the u16 registry field" + ); + u16::try_from(value).unwrap_or(u16::MAX) +} + +/// The compiler-private direct-call bodies of one eligible arrow, reached +/// from `ClosureBodyRecord::targets`. +#[derive(Clone, Copy, Default)] +struct TrustedTargets { + /// Internal direct-call body that may use compiler-proven capture + /// invariants (`js_register_closure_trusted_direct`). + direct: Option, + /// Compiler-private callback body whose cold arms poison the caller's + /// versioned loop before observable fallback code + /// (`js_register_closure_versioned_loop_direct`). + versioned_loop: Option, +} - /// Unified dispatch lookup, populated lazily on first call to a func_ptr. - /// Cuts the per-call cost from TWO RefCell::borrow + HashMap::get - /// (one each for rest and arity) down to ONE — material on hot paths - /// like `array.sort` (25M comparisons) or `Promise.all` of N async - /// chains (150k microtasks for the 1k-batch x 50-promise x 3-await - /// shape). The fast path on a cache hit is one borrow + one - /// HashMap::get + a small-enum branch. - static DISPATCH_CACHE: RefCell> = +crate::perry_thread_local! { + /// One `ClosureBodyRecord` per registered closure body, keyed by the + /// body's `func_ptr`. See the record's doc for what used to be here. + static CLOSURE_BODY_REGISTRY: RefCell> = RefCell::new(crate::fast_hash::new_ptr_hash_map()); + + /// Dense side array for the eligible-arrow subset that has trusted + /// direct-call bodies; `ClosureBodyRecord::targets` is a 1-based index + /// into it. Append-only, so an index never moves. + static TRUSTED_TARGETS: RefCell> = RefCell::new(Vec::new()); +} + +/// The record for `func_ptr`, if module init registered anything about it. +#[inline(always)] +fn body_record(func_ptr: *const u8) -> Option { + CLOSURE_BODY_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied()) +} + +/// Run `update` against the (possibly fresh) record for `func_ptr`. +#[inline(always)] +fn update_body_record(func_ptr: *const u8, update: impl FnOnce(&mut ClosureBodyRecord)) { + CLOSURE_BODY_REGISTRY.with(|r| { + update(r.borrow_mut().entry(func_ptr as usize).or_default()); + }); } /// Magic value stored in ClosureHeader._reserved to identify closures at runtime. /// Used by js_value_typeof to return "function" instead of "object" for closures. pub const CLOSURE_MAGIC: u32 = 0x434C_4F53; // "CLOS" in ASCII -/// Per-call dispatch strategy for a closure body. Decided once at first -/// call, cached in `DISPATCH_CACHE` thereafter. Arrow-ness rides in the same -/// cache entry so receiverless calls can avoid a second TLS registry lookup. +/// Per-call dispatch strategy for a closure body. Derived from the body's +/// `ClosureBodyRecord` on a miss and memoized in `DISPATCH_RECENT`. +/// Arrow-ness rides in the same entry so receiverless calls can avoid a +/// second TLS registry lookup. #[derive(Clone, Copy)] pub struct DispatchStrategy { kind: DispatchKind, @@ -265,73 +377,47 @@ fn resolve_strategy_slow(func_ptr: *const u8) -> DispatchStrategy { #[cfg(test)] RESOLVE_STRATEGY_SLOW_CALLS.with(|calls| calls.set(calls.get() + 1)); - let key = func_ptr as usize; - // Fast path: read existing cache entry. - if let Some(s) = DISPATCH_CACHE.with(|c| c.borrow().get(&key).copied()) { - return s; - } - // First call for this func_ptr: compute the strategy and cache it. if func_ptr == BOUND_METHOD_FUNC_PTR { - let strategy = DispatchStrategy { + return DispatchStrategy { kind: DispatchKind::BoundMethod, is_arrow: false, }; - DISPATCH_CACHE.with(|c| { - c.borrow_mut().insert(key, strategy); - }); - return strategy; } if func_ptr == BOUND_FUNCTION_FUNC_PTR { - let strategy = DispatchStrategy { + return DispatchStrategy { kind: DispatchKind::BoundFunction, is_arrow: false, }; - DISPATCH_CACHE.with(|c| { - c.borrow_mut().insert(key, strategy); - }); - return strategy; } - let kind = if let Some((fixed_arity, synthetic)) = lookup_closure_rest_full(func_ptr) { - DispatchKind::Rest(fixed_arity, synthetic) - } else if let Some(declared) = lookup_closure_arity(func_ptr) { - DispatchKind::Arity(declared) - } else { - DispatchKind::Direct - }; - let strategy = DispatchStrategy { - kind, - is_arrow: is_registered_arrow_function(func_ptr), - }; - DISPATCH_CACHE.with(|c| { - c.borrow_mut().insert(key, strategy); - }); - strategy + // One probe of the unified record yields rest, arity AND arrow-ness; + // an unregistered body (no record) is a plain direct callable. There is + // no separate memo of the answer any more: deriving it from the record + // is a couple of bit tests, cheaper than the second hash probe the old + // `DISPATCH_CACHE` cost, and it cannot go stale — a late registration + // (#6475) rewrites the very record the next miss reads. + match body_record(func_ptr) { + Some(record) => record.dispatch_strategy(), + None => DispatchStrategy::direct(), + } } -/// Register that the closure body at `func_ptr` has a rest parameter at index -/// `fixed_arity` (i.e., the closure has `fixed_arity` fixed params before the -/// rest param, and its declared LLVM arity is `fixed_arity + 1` — the +1 is -/// the rest array). Called once per closure literal at module init time. - -/// #6475: purge a func_ptr's cached dispatch strategy. The strategy caches -/// (`DISPATCH_CACHE` + `DISPATCH_RECENT`) memoize the FIRST -/// resolution per func_ptr — but effect's module-init graph calls `.pipe` -/// (an `arguments`-object method) during init, and inside an import cycle -/// such a call can precede the module's own `js_register_closure_*` batch. -/// The pre-registration miss resolved to `Arity`/`Direct` and was cached -/// FOREVER: every later `obj.pipe(a, b, c)` transmuted three positional args -/// onto the 1-slot `arguments` ABI, so `arguments.length` read 1 and effect's +/// #6475: evict a func_ptr from the recent-bodies dispatch cache. +/// `DISPATCH_RECENT` memoizes the FIRST resolution per func_ptr — but +/// effect's module-init graph calls `.pipe` (an `arguments`-object method) +/// during init, and inside an import cycle such a call can precede the +/// module's own `js_register_closure_*` batch. The pre-registration miss +/// resolved to `Arity`/`Direct` and was cached FOREVER: every later +/// `obj.pipe(a, b, c)` transmuted three positional args onto the 1-slot +/// `arguments` ABI, so `arguments.length` read 1 and effect's /// `pipeArguments` applied only the first stage (`HttpApiBuilder.group` /// returned the wrong pipe stage; web.ts died with "Not a valid effect: /// undefined"). Registration is init-time-rare, so invalidating here keeps -/// the hot-path caches lock-free and unchanged. Also covers the late arity -/// registration for plain closures. +/// the hot-path cache lock-free and unchanged. Also covers the late arity +/// registration for plain closures. (The hash-level dispatch cache this +/// used to purge as well is gone — #9707 derives the strategy from the +/// registration record itself, so there is nothing else to go stale.) fn invalidate_dispatch_strategy(func_ptr: *const u8) { - let key = func_ptr as usize; - DISPATCH_CACHE.with(|c| { - c.borrow_mut().remove(&key); - }); - DISPATCH_RECENT.with(|cache| cache.invalidate(key)); + DISPATCH_RECENT.with(|cache| cache.invalidate(func_ptr as usize)); } #[cfg(test)] @@ -407,9 +493,8 @@ pub extern "C" fn js_register_closure_rest(func_ptr: *const u8, fixed_arity: u32 if func_ptr.is_null() { return; } - CLOSURE_REST_REGISTRY.with(|r| { - r.borrow_mut() - .insert(func_ptr as usize, (fixed_arity, RestDispatchKind::UserRest)); + update_body_record(func_ptr, |record| { + record.set_rest(fixed_arity, RestDispatchKind::UserRest); }); invalidate_dispatch_strategy(func_ptr); } @@ -428,11 +513,8 @@ pub extern "C" fn js_register_closure_synthetic_arguments(func_ptr: *const u8, f if func_ptr.is_null() { return; } - CLOSURE_REST_REGISTRY.with(|r| { - r.borrow_mut().insert( - func_ptr as usize, - (fixed_arity, RestDispatchKind::SyntheticArguments), - ); + update_body_record(func_ptr, |record| { + record.set_rest(fixed_arity, RestDispatchKind::SyntheticArguments); }); invalidate_dispatch_strategy(func_ptr); } @@ -446,11 +528,8 @@ pub extern "C" fn js_register_closure_rest_and_arguments(func_ptr: *const u8, fi if func_ptr.is_null() { return; } - CLOSURE_REST_REGISTRY.with(|r| { - r.borrow_mut().insert( - func_ptr as usize, - (fixed_arity, RestDispatchKind::UserRestAndArguments), - ); + update_body_record(func_ptr, |record| { + record.set_rest(fixed_arity, RestDispatchKind::UserRestAndArguments); }); invalidate_dispatch_strategy(func_ptr); } @@ -460,9 +539,7 @@ pub extern "C" fn js_register_closure_async_function(func_ptr: *const u8) { if func_ptr.is_null() { return; } - CLOSURE_ASYNC_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, ()); - }); + update_body_record(func_ptr, |record| record.flags |= body_flags::ASYNC); } #[inline(always)] @@ -470,33 +547,30 @@ pub fn is_registered_async_function(func_ptr: *const u8) -> bool { if func_ptr.is_null() { return false; } - CLOSURE_ASYNC_FUNCTION_REGISTRY.with(|r| r.borrow().contains_key(&(func_ptr as usize))) + body_record(func_ptr).is_some_and(|record| record.has(body_flags::ASYNC)) } #[inline(always)] pub fn lookup_closure_rest(func_ptr: *const u8) -> Option { - CLOSURE_REST_REGISTRY.with(|r| { - r.borrow() - .get(&(func_ptr as usize)) - .map(|(arity, _)| *arity) - }) + lookup_closure_rest_full(func_ptr).map(|(arity, _)| arity) } #[inline(always)] pub fn lookup_closure_rest_full(func_ptr: *const u8) -> Option<(u32, RestDispatchKind)> { - CLOSURE_REST_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied()) + body_record(func_ptr)?.rest() } /// Register a closure body's declared param count (for closures WITHOUT a rest /// param). Called once per non-rest closure literal at module init time. -/// See `CLOSURE_ARITY_REGISTRY` doc for rationale. +/// See `ClosureBodyRecord::arity` for rationale. #[no_mangle] pub extern "C" fn js_register_closure_arity(func_ptr: *const u8, arity: u32) { if func_ptr.is_null() { return; } - CLOSURE_ARITY_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, arity); + update_body_record(func_ptr, |record| { + record.arity = saturate_u16(arity); + record.flags |= body_flags::HAS_ARITY; }); // #6475: same pre-registration cache-poisoning hazard as the rest-family // registrations above — a call before this registration caches `Direct`. @@ -505,7 +579,7 @@ pub extern "C" fn js_register_closure_arity(func_ptr: *const u8, arity: u32) { #[inline(always)] pub fn lookup_closure_arity(func_ptr: *const u8) -> Option { - CLOSURE_ARITY_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied()) + body_record(func_ptr)?.arity() } /// Register a closure body's ECMAScript `.length` value. @@ -514,14 +588,15 @@ pub extern "C" fn js_register_closure_length(func_ptr: *const u8, length: u32) { if func_ptr.is_null() { return; } - CLOSURE_LENGTH_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, length); + update_body_record(func_ptr, |record| { + record.length = length; + record.flags |= body_flags::HAS_LENGTH; }); } #[inline(always)] pub fn lookup_closure_length(func_ptr: *const u8) -> Option { - CLOSURE_LENGTH_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied()) + body_record(func_ptr)?.length() } #[no_mangle] @@ -529,12 +604,10 @@ pub extern "C" fn js_register_closure_arrow_function(func_ptr: *const u8) { if func_ptr.is_null() { return; } - CLOSURE_ARROW_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().entry(func_ptr as usize).or_insert(None); - }); + update_body_record(func_ptr, |record| record.flags |= body_flags::ARROW); // A closure can be invoked during a cyclic module-init graph before its // metadata batch runs. Keep arrow-ness coherent with the rest/arity data - // stored in the unified dispatch cache. + // memoized in the recent-bodies dispatch cache. invalidate_dispatch_strategy(func_ptr); } @@ -543,7 +616,46 @@ pub fn is_registered_arrow_function(func_ptr: *const u8) -> bool { if func_ptr.is_null() { return false; } - CLOSURE_ARROW_FUNCTION_REGISTRY.with(|r| r.borrow().contains_key(&(func_ptr as usize))) + body_record(func_ptr).is_some_and(|record| record.has(body_flags::ARROW)) +} + +/// Attach one of the compiler-private direct-call bodies to the arrow at +/// `public_func_ptr`, allocating its `TRUSTED_TARGETS` slot on first use. +/// Refuses (returns `false`) for a body that is not a registered arrow, so a +/// registration can never turn an ordinary function into an arrow. +fn attach_trusted_target( + public_func_ptr: *const u8, + target: TrustedDirectTarget, + select: impl FnOnce(&mut TrustedTargets) -> &mut Option, +) -> bool { + CLOSURE_BODY_REGISTRY.with(|r| { + let mut registry = r.borrow_mut(); + let Some(record) = registry.get_mut(&(public_func_ptr as usize)) else { + return false; + }; + if !record.has(body_flags::ARROW) { + return false; + } + TRUSTED_TARGETS.with(|targets| { + let mut targets = targets.borrow_mut(); + if record.targets == 0 { + targets.push(TrustedTargets::default()); + record.targets = u32::try_from(targets.len()) + .expect("more than u32::MAX trusted closure bodies"); + } + *select(&mut targets[record.targets as usize - 1]) = Some(target); + }); + true + }) +} + +#[inline(always)] +fn trusted_targets(func_ptr: *const u8) -> Option { + let record = body_record(func_ptr)?; + if record.targets == 0 { + return None; + } + TRUSTED_TARGETS.with(|targets| targets.borrow().get(record.targets as usize - 1).copied()) } /// Associate a public closure body with its compiler-private direct-call @@ -558,24 +670,20 @@ pub extern "C" fn js_register_closure_trusted_direct( if public_func_ptr.is_null() || trusted_func_ptr.is_null() { return; } - CLOSURE_ARROW_FUNCTION_REGISTRY.with(|r| { - let mut registry = r.borrow_mut(); - let Some(target) = registry.get_mut(&(public_func_ptr as usize)) else { - // Registration must never turn an ordinary function into an arrow. - return; - }; - *target = Some(TrustedDirectTarget { + attach_trusted_target( + public_func_ptr, + TrustedDirectTarget { func_ptr: trusted_func_ptr, capture_count, boxed_capture_mask, - }); - }); + }, + |targets| &mut targets.direct, + ); } #[inline(always)] pub(crate) fn lookup_closure_trusted_direct(func_ptr: *const u8) -> Option { - CLOSURE_ARROW_FUNCTION_REGISTRY - .with(|r| r.borrow().get(&(func_ptr as usize)).copied().flatten()) + trusted_targets(func_ptr)?.direct } #[no_mangle] @@ -588,26 +696,22 @@ pub extern "C" fn js_register_closure_versioned_loop_direct( if public_func_ptr.is_null() || versioned_func_ptr.is_null() { return; } - if !is_registered_arrow_function(public_func_ptr) { - return; - } - CLOSURE_VERSIONED_LOOP_REGISTRY.with(|r| { - r.borrow_mut().insert( - public_func_ptr as usize, - TrustedDirectTarget { - func_ptr: versioned_func_ptr, - capture_count, - boxed_capture_mask, - }, - ); - }); + attach_trusted_target( + public_func_ptr, + TrustedDirectTarget { + func_ptr: versioned_func_ptr, + capture_count, + boxed_capture_mask, + }, + |targets| &mut targets.versioned_loop, + ); } #[inline(always)] pub(crate) fn lookup_closure_versioned_loop_direct( func_ptr: *const u8, ) -> Option { - CLOSURE_VERSIONED_LOOP_REGISTRY.with(|r| r.borrow().get(&(func_ptr as usize)).copied()) + trusted_targets(func_ptr)?.versioned_loop } /// Keepalive anchor for the auto-optimize whole-program build — registration @@ -633,9 +737,7 @@ pub extern "C" fn js_register_closure_strict_function(func_ptr: *const u8) { if func_ptr.is_null() { return; } - CLOSURE_STRICT_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, ()); - }); + update_body_record(func_ptr, |record| record.flags |= body_flags::STRICT); } /// Keepalive anchor for the auto-optimize whole-program build — the strict @@ -650,7 +752,7 @@ pub fn is_registered_strict_function(func_ptr: *const u8) -> bool { if func_ptr.is_null() { return false; } - CLOSURE_STRICT_FUNCTION_REGISTRY.with(|r| r.borrow().contains_key(&(func_ptr as usize))) + body_record(func_ptr).is_some_and(|record| record.has(body_flags::STRICT)) } pub fn closure_is_arrow(closure: *const ClosureHeader) -> bool { @@ -682,16 +784,12 @@ pub extern "C" fn js_register_closure_generator_function(func_ptr: *const u8) { if func_ptr.is_null() { return; } - CLOSURE_GENERATOR_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, true); - }); + update_body_record(func_ptr, |record| record.flags |= body_flags::GENERATOR); } #[inline(always)] pub fn is_registered_generator_function(func_ptr: *const u8) -> bool { - CLOSURE_GENERATOR_FUNCTION_REGISTRY - .with(|r| r.borrow().get(&(func_ptr as usize)).copied()) - .unwrap_or(false) + body_record(func_ptr).is_some_and(|record| record.has(body_flags::GENERATOR)) } /// #3664: register a closure body `func_ptr` as an `async function*`. Also @@ -702,11 +800,8 @@ pub extern "C" fn js_register_closure_async_generator_function(func_ptr: *const if func_ptr.is_null() { return; } - CLOSURE_ASYNC_GENERATOR_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, ()); - }); - CLOSURE_ASYNC_FUNCTION_REGISTRY.with(|r| { - r.borrow_mut().insert(func_ptr as usize, ()); + update_body_record(func_ptr, |record| { + record.flags |= body_flags::ASYNC_GENERATOR | body_flags::ASYNC; }); } @@ -715,8 +810,7 @@ pub fn is_registered_async_generator_function(func_ptr: *const u8) -> bool { if func_ptr.is_null() { return false; } - CLOSURE_ASYNC_GENERATOR_FUNCTION_REGISTRY - .with(|r| r.borrow().contains_key(&(func_ptr as usize))) + body_record(func_ptr).is_some_and(|record| record.has(body_flags::ASYNC_GENERATOR)) } /// Public helper: given a `*const ClosureHeader` pointer, return the @@ -727,13 +821,14 @@ pub fn closure_arity(closure: *const ClosureHeader) -> Option { if func_ptr.is_null() { return None; } + let record = body_record(func_ptr)?; // Closures declared with `...rest` register through a separate // registry path; prefer the fixed-arity portion of that entry when // present so `length` matches the user-visible declared params. - if let Some((arity, _synth)) = lookup_closure_rest_full(func_ptr) { + if let Some((arity, _synth)) = record.rest() { return Some(arity); } - lookup_closure_arity(func_ptr) + record.arity() } /// Public helper for the ECMAScript-visible function `.length`. @@ -749,13 +844,14 @@ pub fn closure_length(closure: *const ClosureHeader) -> Option { if func_ptr.is_null() { return None; } - if let Some(length) = lookup_closure_length(func_ptr) { + let record = body_record(func_ptr)?; + if let Some(length) = record.length() { return Some(length); } - if let Some((arity, _synth)) = lookup_closure_rest_full(func_ptr) { + if let Some((arity, _synth)) = record.rest() { return Some(arity); } - lookup_closure_arity(func_ptr) + record.arity() } /// Build a JS array from a slice of NaN-boxed f64 values and return it @@ -1082,7 +1178,7 @@ pub unsafe fn dispatch_rest_bundled( /// correctly initialised slots instead of stale registers. /// /// `func_ptr` is already validated and known non-BOUND, non-rest. -/// `declared_arity` is what `CLOSURE_ARITY_REGISTRY` recorded for this body +/// `declared_arity` is what `js_register_closure_arity` recorded for this body /// at module init time. Callers reach here only when `args.len() < declared_arity`. /// /// Refs #420: drizzle's `pgTable` is `(name, columns, extraConfig) => …` @@ -1240,50 +1336,210 @@ pub fn real_capture_count(capture_count: u32) -> u32 { capture_count & !(CAPTURES_THIS_FLAG | NO_THIS_REBIND_FLAG) } -/// `PERRY_GC_CENSUS`: entries and estimated bytes of every per-`func_ptr` -/// registry filled from module init. +/// `PERRY_GC_CENSUS`: entries and estimated bytes of the per-`func_ptr` +/// registry filled from module init. `closure.body_registry` is the unified +/// record table (#9707 — it replaced the ten `closure.*_registry` / +/// `closure.dispatch_cache` rows an older census printed); +/// `closure.trusted_targets` is the dense side array only eligible arrows +/// with compiler-private bodies occupy. pub(crate) fn closure_registry_census() -> Vec { - use crate::gc::census::map_bytes; + use crate::gc::census::{map_bytes, vec_bytes}; let mut rows = Vec::new(); - CLOSURE_REST_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.rest_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_ARITY_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.arity_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_LENGTH_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.length_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_ARROW_FUNCTION_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.arrow_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_VERSIONED_LOOP_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.versioned_loop_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_STRICT_FUNCTION_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.strict_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_ASYNC_FUNCTION_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.async_registry", m.len(), map_bytes(&m))); - }); - CLOSURE_GENERATOR_FUNCTION_REGISTRY.with(|m| { + CLOSURE_BODY_REGISTRY.with(|m| { let m = m.borrow(); - rows.push(("closure.generator_registry", m.len(), map_bytes(&m))); + rows.push(("closure.body_registry", m.len(), map_bytes(&m))); }); - CLOSURE_ASYNC_GENERATOR_FUNCTION_REGISTRY.with(|m| { - let m = m.borrow(); - rows.push(("closure.async_generator_registry", m.len(), map_bytes(&m))); - }); - DISPATCH_CACHE.with(|m| { - let m = m.borrow(); - rows.push(("closure.dispatch_cache", m.len(), map_bytes(&m))); + TRUSTED_TARGETS.with(|v| { + let v = v.borrow(); + rows.push(("closure.trusted_targets", v.len(), vec_bytes(&v))); }); rows } + +#[cfg(test)] +mod body_record_tests { + use super::*; + + extern "C" fn rec_a(_: *const ClosureHeader) -> f64 { + 1.0 + } + extern "C" fn rec_b(_: *const ClosureHeader) -> f64 { + 2.0 + } + extern "C" fn rec_c(_: *const ClosureHeader) -> f64 { + 3.0 + } + extern "C" fn rec_d(_: *const ClosureHeader) -> f64 { + 4.0 + } + extern "C" fn rec_trusted(_: *const ClosureHeader) -> f64 { + 5.0 + } + extern "C" fn rec_versioned(_: *const ClosureHeader) -> f64 { + 6.0 + } + extern "C" fn rec_e(_: *const ClosureHeader) -> f64 { + 7.0 + } + extern "C" fn rec_f(_: *const ClosureHeader) -> f64 { + 8.0 + } + + /// The whole point of #9707: one record per body, small enough that the + /// map's bucket is a third of what the ten-map layout summed to. + #[test] + fn record_is_sixteen_bytes_and_the_bucket_is_twenty_four() { + assert_eq!(std::mem::size_of::(), 16); + assert_eq!(std::mem::size_of::<(usize, ClosureBodyRecord)>(), 24); + } + + /// Every attribute a body can carry survives being packed into the one + /// record alongside every other attribute, and each reader still + /// answers from its own field. + #[test] + fn all_attributes_coexist_in_one_record() { + let body = rec_a as *const u8; + js_register_closure_rest_and_arguments(body, 3); + js_register_closure_length(body, 2); + js_register_closure_arrow_function(body); + js_register_closure_strict_function(body); + js_register_closure_async_generator_function(body); + js_register_closure_generator_function(body); + + assert!(matches!( + lookup_closure_rest_full(body), + Some((3, RestDispatchKind::UserRestAndArguments)) + )); + assert_eq!(lookup_closure_rest(body), Some(3)); + assert_eq!( + lookup_closure_arity(body), + None, + "rest does not imply arity" + ); + assert_eq!(lookup_closure_length(body), Some(2)); + assert!(is_registered_arrow_function(body)); + assert!(is_registered_strict_function(body)); + assert!( + is_registered_async_function(body), + "async generators are async" + ); + assert!(is_registered_generator_function(body)); + assert!(is_registered_async_generator_function(body)); + let strategy = resolve_strategy(body); + assert!(strategy.is_arrow()); + assert!(matches!( + strategy.kind(), + DispatchKind::Rest(3, RestDispatchKind::UserRestAndArguments) + )); + + // Registering the same body's arity afterwards adds the attribute + // without disturbing the others, and rest still wins dispatch. + js_register_closure_arity(body, 4); + assert_eq!(lookup_closure_arity(body), Some(4)); + assert_eq!(lookup_closure_rest(body), Some(3)); + assert!(matches!( + resolve_strategy(body).kind(), + DispatchKind::Rest(3, RestDispatchKind::UserRestAndArguments) + )); + } + + /// A body nobody registered has no record and is a plain direct + /// callable; a body with only a non-dispatch attribute has a record and + /// is STILL a plain direct callable. + #[test] + fn unregistered_and_flag_only_bodies_dispatch_direct() { + let unregistered = rec_b as *const u8; + assert!(body_record(unregistered).is_none()); + let strategy = resolve_strategy(unregistered); + assert!(matches!(strategy.kind(), DispatchKind::Direct)); + assert!(!strategy.is_arrow()); + assert_eq!(lookup_closure_arity(unregistered), None); + assert_eq!(lookup_closure_length(unregistered), None); + assert!(!is_registered_strict_function(unregistered)); + + let strict_only = rec_c as *const u8; + js_register_closure_strict_function(strict_only); + assert!(body_record(strict_only).is_some()); + assert!(is_registered_strict_function(strict_only)); + assert!(matches!( + resolve_strategy(strict_only).kind(), + DispatchKind::Direct + )); + assert_eq!(lookup_closure_arity(strict_only), None); + assert_eq!(lookup_closure_rest(strict_only), None); + } + + /// The two compiler-private bodies live in the dense side array, are + /// reachable only through an ARROW record, and a registration against a + /// non-arrow must not create one. + #[test] + fn trusted_targets_hang_off_the_arrow_record() { + let arrow = rec_d as *const u8; + let plain = rec_e as *const u8; + js_register_closure_arity(plain, 1); + + // Not an arrow (yet): both registrations are refused. + js_register_closure_trusted_direct(arrow, rec_trusted as *const u8, 2, 0b10); + js_register_closure_versioned_loop_direct(arrow, rec_versioned as *const u8, 2, 0b10); + assert!(lookup_closure_trusted_direct(arrow).is_none()); + assert!(lookup_closure_versioned_loop_direct(arrow).is_none()); + assert!( + !is_registered_arrow_function(arrow), + "refusal must not mint an arrow" + ); + + js_register_closure_arrow_function(arrow); + js_register_closure_trusted_direct(arrow, rec_trusted as *const u8, 2, 0b10); + let direct = lookup_closure_trusted_direct(arrow).expect("direct target"); + assert_eq!(direct.func_ptr, rec_trusted as *const u8); + assert_eq!(direct.capture_count, 2); + assert_eq!(direct.boxed_capture_mask, 0b10); + assert!( + lookup_closure_versioned_loop_direct(arrow).is_none(), + "the versioned slot is independent of the direct slot" + ); + + js_register_closure_versioned_loop_direct(arrow, rec_versioned as *const u8, 2, 0b10); + let versioned = lookup_closure_versioned_loop_direct(arrow).expect("versioned target"); + assert_eq!(versioned.func_ptr, rec_versioned as *const u8); + // Both targets share ONE side-array slot, so the second registration + // reused the first's index instead of appending. + let slot = body_record(arrow).unwrap().targets; + assert!(slot != 0); + assert_eq!( + lookup_closure_trusted_direct(arrow).unwrap().func_ptr, + rec_trusted as *const u8, + "attaching the versioned body must not clobber the direct one" + ); + + // A registered non-arrow is still refused, and stays targetless. + js_register_closure_trusted_direct(plain, rec_trusted as *const u8, 0, 0); + assert!(lookup_closure_trusted_direct(plain).is_none()); + assert_eq!(body_record(plain).unwrap().targets, 0); + assert!(!is_registered_arrow_function(plain)); + } + + /// The census reports the unified table by its new name, with one entry + /// per registered body — not one per attribute. + #[test] + fn census_counts_bodies_not_attributes() { + let body = rec_f as *const u8; + js_register_closure_arity(body, 1); + js_register_closure_length(body, 1); + js_register_closure_strict_function(body); + let rows = closure_registry_census(); + let (name, entries, bytes) = rows + .iter() + .find(|(name, _, _)| *name == "closure.body_registry") + .copied() + .expect("body registry row"); + assert_eq!(name, "closure.body_registry"); + let distinct_bodies = CLOSURE_BODY_REGISTRY.with(|m| m.borrow().len()); + assert_eq!(entries, distinct_bodies); + assert!(entries >= 1); + assert!(bytes > 0); + assert!(rows + .iter() + .any(|(name, _, _)| *name == "closure.trusted_targets")); + } +} diff --git a/crates/perry-runtime/src/object/native_module/callable_exports.rs b/crates/perry-runtime/src/object/native_module/callable_exports.rs index e0bec11fb3..ea47d3ea81 100644 --- a/crates/perry-runtime/src/object/native_module/callable_exports.rs +++ b/crates/perry-runtime/src/object/native_module/callable_exports.rs @@ -1487,7 +1487,7 @@ thread_local! { /// Per-closure spec `.length` for built-in *prototype methods*. Those /// methods all share one no-op closure thunk /// (`global_this_builtin_noop_thunk`), so the func-ptr-keyed - /// `CLOSURE_ARITY_REGISTRY` can't give `Array.prototype.map.length === 1` + /// the closure body registry can't give `Array.prototype.map.length === 1` /// while `Array.prototype.slice.length === 2` — the last install would /// win for every method. Recording the length per *closure instance* here /// (keyed by the closure pointer, like the user-facing dynamic-prop table diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index ab632d8e0d..f50d35fac8 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -191,9 +191,15 @@ }, { "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_VERSIONED_LOOP_REGISTRY", + "name": "CLOSURE_BODY_REGISTRY", "verdict": "not_a_gc_pointer", - "why": "Maps a closure body `func_ptr` (a code pointer, never moved) to TrustedDirectTarget{func_ptr:*const u8, capture_count:u32, boxed_capture_mask:u64}. Both key and value are code pointers and plain integers; no field can hold a heap pointer, so the table cannot hold a GC root. Same shape as its sibling CLOSURE_ARROW_FUNCTION_REGISTRY in the same thread_local block." + "why": "#9707: the unified per-body record table. Keyed by a closure body `func_ptr` (a CODE address the collector neither moves nor traces); the value is `ClosureBodyRecord` = three plain integers (arity, rest arity, `.length`), a flags word, and a 1-based index into TRUSTED_TARGETS. No field can hold a heap pointer, so the table cannot hold a GC root. It replaced the ten `CLOSURE_*_REGISTRY` / `DISPATCH_CACHE` maps that carried the same verdict." + }, + { + "file": "crates/perry-runtime/src/closure/registry.rs", + "name": "TRUSTED_TARGETS", + "verdict": "not_a_gc_pointer", + "why": "#9707: dense side array of `TrustedTargets`, two `Option` per eligible arrow. TrustedDirectTarget{func_ptr:*const u8, capture_count:u32, boxed_capture_mask:u64} is a static code pointer plus plain integers \u2014 the same payload the deleted CLOSURE_ARROW_FUNCTION_REGISTRY / CLOSURE_VERSIONED_LOOP_REGISTRY held under this verdict. Nothing for the collector." }, { "file": "crates/perry-runtime/src/fs/filehandle.rs", @@ -2043,34 +2049,6 @@ "file": "crates/perry-runtime/src/closure/dynamic_props.rs", "name": "IN_FN_PROTO_FALLBACK" }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_ARROW_FUNCTION_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_ASYNC_FUNCTION_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_ASYNC_GENERATOR_FUNCTION_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_GENERATOR_FUNCTION_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_REST_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "CLOSURE_STRICT_FUNCTION_REGISTRY" - }, - { - "file": "crates/perry-runtime/src/closure/registry.rs", - "name": "DISPATCH_CACHE" - }, { "file": "crates/perry-runtime/src/closure/registry.rs", "name": "DISPATCH_RECENT" diff --git a/scripts/thread_local_cold_allowlist.json b/scripts/thread_local_cold_allowlist.json index 6b4cdcfa91..4744f170ac 100644 --- a/scripts/thread_local_cold_allowlist.json +++ b/scripts/thread_local_cold_allowlist.json @@ -79,7 +79,6 @@ "crates/perry-runtime/src/process/report.rs": 1, "crates/perry-runtime/src/proxy.rs": 1, "crates/perry-runtime/src/pty/reactor.rs": 1, - "crates/perry-runtime/src/readline_helpers.rs": 1, "crates/perry-runtime/src/static_plugins.rs": 1, "crates/perry-runtime/src/timer.rs": 1, "crates/perry-runtime/src/tty.rs": 1, diff --git a/test-files/test_gap_9718_initializer_self_binding.ts b/test-files/test_gap_9718_initializer_self_binding.ts new file mode 100644 index 0000000000..0caa1e3b20 --- /dev/null +++ b/test-files/test_gap_9718_initializer_self_binding.ts @@ -0,0 +1,81 @@ +// #9718: a closure created INSIDE a `let`/`const` declarator's initializer may +// reference the binding that declarator introduces. The closure body does not +// run until after initialization, so node resolves it normally — only the TDZ +// window between entering the declaration and completing it is off limits. +// +// Perry pre-registered such a binding only when the reference came from an +// EARLIER statement or an EARLIER declarator of the same declaration, never +// from the declarator's own initializer, so the reference fell through to the +// unresolved-global path and threw `ReferenceError: is not defined`. +// claude-code's `install` subcommand is exactly this shape: +// let { unmount: O } = await render(el({ onDone: (a, b) => { O(), done(a, b) } })) +// +// Every declarator form is covered: plain binding, object pattern, `{ key }` +// shorthand, array pattern, nested pattern, `let` and `const`, with and +// without `await`, plus a multi-declarator statement (the earlier-declarator +// case that already worked, to pin that this did not regress). + +type Handle = { u: () => string }; + +// Hands the callback to a later turn, then resolves — so the callback always +// runs after the declaration has completed, exactly as in the real bundle. +function render(cb: () => string): Promise { + return new Promise((resolve) => { + pending.push(cb); + resolve({ u: () => "handle" }); + }); +} +function renderSync(cb: () => string): Handle { + pending.push(cb); + return { u: () => "handle" }; +} +const pending: (() => string)[] = []; + +function report(label: string, run: () => string): void { + try { + console.log(label + ": " + run()); + } catch (e) { + console.log(label + ": THREW " + ((e as Error) && (e as Error).name) + " " + ((e as Error) && (e as Error).message)); + } +} + +async function main(): Promise { + // 1. plain binding, awaited initializer + const a = await render(() => a.u() + "/plain-await"); + // 2. object pattern, awaited initializer + const { u: b } = await render(() => b() + "/obj-await"); + // 3. object pattern, synchronous initializer + const { u: c } = renderSync(() => c() + "/obj-sync"); + // 4. array pattern, awaited initializer + const [d] = await render(() => d.u() + "/array-await").then((h) => [h] as [Handle]); + // 5. `let` rather than `const` + let { u: e } = await render(() => e() + "/let-obj-await"); + // 6. `{ key }` shorthand pattern + const { u } = await render(() => u() + "/shorthand-await"); + // 7. nested pattern + const { inner: { u: g } } = await Promise.resolve({ inner: { u: () => "handle" } as Handle }) + .then((v) => { pending.push(() => g() + "/nested-await"); return v; }); + // 8. multi-declarator: an EARLIER declarator's closure references a LATER one + // (already worked before #9718 — pinned so the reordered scan keeps it) + const h = () => i() + "/earlier-refs-later", + i = (): string => "handle"; + + report("plain-await", () => pending[0]!()); + report("obj-await", () => pending[1]!()); + report("obj-sync", () => pending[2]!()); + report("array-await", () => pending[3]!()); + report("let-obj-await", () => pending[4]!()); + report("shorthand-await", () => pending[5]!()); + report("nested-await", () => pending[6]!()); + report("earlier-refs-later", h); + + // The declaration is complete by the time these run, so the direct calls + // must agree with what the closures saw. + report("direct-plain", () => a.u()); + report("direct-obj", () => b()); + report("direct-array", () => d.u()); + report("direct-nested", () => g()); + console.log("count=" + pending.length + " c=" + c() + " e=" + e() + " u=" + u() + " i=" + i()); +} + +void main();