diff --git a/changelog.d/8648-derived-ctor-super-scope.md b/changelog.d/8648-derived-ctor-super-scope.md new file mode 100644 index 0000000000..28a389893c --- /dev/null +++ b/changelog.d/8648-derived-ctor-super-scope.md @@ -0,0 +1,15 @@ +Stopped emitting the runtime derived-`super` scope for constructors that cannot +use it. `js_derived_super_scope_push`/`pop` maintain a thread-local stack whose +only consumers are `js_derived_super_bind_current` and +`js_derived_this_check_current` — the lookups an arrow uses when it compiles as +its own LLVM function and therefore cannot name the constructor's `i1` alloca. +A derived constructor containing no closure paid that thread-local round trip +per construction for a cell nothing could look up. + +The gate is deliberately conservative: any closure in the body keeps the shared +form, without asking whether that closure mentions `this` or `super`. + +Partial fix for #8648: 27% of the regression on a two-class `new B(x, y)` loop +(1.89x -> 1.67x). The remaining cost is the constructor field store moving from +`js_put_value_set` to `js_typed_feedback_class_field_set_guard` + +`js_class_field_set_fallback`, which is not addressed here. diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index ed218fe3ee..70435b62e5 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -737,8 +737,22 @@ pub(super) fn compile_method( || class.native_extends.is_some() || class.extends_expr.is_some() { - crate::expr::this_super_call::push_shared_super_called_slot(&mut ctx); - ctx.shared_super_scope_active = true; + // #8648: only a closure in this body can need the RUNTIME cell. + // An arrow compiles as its own LLVM function and reaches the + // binding through `js_derived_super_bind_current` / + // `js_derived_this_check_current`, which read the thread-local + // stack this push maintains. With no closure here, nothing can + // perform that lookup, and `bind_derived_this_after_super` uses + // the local alloca directly -- so the push/pop pair is a + // thread-local round trip per construction for a cell no one + // reads. Measured: 1.89x on a two-class `new B(x, y)` loop, + // 3.14x on `shapes.ts`. + if crate::collectors::body_contains_closure(&method.body) { + crate::expr::this_super_call::push_shared_super_called_slot(&mut ctx); + ctx.shared_super_scope_active = true; + } else { + crate::expr::this_super_call::push_super_called_slot(&mut ctx); + } } // Stage field initializers around the parent body chain so leaf // fields can read state set by parent body (Refs #420): diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 9be7896227..0ae01715ea 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -82,7 +82,7 @@ pub(crate) use integer_locals::{ collect_flat_row_aliases, is_int32_producing_expr, static_index_window, }; pub(crate) use local_refs::{expr_contains_local_get, mark_all_candidate_refs_in_expr}; -pub(crate) use mutation::{body_contains_call, has_any_mutation}; +pub(crate) use mutation::{body_contains_call, body_contains_closure, has_any_mutation}; pub(crate) use number_by_construction::collect_number_by_construction_locals; pub(crate) use param_ranges::{collect_param_int_ranges, ParamIntRanges}; pub(crate) use pointer_locals::collect_pointer_typed_locals; diff --git a/crates/perry-codegen/src/collectors/mutation.rs b/crates/perry-codegen/src/collectors/mutation.rs index 11ba96c5b8..18bfdd1f4e 100644 --- a/crates/perry-codegen/src/collectors/mutation.rs +++ b/crates/perry-codegen/src/collectors/mutation.rs @@ -33,6 +33,43 @@ pub fn body_contains_call(stmts: &[perry_hir::Stmt]) -> bool { any_top_level_expr(stmts, &mut expr_contains_call) } +/// #8648: does this body create a closure anywhere? +/// +/// A derived constructor's `super()` binding normally lives in a plain `i1` +/// alloca that `bind_derived_this_after_super` reads directly. The RUNTIME +/// form (`js_derived_super_scope_push`/`pop`, a thread-local stack) exists for +/// exactly one reason, stated on `push_shared_super_called_slot`: an arrow +/// inside the constructor compiles as its OWN LLVM function, so it cannot name +/// the outer alloca and has to find the cell through +/// `js_derived_super_bind_current` / `js_derived_this_check_current`. +/// +/// A derived constructor with no closure in it therefore pays a thread-local +/// push and pop per construction for a cell nothing else can look up. #8630 +/// emitted the shared form for every derived constructor, which cost 1.89x on +/// a two-class `new B(x, y)` loop and 3.14x on `shapes.ts` (six `extends`, +/// 120k constructions). +/// +/// Deliberately conservative: ANY closure in the body selects the shared form, +/// without asking whether that closure actually mentions `this` or `super`. +/// The cheap answer is only taken when there is provably no separate LLVM +/// function to serve. +pub fn body_contains_closure(stmts: &[perry_hir::Stmt]) -> bool { + any_top_level_expr(stmts, &mut expr_contains_closure) +} + +fn expr_contains_closure(expr: &perry_hir::Expr) -> bool { + if matches!(expr, perry_hir::Expr::Closure { .. }) { + return true; + } + let mut found = false; + perry_hir::walker::walk_expr_children(expr, &mut |child: &perry_hir::Expr| { + if !found && expr_contains_closure(child) { + found = true; + } + }); + found +} + /// The statement skeleton both predicates walk. `pred` is applied to each /// top-level expression; it is responsible for its own subexpression /// recursion.