From 5c692c2f5dfa5b88de768987e554a27a55d492a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 10:43:29 +0200 Subject: [PATCH 1/3] codegen: resolve read-only boxed capture cells once per closure entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every read of a boxed capture in an ordinary closure body paid `js_box_get_bits`: an `is_registered_box_ptr` probe (thread-local cache + registry, 1.45% of the wolf-ecs entity cycle by itself) followed by one load. The #8644/#8705 trusted-clone machinery already retires this inside its private clones — validated at dispatch, cell pointers cached at entry, cells loaded per use — but only method callback parameters resolve those clones; a hoisted function declaration called directly (`function add(lB){...}`, capturing the ECS and its queries) runs its public body and pays the probe on every read of every iteration. This is the public body's variant of the same cache. Entry resolves each admitted capture slot through the new `js_box_capture_cell_ptr`: a registered pointer answers its own cell — boxes never move (the collector rewrites the value inside the cell) and cell memory is never returned to the allocator while a capturing closure is live (the #8208 argument the update lowering already relies on) — and an unregistered pointer answers a shared immutable `undefined` cell, so per-read behaviour is exactly `js_box_get_bits`'s (#4926: invalid box reads as `undefined`) in both cases. The cached pointers feed the existing `trusted_box_capture_ptrs` read arm: per-use cell load with the inline TDZ check, so writes through sibling closures stay visible. Admission is narrow by construction: only bindings the body never writes (the trusted `LocalSet`/`Update` arms store straight through the cached pointer, which must never reach the fallback cell), read at least twice or inside a loop (a cold-branch single read must not become an unconditional entry call), and never in async, generator-wrapper, or CPS async-step bodies (the repsel context gate's own exclusions). `PERRY_BOX_CAPTURE_ENTRY_CELLS=0` restores the per-read calls. Differential vs node (sibling-closure mutation visibility, hoisted function-decl consts, pre-initialization reads, shared written bindings): identical output, and the kill switch produces byte-identical results. Mac mini, 11 alternating pairs on the #9016+#9018 stack, both windows: add_remove −1.81%/−1.82%, entity_cycle −2.75%/−2.73% (11/11 except one 10/11) — slightly better than the hand-hoisted source ceiling (−1.65%/−2.72%) this was sized against before building. Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- changelog.d/0000-box-capture-entry-cells.md | 3 + crates/perry-codegen/src/codegen/closure.rs | 63 +++++++ .../src/codegen/closure_collect.rs | 170 ++++++++++++++++++ crates/perry-codegen/src/expr/mod.rs | 15 ++ .../src/runtime_decls/strings.rs | 2 +- crates/perry-runtime/src/box.rs | 31 ++++ 6 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 changelog.d/0000-box-capture-entry-cells.md diff --git a/changelog.d/0000-box-capture-entry-cells.md b/changelog.d/0000-box-capture-entry-cells.md new file mode 100644 index 0000000000..6208ef1b00 --- /dev/null +++ b/changelog.d/0000-box-capture-entry-cells.md @@ -0,0 +1,3 @@ +### Changed + +- A closure body that repeatedly reads a boxed capture it never assigns now resolves the box's cell address once at entry and loads the cell directly per read, instead of calling `js_box_get_bits` (a registry probe plus load) on every read. Mutations through other closures sharing the binding remain visible — only the never-moving cell address is cached, not the value. diff --git a/crates/perry-codegen/src/codegen/closure.rs b/crates/perry-codegen/src/codegen/closure.rs index 7361250449..1a2b549263 100644 --- a/crates/perry-codegen/src/codegen/closure.rs +++ b/crates/perry-codegen/src/codegen/closure.rs @@ -976,6 +976,69 @@ pub(super) fn compile_closure( } } trusted + } else if crate::expr::box_capture_entry_cells_enabled() + && !is_async + // Match the repsel context gate: generator wrappers and CPS async-step + // closures route locals through shared cells, and their entry blocks + // have re-entry semantics this cache has not been audited against. + && !cross_module.local_generator_funcs.contains(&func_id) + && !cross_module.async_step_closures.contains(&func_id) + { + // The PUBLIC body's variant of the cache above (#9016 follow-up). The + // dispatcher has validated nothing here, so each cached pointer is + // resolved through `js_box_capture_cell_ptr`, which answers the box's + // own (never-moving) cell for a registered pointer and a shared + // immutable `undefined` cell otherwise — per-read behaviour is then + // identical to `js_box_get_bits` in both cases. Admission is + // deliberately narrow: + // + // * only bindings this body NEVER writes — the `LocalSet`/`Update` + // trusted arms store straight through the cached pointer, which must + // never reach the shared fallback cell; + // * only bindings read more than once or read inside a loop — a + // single straight-line read pays the same either way, and a read on + // a never-taken branch must not become an unconditional entry call; + // * not in async bodies — their entry SSA values do not survive a + // suspension. + // + // The cell CONTENTS are still loaded per use, so a write through any + // other closure sharing the box stays visible; only the pointer — and + // the per-read registry probe `is_registered_box_ptr`, 1.45% of the + // wolf-ecs entity cycle — is hoisted to entry. + let mut cached = HashMap::new(); + let mut boxed_captures: Vec<_> = closure_captures + .iter() + .filter(|(id, _)| closure_boxed_vars.contains(id)) + .map(|(id, index)| (*id, *index)) + .collect(); + boxed_captures.sort_unstable_by_key(|(_, index)| *index); + if !boxed_captures.is_empty() { + let uses = super::closure_collect::collect_capture_use( + body, + boxed_captures.iter().map(|(id, _)| *id), + ); + boxed_captures.retain(|(id, _)| { + uses.get(id) + .is_some_and(|u| u.writes == 0 && (u.reads >= 2 || u.loop_reads >= 1)) + }); + } + if !boxed_captures.is_empty() { + let header_size = + crate::target_layout::closure_header_size_bytes(&cross_module.target_triple) + .to_string(); + let blk = lf.block_mut(0).expect("closure body has an entry block"); + let closure_ptr = blk.inttoptr(I64, "%this_closure"); + let captures_base = blk.gep(I8, &closure_ptr, &[(I64, &header_size)]); + for (id, index) in boxed_captures { + let index = index.to_string(); + let capture_slot = blk.gep(I64, &captures_base, &[(I64, &index)]); + let bits = blk.load(I64, &capture_slot); + let cell_bits = blk.call(I64, "js_box_capture_cell_ptr", &[(I64, &bits)]); + let ptr = blk.inttoptr(I64, &cell_bits); + cached.insert(id, crate::expr::TrustedBoxCapturePtr { bits, ptr }); + } + } + cached } else { HashMap::new() }; diff --git a/crates/perry-codegen/src/codegen/closure_collect.rs b/crates/perry-codegen/src/codegen/closure_collect.rs index 4a68d8bf9f..ec6a84fa4c 100644 --- a/crates/perry-codegen/src/codegen/closure_collect.rs +++ b/crates/perry-codegen/src/codegen/closure_collect.rs @@ -358,6 +358,176 @@ pub(super) fn count_body_nodes(body: &[perry_hir::Stmt]) -> usize { body.iter().map(count_stmt_nodes).sum() } +/// Per-binding use profile of one closure BODY, for the entry-cached box-cell +/// read optimization. Nested `Expr::Closure` bodies are deliberately not +/// entered: their capture reads lower in their own functions through their own +/// maps, and a write there mutates the shared CELL, which the per-use cell +/// load in this body observes anyway. +#[derive(Default, Clone, Copy)] +pub(crate) struct CaptureUse { + pub(crate) reads: u32, + pub(crate) writes: u32, + pub(crate) loop_reads: u32, +} + +fn scan_capture_use_expr( + expr: &perry_hir::Expr, + in_loop: bool, + uses: &mut std::collections::HashMap, +) { + match expr { + perry_hir::Expr::LocalGet(id) => { + if let Some(u) = uses.get_mut(id) { + u.reads += 1; + if in_loop { + u.loop_reads += 1; + } + } + } + perry_hir::Expr::LocalSet(id, value) => { + if let Some(u) = uses.get_mut(id) { + u.writes += 1; + } + scan_capture_use_expr(value, in_loop, uses); + } + perry_hir::Expr::Update { id, .. } => { + if let Some(u) = uses.get_mut(id) { + u.writes += 1; + } + } + perry_hir::Expr::Closure { .. } => {} + other => { + perry_hir::walker::walk_expr_children(other, &mut |child| { + scan_capture_use_expr(child, in_loop, uses); + }); + } + } +} + +fn scan_capture_use_stmt( + stmt: &perry_hir::Stmt, + in_loop: bool, + uses: &mut std::collections::HashMap, +) { + match stmt { + perry_hir::Stmt::Let { init, .. } => { + if let Some(init) = init { + scan_capture_use_expr(init, in_loop, uses); + } + } + perry_hir::Stmt::Expr(expr) | perry_hir::Stmt::Throw(expr) => { + scan_capture_use_expr(expr, in_loop, uses); + } + perry_hir::Stmt::Return(expr) => { + if let Some(expr) = expr { + scan_capture_use_expr(expr, in_loop, uses); + } + } + perry_hir::Stmt::If { + condition, + then_branch, + else_branch, + } => { + scan_capture_use_expr(condition, in_loop, uses); + for s in then_branch { + scan_capture_use_stmt(s, in_loop, uses); + } + if let Some(body) = else_branch { + for s in body { + scan_capture_use_stmt(s, in_loop, uses); + } + } + } + perry_hir::Stmt::While { condition, body } => { + scan_capture_use_expr(condition, true, uses); + for s in body { + scan_capture_use_stmt(s, true, uses); + } + } + perry_hir::Stmt::DoWhile { body, condition } => { + for s in body { + scan_capture_use_stmt(s, true, uses); + } + scan_capture_use_expr(condition, true, uses); + } + perry_hir::Stmt::For { + init, + condition, + update, + body, + } => { + if let Some(init) = init { + scan_capture_use_stmt(init, in_loop, uses); + } + if let Some(condition) = condition { + scan_capture_use_expr(condition, true, uses); + } + if let Some(update) = update { + scan_capture_use_expr(update, true, uses); + } + for s in body { + scan_capture_use_stmt(s, true, uses); + } + } + perry_hir::Stmt::Try { + body, + catch, + finally, + } => { + for s in body { + scan_capture_use_stmt(s, in_loop, uses); + } + if let Some(catch) = catch { + for s in &catch.body { + scan_capture_use_stmt(s, in_loop, uses); + } + } + if let Some(body) = finally { + for s in body { + scan_capture_use_stmt(s, in_loop, uses); + } + } + } + perry_hir::Stmt::Switch { + discriminant, + cases, + } => { + scan_capture_use_expr(discriminant, in_loop, uses); + for case in cases { + if let Some(test) = &case.test { + scan_capture_use_expr(test, in_loop, uses); + } + for s in &case.body { + scan_capture_use_stmt(s, in_loop, uses); + } + } + } + perry_hir::Stmt::Labeled { body, .. } => scan_capture_use_stmt(body, in_loop, uses), + perry_hir::Stmt::Break + | perry_hir::Stmt::Continue + | perry_hir::Stmt::LabeledBreak(_) + | perry_hir::Stmt::LabeledContinue(_) + | perry_hir::Stmt::PreallocateBoxes(_) + | perry_hir::Stmt::PreallocateTdzBoxes(_) + | perry_hir::Stmt::ReleaseBoxes(_) => {} + } +} + +/// Profile how `candidate_ids` are used in `body`. Only pre-seeded ids are +/// counted, so the walk stays O(body) with no per-node allocation. +pub(crate) fn collect_capture_use( + body: &[perry_hir::Stmt], + candidate_ids: impl Iterator, +) -> std::collections::HashMap { + let mut uses: std::collections::HashMap = candidate_ids + .map(|id| (id, CaptureUse::default())) + .collect(); + for stmt in body { + scan_capture_use_stmt(stmt, false, &mut uses); + } + uses +} + pub(crate) fn select_trusted_box_closures( closures: &[(perry_hir::types::FuncId, perry_hir::Expr)], direct_call_closures: &std::collections::HashSet, diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 831e1797fd..692f626b7c 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -4039,3 +4039,18 @@ pub(crate) fn lower_expr_value(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result Ok(None), } } + +/// `PERRY_BOX_CAPTURE_ENTRY_CELLS` gate (default on): resolve a read-only +/// boxed capture's cell pointer once at closure entry instead of calling +/// `js_box_get_bits` per read. `=0`/`off`/`false` restores the per-read calls +/// for A/B bisection. +pub(crate) fn box_capture_entry_cells_enabled() -> bool { + use std::sync::OnceLock; + static CACHED: OnceLock = OnceLock::new(); + *CACHED.get_or_init(|| { + !matches!( + std::env::var("PERRY_BOX_CAPTURE_ENTRY_CELLS").as_deref(), + Ok("0") | Ok("off") | Ok("false") + ) + }) +} diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 71566af703..c99a27dfbf 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1327,6 +1327,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // and a type-preserving step by 1n/1.0. Keeps `let i = 10n; i++` a BigInt. module.declare_function("js_to_numeric", DOUBLE, &[DOUBLE]); module.declare_function("js_numeric_step", DOUBLE, &[DOUBLE, I32]); + module.declare_function("js_box_capture_cell_ptr", I64, &[I64]); // Refs #486: dispatch path for `+` when neither operand has a static // type (string|number|bigint). Per JS spec, string concat takes // priority; otherwise BigInt or numeric add. Hono's @@ -1542,7 +1543,6 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_with_implicit_read", DOUBLE, &[DOUBLE, DOUBLE]); // Iterator-protocol result validation (for-of lazy loop). module.declare_function("js_iterator_result_validate", DOUBLE, &[DOUBLE]); - module.declare_function("js_for_of_next", DOUBLE, &[DOUBLE]); module.declare_function("js_global_get_or_throw_unresolved", DOUBLE, &[DOUBLE]); // Ambient `require` for compiled external / compilePackages modules (#5373): // bind a bare `require` to a createRequire-backed closure instead of throwing diff --git a/crates/perry-runtime/src/box.rs b/crates/perry-runtime/src/box.rs index 5657352685..ea8480f193 100644 --- a/crates/perry-runtime/src/box.rs +++ b/crates/perry-runtime/src/box.rs @@ -1036,6 +1036,37 @@ pub extern "C" fn js_box_get_bits(ptr: *mut Box) -> i64 { /// `ptr` must be non-null and must name a live Perry box cell whose capture /// edge remains live for the duration of the call. The exact-arrow resolver /// establishes this before selecting the only generated callers. +/// The immutable cell [`js_box_capture_cell_ptr`] substitutes for an +/// unregistered capture slot. It permanently holds `TAG_UNDEFINED`, mirroring +/// `js_box_get_bits`'s answer for an invalid box pointer (#4926: a +/// read-before-initialization boxed variable reads as `undefined`), and it is +/// NEVER written: the codegen that caches cell pointers admits only bindings +/// its body never assigns, so no store can reach a substituted cell. +static BOX_CAPTURE_UNDEFINED_CELL: Box = Box { + value: crate::value::TAG_UNDEFINED, +}; + +/// Resolve a closure capture slot's raw bits to a readable box CELL address, +/// once per closure invocation. +/// +/// A registered box answers its own address: the cell contents may change (the +/// binding is mutable through other closures), but the CELL never moves — the +/// collector rewrites the value inside the box, never the box address, and box +/// memory is never returned to the allocator while a capturing closure is live +/// (the #8208 argument `expr/literals_vars.rs` already relies on across +/// `js_to_numeric`). An unregistered pointer answers the shared immutable +/// `undefined` cell above, so every subsequent read through the cached address +/// yields exactly what `js_box_get_bits` would have returned per-read. +#[no_mangle] +pub extern "C" fn js_box_capture_cell_ptr(bits: i64) -> i64 { + let ptr = bits as usize as *mut Box; + if is_registered_box_ptr(ptr) { + bits + } else { + &BOX_CAPTURE_UNDEFINED_CELL as *const Box as i64 + } +} + #[no_mangle] pub unsafe extern "C" fn js_box_get_bits_trusted(ptr: *mut Box) -> i64 { let bits = unsafe { (*ptr).value }; From 7e3f67372e3927e79e59dbbdc98db52080143b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 10:58:15 +0200 Subject: [PATCH 2/3] fix: restore #9017's runtime declares clobbered by a cross-branch file checkout Claude-Session: https://claude.ai/code/session_019WVcWKmYsUBnnFB7nBgbBJ --- crates/perry-codegen/src/runtime_decls/strings.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index c99a27dfbf..d91c0fbae0 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -709,6 +709,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // `js_string_char_at` / `js_array_get_f64` / `js_object_get_field_by_name_f64` // based on the receiver's NaN-box tag at runtime. Used by IndexGet's // fallback path when codegen can't statically prove the receiver type. + // By-value computed read: key arrives NaN-boxed so an SSO key can be + // answered from the read stub without being materialised to the heap. + module.declare_function( + "js_typed_feedback_object_get_field_by_value_f64", + DOUBLE, + &[I64, I64, DOUBLE], + ); module.declare_function("js_dyn_index_get", DOUBLE, &[DOUBLE, DOUBLE]); // #8655: guarded packed-array / dense Array-subclass read before the // fully generic dynamic dispatcher. Used by unknown-receiver loop reads. @@ -1543,6 +1550,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_with_implicit_read", DOUBLE, &[DOUBLE, DOUBLE]); // Iterator-protocol result validation (for-of lazy loop). module.declare_function("js_iterator_result_validate", DOUBLE, &[DOUBLE]); + module.declare_function("js_for_of_next", DOUBLE, &[DOUBLE]); module.declare_function("js_global_get_or_throw_unresolved", DOUBLE, &[DOUBLE]); // Ambient `require` for compiled external / compilePackages modules (#5373): // bind a bare `require` to a createRequire-backed closure instead of throwing From f17647a72ff2142d420e73bbc14245159b718bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 11:41:15 +0200 Subject: [PATCH 3/3] docs: renumber the changeset fragment 0000 -> 9026 The `0000-` placeholder is never a legal fragment number; #9010's gate rejects it outright rather than letting it misattribute the change at release time. --- ...box-capture-entry-cells.md => 9026-box-capture-entry-cells.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{0000-box-capture-entry-cells.md => 9026-box-capture-entry-cells.md} (100%) diff --git a/changelog.d/0000-box-capture-entry-cells.md b/changelog.d/9026-box-capture-entry-cells.md similarity index 100% rename from changelog.d/0000-box-capture-entry-cells.md rename to changelog.d/9026-box-capture-entry-cells.md