From 67d46da3093e13724db0ecbbfdbafa2e04326014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 29 Sep 2026 12:09:56 +0000 Subject: [PATCH 1/2] perf(transform): the static unroller keeps one source function literal as one function A loop body containing a closure, arrow, object-literal method or class expression is no longer unrolled. Every clone received a fresh FuncId, so objects built in a short counted loop carried one code pointer per copy and the method site over them latched megamorphic (method/varying/lit 3,075 instr/iter; 164 with this change, the same as varying/factory). The unroller exists to fold constant indices into kernels, which never create functions per iteration. --- ...-literal-one-function-in-unrolled-loops.md | 9 ++ crates/perry-transform/src/unroll/mod.rs | 124 +++++++++--------- crates/perry/tests/method_site.rs | 27 ++++ 3 files changed, 98 insertions(+), 62 deletions(-) create mode 100644 changelog.d/method-literal-one-function-in-unrolled-loops.md diff --git a/changelog.d/method-literal-one-function-in-unrolled-loops.md b/changelog.d/method-literal-one-function-in-unrolled-loops.md new file mode 100644 index 0000000000..9b4137d6b0 --- /dev/null +++ b/changelog.d/method-literal-one-function-in-unrolled-loops.md @@ -0,0 +1,9 @@ +### Performance + +The static loop unroller no longer clones a loop body that contains a function +literal (a closure, an arrow, an object-literal method or a class expression). +Each copy used to become its own compiled function, so objects built in a short +counted loop such as `for (let i = 0; i < 8; i++) objs.push({ m() { ... } })` +carried eight different code pointers for one source method, and a method call +site over them primed past its ways and went megamorphic. One source function +literal is now one function, and the site keeps a single entry. diff --git a/crates/perry-transform/src/unroll/mod.rs b/crates/perry-transform/src/unroll/mod.rs index 45cd711e42..e3d166712b 100644 --- a/crates/perry-transform/src/unroll/mod.rs +++ b/crates/perry-transform/src/unroll/mod.rs @@ -43,13 +43,16 @@ //! unrolled stmts. Out of scope for v1. //! - **`Stmt::Labeled`** — same reason; the label is loop-scoped and //! would alias with siblings post-unroll. -//! - **Closures capturing the IV** — each iteration needs to capture a -//! different value of `i`, but unrolling produces stmts at the caller -//! scope where `i` no longer exists. Substituting `LocalGet(i)` to -//! `Integer(N)` inside the closure body works only for closures that -//! capture-by-value at construction time AND aren't called after the -//! IV's loop-scope ends. Conservative: reject all closures referencing -//! the IV. +//! - **Function literals** (closures, arrows, object-literal and +//! prototype-literal methods, class expressions) — one source function +//! literal must stay ONE function. Cloning the body would give every copy +//! its own compiled body (a fresh FuncId, #456), so a receiver built in +//! the loop carries a different code pointer per iteration and every +//! identity-keyed memo on it (method sites, call-site feedback) sees N +//! functions where the program has one: `for (let i = 0; i < 8; i++) +//! objs.push({ m() {} })` latched its method site megamorphic. The +//! unroller exists to fold constant indices into kernels, which never +//! create functions per iteration. //! - **`LocalSet(i, ...)` or `Update { id: i }` inside body** — user is //! manually mutating the IV; unrolling would lose those writes. //! Allowed only in the for-loop's own `update` slot (by definition). @@ -502,8 +505,7 @@ fn try_unroll_for( // * codegen emits one `@perry_global_*__` per module-init // Stmt::Let with a referenced id, and N copies of the same // id cause LLVM duplicate-global errors (issue #456); and - // * each iteration's `() => captured` closure is supposed to - // bind a distinct value, which requires distinct capture ids. + // * a later read of a per-copy `let` must see its own copy. let mut out: Vec = Vec::with_capacity((trips as usize) * body.len()); for n in 0..trips { let value = lo + n; @@ -625,31 +627,9 @@ fn expr_is_unrollable(e: &Expr, iv_id: LocalId) -> bool { match e { Expr::LocalSet(id, _) if *id == iv_id => return false, Expr::Update { id, .. } if *id == iv_id => return false, - // Closures: reject any closure that even mentions the IV. A - // closure captured-by-value at construction would semantically - // freeze the IV's current value, but our HIR captures are by - // ID; substituting LocalGet(iv) → Integer(N) inside the - // closure body works only if the closure isn't called outside - // the IV's live range. The image_convolution kernel doesn't - // create closures inside its blur loops, so this restriction - // is free for the target workload. - Expr::Closure { body, captures, .. } => { - if captures.contains(&iv_id) { - return false; - } - // Defensive: walk the closure body to catch any direct - // `LocalGet(iv_id)` reference that wasn't materialized as a - // capture entry (shouldn't happen in well-formed HIR, but - // checking is cheap). Closure body's break/continue are - // always lexically scoped to a loop *inside* the closure - // (free `break` outside a loop is a JS syntax error), so we - // start at loop_depth=1 to suppress the always-true Break/ - // Continue rejection. - if !body.iter().all(|s| stmt_is_unrollable(s, iv_id, 1)) { - return false; - } - return true; - } + // A function literal is refused whatever it captures: the copies + // would each be a distinct function (see the module doc). + Expr::Closure { .. } | Expr::ClassExprFresh { .. } => return false, _ => {} } // Recurse into all sub-expressions. @@ -1038,12 +1018,12 @@ fn refresh_in_expr( mutable_captures, .. } => { - // Each cloned closure must get its own FuncId. Codegen keys - // compiled functions by FuncId, so two cloned `() => captured` - // closures sharing one FuncId would collapse into a single - // compiled function — every iteration's `fns[i]()` would then - // read the same global. Bumping FuncId per clone keeps each - // closure on its own compiled body. + // `expr_is_unrollable` refuses bodies that contain a function + // literal, so an unrolled body must not be assumed to reach + // here; the arm keeps the refresher total for any caller. If a + // closure is cloned, each clone needs its own FuncId: codegen + // keys compiled functions by FuncId, so two clones sharing one + // would collapse into a single compiled function. *func_id = *next_func_id; *next_func_id = next_func_id.saturating_add(1); @@ -1596,10 +1576,10 @@ mod tests { /// disturb. #[test] fn loop_local_let_still_refreshed_per_copy() { - // for (let i = 0; i < 3; i++) { let x = i; fns.push(() => x); } + // for (let i = 0; i < 3; i++) { let x = i; xs.push(x); } let i = 1u32; let x = 2u32; - let fns = 3u32; // declared outside the loop (not refreshed) + let xs = 3u32; // declared outside the loop (not refreshed) let body = vec![ Stmt::Let { id: x, @@ -1609,22 +1589,8 @@ mod tests { init: Some(ivar(i)), }, Stmt::Expr(Expr::ArrayPush { - array_id: fns, - value: Box::new(Expr::Closure { - func_id: 0, - params: vec![], - return_type: Type::Number, - body: vec![Stmt::Return(Some(Expr::LocalGet(x)))], - captures: vec![x], - mutable_captures: vec![], - captures_this: false, - captures_new_target: false, - enclosing_class: None, - is_arrow: false, - is_strict: false, - is_async: false, - is_generator: false, - }), + array_id: xs, + value: Box::new(Expr::LocalGet(x)), field_writeback: None, }), ]; @@ -1635,7 +1601,7 @@ mod tests { assert!(changed, "expected unroll to fire"); assert_eq!(stmts.len(), 6, "3 trips * 2 body stmts"); // Collect the `let x` id of each copy — they must all be DISTINCT - // (refreshed), and the closure in each copy must capture its own id. + // (refreshed), and the push in each copy must read its own id. let mut let_ids = Vec::new(); for pair in stmts.chunks(2) { let decl_id = match &pair[0] { @@ -1644,10 +1610,10 @@ mod tests { }; match &pair[1] { Stmt::Expr(Expr::ArrayPush { value, .. }) => match value.as_ref() { - Expr::Closure { captures, .. } => { - assert_eq!(captures, &vec![decl_id], "closure captures its copy's x"); + Expr::LocalGet(id) => { + assert_eq!(*id, decl_id, "the push reads its copy's x"); } - other => panic!("expected Closure, got {:?}", other), + other => panic!("expected LocalGet, got {:?}", other), }, other => panic!("expected ArrayPush, got {:?}", other), } @@ -1662,6 +1628,40 @@ mod tests { ); } + /// Sabotage: allow a closure that does not mention the IV -> this is red. + /// A function literal in the body would be cloned into one function per + /// copy, splitting the identity every method-site memo keys on. + #[test] + fn rejects_loop_with_a_function_literal_that_ignores_the_iv() { + // for (let i = 0; i < 3; i++) { fns.push(function () { return 1; }); } + let i = 1u32; + let fns = 2u32; + let body = vec![Stmt::Expr(Expr::ArrayPush { + array_id: fns, + value: Box::new(Expr::Closure { + func_id: 0, + params: vec![], + return_type: Type::Number, + body: vec![Stmt::Return(Some(integer(1)))], + captures: vec![], + mutable_captures: vec![], + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: false, + is_strict: false, + is_async: false, + is_generator: false, + }), + field_writeback: None, + })]; + let f = make_for(i, 0, 3, body, CompareOp::Lt); + assert!( + try_unroll(&f).is_none(), + "a function literal must not be cloned" + ); + } + #[test] fn unrolled_local_copies_keep_their_source_span() { let i = 1u32; diff --git a/crates/perry/tests/method_site.rs b/crates/perry/tests/method_site.rs index 76c13faf5a..a3da6c1826 100644 --- a/crates/perry/tests/method_site.rs +++ b/crates/perry/tests/method_site.rs @@ -453,3 +453,30 @@ console.log(s, t); (own={own} inherited={inherited} misses={misses})" ); } + +/// Sabotage: let the static unroller clone a loop body holding a function +/// literal -> the 8 copies are 8 code pointers and the site primes past its +/// ways and latches megamorphic. +#[test] +fn a_method_literal_built_in_a_short_counted_loop_is_one_function() { + let (stdout, own, inherited, misses) = run(r#"const N = process.argv.length > 99 ? 1 : 4000; +const objs: any[] = []; +function build() { for (let i = 0; i < 8; i++) objs.push({ a: i, b: 2, m() { return this.a; } }); } +build(); +function run(n: number): number { + let h = 0; + for (let k = 0; k < n; k++) h += objs[k & 7].m(); + return h; +} +console.log(run(N), objs[0].m === objs[7].m); +"#); + // One source literal is one function: node prints `false` for the + // identity (each evaluation is a fresh function object) but every object + // shares one body, so the site keeps one entry for all 8 receivers. + assert_eq!(stdout, "14000 false"); + assert!( + inherited == 0 && own <= 2 && misses <= 4, + "8 objects from one method literal must share one site entry \ + (own={own} inherited={inherited} misses={misses})" + ); +} From 8c88688f4ff972768b031e3e9fd1d0ddcc05ad5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 29 Sep 2026 19:24:44 +0000 Subject: [PATCH 2/2] test: node-parity tests for the receiver a method body sees test_gap_sloppy_this_bound_once covers OrdinaryCallBindThis for sloppy functions (one binding per activation, primitive wrappers, nullish to globalThis, strict functions untouched). test_gap_method_site_receiver covers object-literal and prototype methods called through a method site on every call route. Both match node on this base. --- changelog.d/method-receiver-parity-tests.md | 7 ++ test-files/test_gap_method_site_receiver.ts | 66 ++++++++++++++ test-files/test_gap_sloppy_this_bound_once.ts | 90 +++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 changelog.d/method-receiver-parity-tests.md create mode 100644 test-files/test_gap_method_site_receiver.ts create mode 100644 test-files/test_gap_sloppy_this_bound_once.ts diff --git a/changelog.d/method-receiver-parity-tests.md b/changelog.d/method-receiver-parity-tests.md new file mode 100644 index 0000000000..1d7528176a --- /dev/null +++ b/changelog.d/method-receiver-parity-tests.md @@ -0,0 +1,7 @@ +### Tests + +Two node-parity tests pin the receiver a method body sees: sloppy `this` is +bound once per activation (one wrapper for a primitive, `globalThis` for +nullish, objects unchanged), and object-literal and prototype methods called +through a method site get their receiver on every route (nested calls, arrows, +throws, `call`/`apply`, getters, constructors, generators and async methods). diff --git a/test-files/test_gap_method_site_receiver.ts b/test-files/test_gap_method_site_receiver.ts new file mode 100644 index 0000000000..607a54e5d8 --- /dev/null +++ b/test-files/test_gap_method_site_receiver.ts @@ -0,0 +1,66 @@ +// Object-literal and prototype methods called through a method site receive +// their receiver as `this`: nested method calls, arrows inheriting `this`, a +// throw through a method, extra/missing arguments, `arguments`, the generic +// path (call/apply/detached), an inherited method, a getter, a function +// expression called and constructed, generator/async methods, and a strict +// function given a primitive. Output must match node. +const N = process.argv.length > 99 ? 1 : 3000; +function plainFn(this: any) { return this === globalThis; } +function mk(k: number): any { + return { + k, + m(x: number) { return x + this.k; }, + outer(x: number) { const r = this.inner(x); return r + this.k; }, + inner(x: number) { return x * 2 + this.k; }, + thrower(x: number) { if (x < 0) throw new Error("neg" + this.k); return this.k; }, + wrap(x: number) { try { this.thrower(-x); } catch (e) { return this.k * 100; } return this.k; }, + arrow(x: number) { const f = () => this.k + x; return f(); }, + plain(x: number) { return plainFn() ? x + this.k : -1; }, + many(a: number, b: number, c: number, d: number) { return this.k + (b === undefined ? 0 : b) + (d === undefined ? 7 : d); }, + args(x: number) { return arguments.length + this.k; }, + }; +} +// Generator and async methods live on their own literal. +function mkg(k: number): any { + return { k, *gen() { yield this.k; yield this.k + 1; }, async am() { await null; return this.k * 3; } }; +} +const a = mk(1); +const b = mk(2); +let s = 0, t = 0, v = 0, w = 0, y = 0, z = 0, q = 0, caught = 0; +for (let i = 0; i < N; i++) { + const o = (i & 1) ? a : b; + s += o.m(i); + t += o.outer(i); + v += o.arrow(i); + w += o.plain(i); + y += o.wrap(i + 1); + z += o.many(i, 1); + q += o.args(i, i); + try { o.thrower(i % 7 === 0 ? -1 : i); } catch (e) { caught++; } +} +console.log(s, t, v, w, y, z, q, caught); +// The generic path (no site): call/apply/detached, through the public body. +const f = a.m; +console.log(a.m.call(b, 10), a.m.apply({ k: 40 }, [2]), f.call({ k: 7 }, 1), a.outer.call(b, 3)); +// An inherited method through a site. +const proto: any = { m(x: number) { return x * 3 + this.k; } }; +const kids: any[] = []; +for (let j = 0; j < 4; j++) { const c = Object.create(proto); c.k = j; kids.push(c); } +let r = 0; +for (let i = 0; i < N; i++) r += kids[i & 3].m(i); +console.log(r); +// A getter, and a function expression stored as a method, called and constructed. +const g: any = { k: 3, get dbl() { return this.k * 2; } }; +let gs = 0; +for (let i = 0; i < N; i++) gs += g.dbl; +const holder: any = { k: 9 }; +holder.F = function (this: any, x: number) { this.x = x; return this; }; +let fs = 0; +for (let i = 0; i < N; i++) fs += holder.F(i).k; +const made = new holder.F(4); +console.log(gs, fs, holder.x, made.x, made === holder, made instanceof holder.F); +// Generator and async methods keep their receiver. +const ga = mkg(1); +const gb = mkg(2); +console.log(JSON.stringify([...ga.gen()]), JSON.stringify([...gb.gen()])); +ga.am().then((x: number) => console.log("async", x)); diff --git a/test-files/test_gap_sloppy_this_bound_once.ts b/test-files/test_gap_sloppy_this_bound_once.ts new file mode 100644 index 0000000000..af536c2db1 --- /dev/null +++ b/test-files/test_gap_sloppy_this_bound_once.ts @@ -0,0 +1,90 @@ +// Sloppy-mode `this` is bound ONCE per activation (OrdinaryCallBindThis): +// undefined/null become globalThis, a primitive becomes ONE wrapper object, +// objects pass unchanged. Every `this` in one activation — and every arrow +// that inherits it — must name that same value: `this === this` holds for a +// primitive receiver. Covers function declarations, nested declarations, +// function expressions, object-literal methods, generators, async functions, +// class references, and a "use strict" function in a sloppy file. +function kind(this: any) { return typeof this; } +function same(this: any) { return this === this; } +function viaArrow(this: any) { const a = () => this; return a() === this && a() === a(); } +function tagOf(this: any) { return Object.prototype.toString.call(this); } +function isGlobal(this: any) { return this === globalThis; } +function keep(this: any) { return this; } +function twice(this: any) { const x = this; const y = this; return x === y; } +function mutate(this: any) { this.extra = 1; return this.extra; } + +const receivers: any[] = [1, 2.5, -0, NaN, true, false, "s", "", 10n]; +for (const r of receivers) { + const label = typeof r === "bigint" ? "bigint" : JSON.stringify(r); + console.log( + "decl", label, kind.call(r), same.call(r), viaArrow.call(r), tagOf.call(r), + twice.call(r), mutate.call(r), keep.call(r) === keep.call(r), + ); +} +console.log("nullish", isGlobal.call(undefined), isGlobal.call(null), isGlobal(), kind.call(undefined)); +const o = { a: 1 }; +console.log("object", keep.call(o) === o, same.call(o), kind.call(o)); +const fnRecv = function () { return 3; }; +console.log("function", keep.call(fnRecv) === fnRecv, kind.call(fnRecv)); + +// Nested declaration and a function expression. +function outer(this: any) { + function inner(this: any) { return [typeof this, this === this, this instanceof Number]; } + return inner.call(this); +} +console.log("nested", JSON.stringify(outer.call(7)), JSON.stringify(outer.call(o))); +const expr = function (this: any) { return [typeof this, this === this, this instanceof String]; }; +console.log("expr", JSON.stringify(expr.call("x")), JSON.stringify(expr.call(4))); + +// Object-literal method called with a primitive. +const lit: any = { m() { return [typeof this, this === this, this instanceof Boolean]; } }; +console.log("literal", JSON.stringify(lit.m.call(true)), JSON.stringify(lit.m.call(9))); + +// A method call ON a primitive: the receiver reaches the body unboxed, and +// the body binds it once. +(Number.prototype as any).kindOf = function (this: any) { return [typeof this, this === this, this instanceof Number]; }; +(String.prototype as any).kindOf = function (this: any) { const a = () => this; return [typeof this, this === a(), this.length]; }; +(Boolean.prototype as any).kindOf = function (this: any) { return [typeof this, this === this, this.valueOf()]; }; +const five: any = 5; +const str: any = "abc"; +const yes: any = true; +console.log("proto-method", JSON.stringify(five.kindOf()), JSON.stringify(str.kindOf()), JSON.stringify(yes.kindOf())); +let hot = 0; +for (let i = 0; i < 3000; i++) { const r = (i as any).kindOf(); if (r[0] === "object" && r[1] && r[2]) hot++; } +console.log("proto-method-hot", hot); + +// A detached function expression / method called plainly binds globalThis. +const detachedExpr = function (this: any) { return this === globalThis; }; +const detachedLit: any = { m() { return this === globalThis; } }; +const dm = detachedLit.m; +console.log("detached", detachedExpr(), dm(), typeof (function (this: any) { return this; })()); + +// A primitive receiver's wrapper is a fresh object per ACTIVATION. +console.log("per-activation", keep.call(5) === keep.call(5), keep.call(5) == keep.call(5)); +console.log("value", keep.call(5).valueOf(), keep.call("ab").length, keep.call(true).valueOf()); + +// Class reference receiver stays the class. +class C { static tag = "C"; } +function readTag(this: any) { return this === C ? this.tag : "boxed"; } +console.log("classref", readTag.call(C), keep.call(C) === C); + +// Generators and async functions bind their receiver the same way. +function* gen(this: any) { yield typeof this; yield this === this; const a = () => this; yield a() === this; } +console.log("generator", JSON.stringify([...gen.call(3)]), JSON.stringify([...gen.call("q")])); +async function af(this: any) { const before = this; await null; return [typeof this, this === before, this === this]; } +af.call(8).then((v) => console.log("async", JSON.stringify(v))); + +// A "use strict" function in a sloppy file sees the primitive itself. +function strictOne(this: any) { "use strict"; return [typeof this, this === this, this === 6]; } +function strictKeep(this: any) { "use strict"; return this; } +console.log("strict", JSON.stringify(strictOne.call(6)), strictKeep.call(undefined) === undefined, strictKeep.call("z") === "z"); + +// Many activations under allocation pressure keep their own wrappers. +let ok = 0; +for (let i = 0; i < 20000; i++) { + const w = keep.call(i); + const junk = { i, s: "x" + i }; + if (typeof w === "object" && w.valueOf() === i && same.call(i) && junk.i === i) ok++; +} +console.log("pressure", ok);