Skip to content

Commit c6aab50

Browse files
author
Ralph Küpper
committed
perf(codegen): stamp a pointer-free shape's typed layout into the allocation header
Four allocation benchmarks sat within a 6% band at 2.50-2.65x node, which is the signature of one shared per-allocation cost rather than four problems. Profiling the band (200M-allocation variants, two samples each, agreeing within 1.5pp) found it: `js_gc_declare_typed_shape_layout` was 30% of `churn_alloc` and `push_cls`, and it spent that re-deriving per OBJECT a fact that is a property of the SHAPE. #7510's memo had already reduced the map round-trip to a direct-mapped probe; what remained was the probe itself, a type-table lookup, a field-count compare, and the cross-crate call. For a shape whose pointer mask is statically EMPTY the answer is a constant: `GC_LAYOUT_POINTER_FREE | GC_OBJ_TYPED_LAYOUT_INTACT`. The inline-bump `new` path already writes a packed `GcHeader` constant that carries the state half, so the intact bit is folded into the same store and the call disappears. What survives is the one half that depends on the recycled ADDRESS rather than the shape - clearing a previous tenant's per-object record - now a one-argument `js_gc_forget_object_layout` behind a `PERRY_PER_OBJECT_LAYOUTS_ANY` test whose `0` state proves every thread's per-object tables empty. Two smaller levers in the same band: * `js_ctor_return_override` was called per construction to answer a question that is `undefined` for every constructor without an explicit `return` - 8% of `churn_alloc`. `JSValue::is_undefined` is `bits == TAG_UNDEFINED`, so one 64-bit compare decides it inline and the runtime call stays on the cold arm, where derived-constructor TypeErrors and object returns still need it. * A `new` in a function the hot-loop-callee pre-pass admitted is a `new` in a loop one frame out, so it takes the inline bump too. `cycles.ts`'s `makeCycle` is the shape: 5 statements, hence `alwaysinline` and hence never `inlinehint`, so the existing gate read the one flag it could not have. Measured on the quiet M1 mini, best-of-5, outputs byte-identical to node with exit 0 verified for all 27 programs: | bench | before | after | |---|--:|--:| | churn | 0.4217 | 0.2900 | | churn_alloc | 0.3720 | 0.2409 | | push_cls | 0.3665 | 0.2368 | `churn_alloc` goes 18.6 -> 12.0 ns per allocation (node is 7.1 on the same shape). `gc-handoff/bench/alloc_declare_{pf,ptr}.ts` isolate it: identical programs differing only in whether the second field's declared type makes the pointer mask non-empty. Soundness: the collector's view is bit-identical. `heap_payload_slot_selection` skips a `GC_LAYOUT_POINTER_FREE` payload without consulting any map, and the pre-existing path also reached `POINTER_FREE` for an empty pointer mask. A later pointer store still downgrades - with no descriptor to classify against, `layout_note_slot` falls through to its generic pointer-mask branch, which mints a per-object mask and flips the state to `SIDE_MASK`, needing no descriptor at all. A pointer-BEARING shape keeps the full runtime declare, because its `SIDE_MASK` state means the tracer reads a mask and that call is what installs it.
1 parent 82f0e96 commit c6aab50

13 files changed

Lines changed: 770 additions & 35 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,12 @@ pub(super) fn compile_function(
426426
// functions (the hint would be redundant) and async/generator forms.
427427
// Try-containing functions are ordinary inline candidates since #7302
428428
// (invoke-EH removed the setjmp-era noinline requirement).
429+
// #7815: record the raw admission, before the `inline_hint` window narrows
430+
// it. `lower_call/new_alloc.rs` uses this to decide the inline-bump
431+
// allocation, which wants "is this code hot" and not "may LLVM's inline
432+
// threshold move" — an `alwaysinline` callee is excluded from the latter
433+
// and is the hottest possible case for the former.
434+
lf.hot_loop_callee = cross_module.hot_loop_callees.contains(&f.id);
429435
if !lf.force_inline
430436
&& inline_hot_small_enabled()
431437
&& (INLINE_HOT_SMALL_MIN..=inline_hot_small_size_cap()).contains(&f.body.len())

crates/perry-codegen/src/function.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@ pub struct LlFunction {
3636
/// inline-hot-small heuristic in `codegen/function.rs`. `alwaysinline`
3737
/// already implies the hint, so the two are never emitted together.
3838
pub inline_hint: bool,
39+
/// #7815: `collectors::collect_hot_loop_callees` admitted this function —
40+
/// it has at least one direct call site inside a LOOP and at most
41+
/// `inline_hot_small_max_call_sites` call sites in the whole module.
42+
///
43+
/// Not the same question as [`Self::inline_hint`], which is that set
44+
/// INTERSECTED with a body-length window and with "not already
45+
/// `alwaysinline`". A ≤8-statement function is `alwaysinline` and therefore
46+
/// never hinted, yet it is exactly the shape whose body ends up executing
47+
/// once per loop iteration — `cycles.ts`'s `makeCycle`. Sites that want
48+
/// "is this code hot?" rather than "should LLVM's threshold move?" read
49+
/// this.
50+
pub hot_loop_callee: bool,
3951
/// Invoke-EH (#7302): this function contains landing pads (Itanium) or
4052
/// funclet pads (SEH), so its `define` line must carry
4153
/// `personality ptr @<name>` — `perry_eh_personality` on Mach-O/ELF,
@@ -215,6 +227,7 @@ impl LlFunction {
215227
linkage: String::new(),
216228
force_inline: false,
217229
inline_hint: false,
230+
hot_loop_callee: false,
218231
personality: None,
219232
blocks: Vec::new(),
220233
block_counter: 0,

crates/perry-codegen/src/gc_call_effects.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect {
8383
| "js_gc_note_slot_layout_aware"
8484
| "js_gc_init_typed_shape_layout"
8585
| "js_gc_declare_typed_shape_layout"
86+
// #7815: `layout_forget_object` behind a null check — two thread-local
87+
// side-table removals, no allocation and no re-entry.
88+
| "js_gc_forget_object_layout"
8689
// `typed_feedback.rs`: counters/registries only. This intentionally
8790
// does not include feedback wrappers that perform the actual object
8891
// get/set operation.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ mod scalar_method;
7171
/// module header for why the default build cannot fault on them.
7272
#[cfg(test)]
7373
mod timer_rooting_tests;
74+
#[cfg(test)]
75+
mod typed_shape_bake_tests;
7476
/// #7510: which of the two typed-shape layout entry points a `new` site emits,
7577
/// and where. Split out of `new.rs` to keep it under the 2000-line cap.
7678
mod typed_shape_init;

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

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,8 @@ fn lower_new_impl_inner<'a>(
500500

501501
// #7615 slice 8: the field-count computation and the three-arm instance
502502
// allocation moved verbatim to `new_alloc.rs` (see its header for why).
503-
let obj_handle = super::new_alloc::emit_instance_alloc(ctx, class_name, class);
503+
let alloc = super::new_alloc::emit_instance_alloc(ctx, class_name, class);
504+
let obj_handle = alloc.handle;
504505
// #7154: root the instance for the duration of the constructor body.
505506
//
506507
// Until now the instance existed ONLY as an SSA register while that body
@@ -532,7 +533,7 @@ fn lower_new_impl_inner<'a>(
532533
//
533534
// Before the instance root's push, so the handle this names is the one the
534535
// allocator returned: nothing between here and there can collect.
535-
emit_typed_shape_layout_declare(ctx, class_name, &obj_handle);
536+
emit_typed_shape_layout_declare(ctx, class_name, &obj_handle, alloc.typed_layout_baked);
536537
let instance = {
537538
let protected = construction_runs_user_code(ctx, class_name);
538539
Instance {
@@ -669,16 +670,8 @@ fn lower_new_impl_inner<'a>(
669670
|| class.extends_name.is_some()
670671
|| class.native_extends.is_some()
671672
|| class.extends_expr.is_some();
672-
let is_derived_lit = if is_derived { "1" } else { "0" };
673-
let final_box = ctx.block().call(
674-
DOUBLE,
675-
"js_ctor_return_override",
676-
&[
677-
(DOUBLE, &obj_box),
678-
(DOUBLE, &ctor_ret),
679-
(crate::types::I32, is_derived_lit),
680-
],
681-
);
673+
let final_box =
674+
super::new_helpers::emit_ctor_return_override(ctx, &obj_box, &ctor_ret, is_derived);
682675
return Ok(final_box);
683676
}
684677
if let Some(save) = &saved_new_target {
@@ -1560,16 +1553,7 @@ fn lower_new_impl_inner<'a>(
15601553
}
15611554
ctx.current_block = after_idx;
15621555
let raw = ctx.block().load(DOUBLE, &ret.result_slot);
1563-
let is_derived = if ret.is_derived { "1" } else { "0" };
1564-
ctx.block().call(
1565-
DOUBLE,
1566-
"js_ctor_return_override",
1567-
&[
1568-
(DOUBLE, &obj_box),
1569-
(DOUBLE, &raw),
1570-
(crate::types::I32, is_derived),
1571-
],
1572-
)
1556+
super::new_helpers::emit_ctor_return_override(ctx, &obj_box, &raw, ret.is_derived)
15731557
} else {
15741558
obj_box
15751559
};

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

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,42 @@ use crate::types::{I32, I64, I8, PTR};
4242
/// scan-outward-past-switch-frames logic uses. A `new` inside a bare `switch`
4343
/// is therefore correctly treated as not-in-a-loop.
4444
fn new_site_is_in_loop(ctx: &FnCtx<'_>) -> bool {
45-
ctx.loop_targets
45+
if ctx
46+
.loop_targets
4647
.iter()
4748
.any(|(continue_label, _, _)| !continue_label.is_empty())
49+
{
50+
return true;
51+
}
52+
// #7815: a `new` in a function the hot-loop-callee pre-pass admitted is a
53+
// `new` in a loop, one frame out.
54+
//
55+
// The gate below this comment is about SPEED-vs-SIZE, and
56+
// `collect_hot_loop_callees` answers exactly the question the loop test
57+
// does — is this site hot enough to be worth ~268 bytes — with the
58+
// anti-bloat backstop already attached: it admits only a function that
59+
// (a) has a direct call site inside a loop and (b) has at most
60+
// `inline_hot_small_max_call_sites` (4) direct call sites in the whole
61+
// module. So the added code is bounded by 4 × (news in the function),
62+
// which is the same order the loop arm already accepts.
63+
//
64+
// Deliberately NOT `func.inline_hint`: that is this set intersected with a
65+
// 9..=20-statement window and with "not already `alwaysinline`", and the
66+
// functions this needs most fall out of BOTH. `makeCycle` is 5 statements,
67+
// so it is `alwaysinline` and never hinted — while being the single
68+
// hottest function in the program.
69+
//
70+
// `cycles.ts` is the shape that needs it: `makeCycle` is called 10M times
71+
// from `main`'s loop and allocates two `Cell`s, but its own body has no
72+
// loop, so both allocations took the outlined
73+
// `js_object_alloc_class_inline_keys` — 22% of the program's samples, plus
74+
// a further 5% in `arena_alloc`'s inline-state sync, for work the inline
75+
// bump does in eight stores.
76+
//
77+
// Reading `func.hot_loop_callee` here is well-ordered: `codegen/function.rs`
78+
// sets it from `cross_module.hot_loop_callees` before the entry block is
79+
// created and before any expression is lowered.
80+
ctx.func.hot_loop_callee
4881
}
4982

5083
/// Emit the instance allocation for `new <class_name>(...)` and return the raw
@@ -62,7 +95,37 @@ fn new_site_is_in_loop(ctx: &FnCtx<'_>) -> bool {
6295
/// emission, which is the `RootedGroup::adopt_emitted` push that roots it for
6396
/// the constructor body; nothing between the allocator call and that push can
6497
/// collect.
65-
pub(super) fn emit_instance_alloc(ctx: &mut FnCtx<'_>, class_name: &str, class: &Class) -> String {
98+
/// What [`emit_instance_alloc`] produced: the instance's user pointer, plus
99+
/// whether the allocation already stamped this class's canonical typed-shape
100+
/// layout into the object's `GcHeader` constant (#7815).
101+
pub(super) struct InstanceAlloc {
102+
pub(super) handle: String,
103+
/// `true` ⟹ the header already reads `GC_LAYOUT_POINTER_FREE |
104+
/// GC_OBJ_TYPED_LAYOUT_INTACT`, so the construction site owes the runtime
105+
/// only the address-dependent half of `js_gc_declare_typed_shape_layout`
106+
/// (clearing a recycled address's stale per-object record).
107+
pub(super) typed_layout_baked: bool,
108+
}
109+
110+
pub(super) fn emit_instance_alloc(
111+
ctx: &mut FnCtx<'_>,
112+
class_name: &str,
113+
class: &Class,
114+
) -> InstanceAlloc {
115+
let mut typed_layout_baked = false;
116+
let handle = emit_instance_alloc_inner(ctx, class_name, class, &mut typed_layout_baked);
117+
InstanceAlloc {
118+
handle,
119+
typed_layout_baked,
120+
}
121+
}
122+
123+
fn emit_instance_alloc_inner(
124+
ctx: &mut FnCtx<'_>,
125+
class_name: &str,
126+
class: &Class,
127+
typed_layout_baked: &mut bool,
128+
) -> String {
66129
// Compute total field count including inherited parent fields.
67130
// The runtime allocates at least 8 inline slots regardless, so this
68131
// mostly matters for shapes >8 fields.
@@ -296,8 +359,41 @@ pub(super) fn emit_instance_alloc(ctx: &mut FnCtx<'_>, class_name: &str, class:
296359
// `js_gc_note_slot_layout` so the GC sees real pointer-bearing
297360
// slots regardless of this initial tag.
298361
const GC_LAYOUT_POINTER_FREE: u64 = 0x4000;
362+
/// `GC_OBJ_TYPED_LAYOUT_INTACT` — the bit
363+
/// `class_field_inline_guard` requires before it will read or write
364+
/// a raw-f64 slot directly. Runtime-side name:
365+
/// `gc::layout::GC_OBJ_TYPED_LAYOUT_INTACT`.
366+
const GC_OBJ_TYPED_LAYOUT_INTACT: u64 = 0x1000;
299367
const OBJECT_TYPE_REGULAR: u64 = 1;
300368

369+
// #7815: when this class's canonical layout is declarable at
370+
// allocation AND its pointer mask is statically empty, the state
371+
// this header already carries (`GC_LAYOUT_POINTER_FREE`) is the
372+
// FINAL one, and the only thing `js_gc_declare_typed_shape_layout`
373+
// would add per instance is the intact bit. Stamping it into the
374+
// same constant store removes the call: on `churn_alloc` /
375+
// `push_cls` that call was ~30% of the program, almost all of it
376+
// re-deriving per object a fact that is a property of the SHAPE
377+
// (see `gc::shape_install`'s module docs — the memo already reduced
378+
// the map round-trip to a direct-mapped probe, and what is left is
379+
// that probe, the type-table lookup, and the call itself).
380+
//
381+
// Requires `field_count == slot_count`: that mismatch is the one
382+
// case `init_typed_shape_layout` answers by DOWNGRADING
383+
// (`layout_set_typed_unknown`), and a constant cannot express "it
384+
// depends". Computed here, before `ctx.block()` takes its mutable
385+
// borrow.
386+
*typed_layout_baked = super::typed_shape_init::layout_pointer_free_at_allocation(
387+
ctx,
388+
class_name,
389+
field_count,
390+
);
391+
let typed_intact_bits = if *typed_layout_baked {
392+
GC_OBJ_TYPED_LAYOUT_INTACT
393+
} else {
394+
0
395+
};
396+
301397
let alloc_field_count = std::cmp::max(field_count as u64, MIN_FIELD_SLOTS);
302398
let payload_size = object_header_size + alloc_field_count * FIELD_SLOT_SIZE;
303399
// Round the whole allocation up to FIELD_SLOT_SIZE (8). The inline
@@ -410,7 +506,7 @@ pub(super) fn emit_instance_alloc(ctx: &mut FnCtx<'_>, class_name: &str, class:
410506
// bits 32..63 = size (u32)
411507
let gc_packed: u64 = GC_TYPE_OBJECT
412508
| (GC_FLAG_ARENA << 8)
413-
| (GC_LAYOUT_POINTER_FREE << 16)
509+
| ((GC_LAYOUT_POINTER_FREE | typed_intact_bits) << 16)
414510
| ((total_size as u64) << 32);
415511
// GC_STORE_AUDIT(INIT): inline headers initialize freshly allocated unpublished object storage.
416512
blk.store(I64, &gc_packed.to_string(), &raw);

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,3 +585,63 @@ pub(crate) fn ctor_chain_uses_new_target(ctx: &FnCtx<'_>, class: &Class) -> bool
585585
}
586586
false
587587
}
588+
589+
/// ECMAScript constructor return-override, with the `undefined` case decided
590+
/// **inline** (#7815).
591+
///
592+
/// Every `new` site has to apply the spec's return-override rule: a
593+
/// constructor that returns an Object yields that object, `undefined` yields
594+
/// the implicit `this`, and any other primitive yields `this` for a base
595+
/// constructor or throws for a derived one. `js_ctor_return_override` decides
596+
/// all three.
597+
///
598+
/// The overwhelmingly common case is the one it can answer from a single
599+
/// 64-bit compare: a constructor with no `return <expr>` at all completes as
600+
/// `undefined`, and the runtime's answer for `undefined` is *exactly*
601+
/// `this_val` — `JSValue::is_undefined` is `bits == TAG_UNDEFINED`, and
602+
/// `constructor_return_overrides_this(undefined)` is `false` at its first
603+
/// `is_pointer()` test. Emitting that compare here turns a cross-crate call
604+
/// into a never-taken branch: measured at 8.2% of `churn_alloc` and `push_cls`,
605+
/// where the synthesized object-literal constructor's only `ret` is the
606+
/// `TAG_UNDEFINED` constant.
607+
///
608+
/// The slow arm is byte-for-byte the previous emission, so derived-constructor
609+
/// `TypeError`s, `return new Promise(…)`, arguments objects and arrays all keep
610+
/// the runtime's answer. Skipping the call cannot lose a relocation either:
611+
/// `js_ctor_return_override` is in `root_reload`'s no-reload set, so no live
612+
/// value's address depends on having made it.
613+
pub(super) fn emit_ctor_return_override(
614+
ctx: &mut FnCtx<'_>,
615+
obj_box: &str,
616+
ctor_ret: &str,
617+
is_derived: bool,
618+
) -> String {
619+
let override_idx = ctx.new_block("ctor_ret.override");
620+
let merge_idx = ctx.new_block("ctor_ret.merge");
621+
let override_label = ctx.block_label(override_idx);
622+
let merge_label = ctx.block_label(merge_idx);
623+
let this_pred_label = ctx.block_label(ctx.current_block);
624+
{
625+
let blk = ctx.block();
626+
let bits = blk.bitcast_double_to_i64(ctor_ret);
627+
let is_undef = blk.icmp_eq(crate::types::I64, &bits, crate::nanbox::TAG_UNDEFINED_I64);
628+
blk.cond_br(&is_undef, &merge_label, &override_label);
629+
}
630+
ctx.current_block = override_idx;
631+
let is_derived_lit = if is_derived { "1" } else { "0" };
632+
let overridden = ctx.block().call(
633+
DOUBLE,
634+
"js_ctor_return_override",
635+
&[(DOUBLE, obj_box), (DOUBLE, ctor_ret), (I32, is_derived_lit)],
636+
);
637+
let override_pred_label = ctx.block_label(ctx.current_block);
638+
ctx.block().br(&merge_label);
639+
ctx.current_block = merge_idx;
640+
ctx.block().phi(
641+
DOUBLE,
642+
&[
643+
(obj_box, &this_pred_label),
644+
(&overridden, &override_pred_label),
645+
],
646+
)
647+
}

0 commit comments

Comments
 (0)