diff --git a/changelog.d/9026-box-capture-entry-cells.md b/changelog.d/9026-box-capture-entry-cells.md new file mode 100644 index 0000000000..6208ef1b00 --- /dev/null +++ b/changelog.d/9026-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 f62812ea52..d91c0fbae0 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1334,6 +1334,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 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 };