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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions changelog.d/8648-derived-ctor-super-scope.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 16 additions & 2 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion crates/perry-codegen/src/collectors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions crates/perry-codegen/src/collectors/mutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading