Skip to content

Commit a16bec9

Browse files
author
Ralph Küpper
committed
perf(codegen): elide length-only arguments objects
1 parent db821b5 commit a16bec9

8 files changed

Lines changed: 208 additions & 3 deletions

File tree

crates/perry-codegen/src/codegen/arguments.rs

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashSet;
22

3-
use perry_hir::Param;
3+
use perry_hir::{Expr, Param, Stmt};
44

55
use crate::block::LlBlock;
66
use crate::expr::{nanbox_pointer_inline, FnCtx};
@@ -40,11 +40,21 @@ pub(crate) fn store_param_slot(
4040
pub(crate) fn materialize_arguments_object(
4141
ctx: &mut FnCtx<'_>,
4242
params: &[Param],
43+
body: Option<&[Stmt]>,
4344
callee: ArgumentsCallee<'_>,
4445
) {
4546
let Some(synth_param) = params.iter().find(|p| p.arguments_object.is_some()) else {
4647
return;
4748
};
49+
// Call lowering has already bundled every supplied argument into the
50+
// synthesized slot as a marked Array. When the only observable operation
51+
// is `arguments.length`, that bundle has exactly the required value and a
52+
// full ECMAScript Arguments object would only add allocation, mapped-index
53+
// setup, and GC pressure. Keep the existing conservative materialization
54+
// path for every other use (including callers that cannot provide a body).
55+
if body.is_some_and(|body| arguments_used_only_for_length(body, synth_param.id)) {
56+
return;
57+
}
4858
let Some(meta) = synth_param.arguments_object.as_ref() else {
4959
return;
5060
};
@@ -102,6 +112,37 @@ pub(crate) fn materialize_arguments_object(
102112
ctx.block().store(DOUBLE, &boxed_args, &arguments_slot);
103113
}
104114

115+
/// Prove that replacing the synthesized Arguments object with its raw argument
116+
/// bundle cannot be observed. The proof is deliberately fail-closed: HIR's
117+
/// canonical local-reference collector counts every use of the synthetic local,
118+
/// including specialized local-bearing expressions such as `ArrayPop(id)`.
119+
/// Every one of those uses must correspond to an exact `arguments.length` read
120+
/// found by the generic expression traversal.
121+
fn arguments_used_only_for_length(body: &[Stmt], arguments_id: u32) -> bool {
122+
let mut refs = Vec::new();
123+
let mut visited_closures = HashSet::new();
124+
for stmt in body {
125+
perry_hir::collect_local_refs_stmt(stmt, &mut refs, &mut visited_closures);
126+
}
127+
128+
let total_uses = refs.iter().filter(|id| **id == arguments_id).count();
129+
let mut length_reads = 0usize;
130+
crate::collectors::for_each_expr_in_stmts(body, &mut |expr| {
131+
if matches!(
132+
expr,
133+
Expr::PropertyGet {
134+
object,
135+
property,
136+
..
137+
} if property == "length"
138+
&& matches!(object.as_ref(), Expr::LocalGet(id) if *id == arguments_id)
139+
) {
140+
length_reads += 1;
141+
}
142+
});
143+
length_reads > 0 && total_uses == length_reads
144+
}
145+
105146
fn mapped_arguments_params(params: &[Param]) -> Vec<(u32, u32)> {
106147
params
107148
.iter()
@@ -110,6 +151,63 @@ fn mapped_arguments_params(params: &[Param]) -> Vec<(u32, u32)> {
110151
.collect()
111152
}
112153

154+
#[cfg(test)]
155+
mod length_only_tests {
156+
use super::arguments_used_only_for_length;
157+
use perry_hir::{Expr, Stmt};
158+
159+
const ARGUMENTS: u32 = 17;
160+
161+
fn length() -> Expr {
162+
Expr::PropertyGet {
163+
object: Box::new(Expr::LocalGet(ARGUMENTS)),
164+
property: "length".to_string(),
165+
byte_offset: 0,
166+
}
167+
}
168+
169+
#[test]
170+
fn accepts_exact_length_reads_at_arbitrary_depth() {
171+
let body = vec![Stmt::Return(Some(Expr::Binary {
172+
op: perry_hir::BinaryOp::Add,
173+
left: Box::new(Expr::Integer(1)),
174+
right: Box::new(length()),
175+
}))];
176+
assert!(arguments_used_only_for_length(&body, ARGUMENTS));
177+
}
178+
179+
#[test]
180+
fn rejects_identity_index_and_mixed_uses() {
181+
assert!(!arguments_used_only_for_length(
182+
&[Stmt::Return(Some(Expr::LocalGet(ARGUMENTS)))],
183+
ARGUMENTS
184+
));
185+
assert!(!arguments_used_only_for_length(
186+
&[Stmt::Return(Some(Expr::IndexGet {
187+
object: Box::new(Expr::LocalGet(ARGUMENTS)),
188+
index: Box::new(Expr::Integer(0)),
189+
}))],
190+
ARGUMENTS
191+
));
192+
assert!(!arguments_used_only_for_length(
193+
&[
194+
Stmt::Expr(length()),
195+
Stmt::Return(Some(Expr::LocalGet(ARGUMENTS))),
196+
],
197+
ARGUMENTS
198+
));
199+
}
200+
201+
#[test]
202+
fn rejects_specialized_local_bearing_operations() {
203+
let body = vec![
204+
Stmt::Expr(length()),
205+
Stmt::Return(Some(Expr::ArrayPop(ARGUMENTS))),
206+
];
207+
assert!(!arguments_used_only_for_length(&body, ARGUMENTS));
208+
}
209+
}
210+
113211
/// Does `property`, resolved against `class_name`'s ancestry, declare a USER
114212
/// `...rest` parameter — as opposed to (or in addition to) the trailing
115213
/// `arguments` slot #677 synthesizes?

crates/perry-codegen/src/codegen/closure.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,7 @@ pub(super) fn compile_closure(
12011201
super::arguments::materialize_arguments_object(
12021202
&mut ctx,
12031203
params,
1204+
Some(body),
12041205
super::arguments::ArgumentsCallee::CurrentClosure,
12051206
);
12061207

crates/perry-codegen/src/codegen/function.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,6 +1252,7 @@ pub(super) fn compile_function(
12521252
super::arguments::materialize_arguments_object(
12531253
&mut ctx,
12541254
&f.params,
1255+
Some(&f.body),
12551256
super::arguments::ArgumentsCallee::FunctionWrapper(&wrapper_name),
12561257
);
12571258

crates/perry-codegen/src/codegen/method.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ pub(super) fn compile_method(
630630
super::arguments::materialize_arguments_object(
631631
&mut ctx,
632632
&method.params,
633+
Some(&method.body),
633634
super::arguments::ArgumentsCallee::Undefined,
634635
);
635636

@@ -1856,6 +1857,7 @@ pub(super) fn compile_static_method(
18561857
super::arguments::materialize_arguments_object(
18571858
&mut ctx,
18581859
&f.params,
1860+
Some(&f.body),
18591861
super::arguments::ArgumentsCallee::Undefined,
18601862
);
18611863
if f.is_async {

crates/perry-codegen/src/collectors/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ pub(crate) use refs::{
107107
};
108108
pub(crate) use safepoint_sites::count_safepoint_sites;
109109
pub(crate) use scalar_method_dispatch::{
110-
collect_module_dispatch_facts, mark_unstable_scalar_method_receivers, ModuleDispatchFacts,
110+
collect_module_dispatch_facts, for_each_expr_in_stmts, mark_unstable_scalar_method_receivers,
111+
ModuleDispatchFacts,
111112
};
112113
pub(crate) use scalar_methods::simple_scalar_method_summary;
113114
pub(crate) use shadow_slots::{

crates/perry-codegen/src/collectors/scalar_method_dispatch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ pub(super) fn for_each_expr(expr: &Expr, f: &mut dyn FnMut(&Expr)) {
559559
}
560560
}
561561

562-
pub(super) fn for_each_expr_in_stmts(stmts: &[Stmt], f: &mut dyn FnMut(&Expr)) {
562+
pub(crate) fn for_each_expr_in_stmts(stmts: &[Stmt], f: &mut dyn FnMut(&Expr)) {
563563
for stmt in stmts {
564564
for_each_expr_in_stmt(stmt, f);
565565
}

crates/perry-codegen/src/lower_call/new_ctor_args.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ pub(crate) fn bind_inline_constructor_params(
106106
crate::codegen::arguments::materialize_arguments_object(
107107
ctx,
108108
params,
109+
None,
109110
crate::codegen::arguments::ArgumentsCallee::Undefined,
110111
);
111112

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//! Regression coverage for eliding full Arguments-object materialization when
2+
//! a function observes only `arguments.length`.
3+
//!
4+
//! Call lowering already supplies a marked raw-argument Array containing every
5+
//! actual argument. Its length is therefore exact for ordinary functions,
6+
//! methods, function expressions, and captured outer `arguments`. Any other
7+
//! observation must retain the full ECMAScript Arguments object.
8+
9+
use std::path::PathBuf;
10+
use std::process::Command;
11+
12+
fn perry_bin() -> PathBuf {
13+
PathBuf::from(env!("CARGO_BIN_EXE_perry"))
14+
}
15+
16+
fn compile_and_run(source: &str) -> String {
17+
let dir = tempfile::tempdir().expect("tempdir");
18+
let entry = dir.path().join("main.ts");
19+
let output = dir.path().join("main_bin");
20+
std::fs::write(&entry, source).expect("write entry");
21+
22+
let compile = Command::new(perry_bin())
23+
.current_dir(dir.path())
24+
.arg("compile")
25+
.arg(&entry)
26+
.arg("-o")
27+
.arg(&output)
28+
.arg("--no-cache")
29+
.output()
30+
.expect("run perry compile");
31+
assert!(
32+
compile.status.success(),
33+
"perry compile failed\nstdout:\n{}\nstderr:\n{}",
34+
String::from_utf8_lossy(&compile.stdout),
35+
String::from_utf8_lossy(&compile.stderr)
36+
);
37+
38+
let run = Command::new(&output)
39+
.current_dir(dir.path())
40+
.output()
41+
.expect("run compiled binary");
42+
assert!(
43+
run.status.success(),
44+
"compiled binary failed (exit {:?})\nstdout:\n{}\nstderr:\n{}",
45+
run.status.code(),
46+
String::from_utf8_lossy(&run.stdout),
47+
String::from_utf8_lossy(&run.stderr)
48+
);
49+
String::from_utf8_lossy(&run.stdout).into_owned()
50+
}
51+
52+
#[test]
53+
fn length_only_works_across_callable_kinds_and_arities() {
54+
let stdout = compile_and_run(
55+
r#"
56+
function declared(a?: any, b?: any) { return arguments.length; }
57+
const expression = function (a?: any) { return arguments.length; };
58+
class Probe {
59+
method(a?: any) { return arguments.length; }
60+
static method(a?: any) { return arguments.length; }
61+
}
62+
function captured(a?: any) {
63+
return () => arguments.length;
64+
}
65+
66+
const probe = new Probe();
67+
console.log(declared(), declared(1), declared(1, 2, 3));
68+
console.log(expression(), expression(1, 2));
69+
console.log(probe.method(), probe.method(1, 2));
70+
console.log(Probe.method(), Probe.method(1, 2, 3));
71+
console.log(captured(1, 2, 3, 4)());
72+
"#,
73+
);
74+
assert_eq!(stdout, "0 1 3\n0 2\n0 2\n0 3\n4\n");
75+
}
76+
77+
#[test]
78+
fn observable_and_mixed_arguments_uses_still_materialize() {
79+
let stdout = compile_and_run(
80+
r#"
81+
import { types } from "node:util";
82+
83+
function observable(a?: any) {
84+
return types.isArgumentsObject(arguments);
85+
}
86+
function mixed(a?: any) {
87+
return arguments.length + ":" + arguments[0] + ":" +
88+
types.isArgumentsObject(arguments);
89+
}
90+
function writesLength() {
91+
arguments.length = 7;
92+
return arguments.length;
93+
}
94+
95+
console.log(observable(1));
96+
console.log(mixed("first", "second"));
97+
console.log(writesLength(1, 2));
98+
"#,
99+
);
100+
assert_eq!(stdout, "true\n2:first:true\n7\n");
101+
}

0 commit comments

Comments
 (0)