From bd4085c18250315a668e4ba66c670616f161bfc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 23 Aug 2026 22:31:21 +0200 Subject: [PATCH] perf(codegen): delete six instructions from the generic property-read hit path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tree-walking-interpreter benchmarks `interp` and `iso_miss` are instruction-bound with IPC matching Node's, so the only lever is emitting less work. Profiling put ~40% of `evalNode`'s emitted IR in the generic `obj.property` dispatch diamond, and the megamorphic handler was cold — the cost is the per-read ladder itself, ~30 AArch64 instructions on a hit. Two independent removals, both in the PIC lowering. No runtime change, no cache-layout change. The ShapeId range test was redundant against the cache compare. The token was built as `is_stamp ? (pcid | 1<<62) : 0` plus a separate non-zero compare, which the backend spent six instructions on. `pic_prime_get` is the only writer of the cached token and is only ever handed `object_shape_stamp(obj) | PIC_ID_TOKEN_BIT`, which is zero outside the ShapeId range — so an equal token already proves the receiver carries that shape. The one case equality alone does not catch is a cached `0 | 1<<62` aliasing a `parent_class_id == 0` receiver; a single `pcid != 0` compare replaces the range test and preserves the #809 behaviour. The pooled property key was materialised on the fast path but used only on cold ones. Every consumer — the IC-miss handler, both by-name arms, the class-ref helper — sits on a cold edge, so the load is now emitted per consumer instead of once up front. That is also the more correct reading: the pool entry is a mutable global GC evacuation rewrites. Measured on one host, instructions retired, best of three, both arms from the same target directory: interp 8.167 G -> 7.591 G -7.05% (1.34x -> 1.25x vs Node 26.5.1) iso_miss 12.006 G -> 11.420 G -4.88% (1.87x -> 1.78x) shapes, tree, churn, fib40, push_cls: within 0.05% peak RSS unchanged; all seven print byte-identical output to node Also measured and rejected: folding each `(token, slot)` pair into one `i128` load so the backend emits `ldp`. It shrinks `evalNode` by 1.3% of its instructions but only trades `adrp`+`ldr` for `adrp`+`add`+`ldr`, and both benchmarks moved less than run-to-run noise. Refs #8591 Claude-Session: https://claude.ai/code/session_01LWQ5Pqfwj4DT5BPQjkhbUx --- changelog.d/8591-pic-hit-path-shape-token.md | 43 +++++++ .../src/expr/property_get/generic_dispatch.rs | 107 +++++++++++++----- 2 files changed, 123 insertions(+), 27 deletions(-) create mode 100644 changelog.d/8591-pic-hit-path-shape-token.md diff --git a/changelog.d/8591-pic-hit-path-shape-token.md b/changelog.d/8591-pic-hit-path-shape-token.md new file mode 100644 index 0000000000..cbbfc771fc --- /dev/null +++ b/changelog.d/8591-pic-hit-path-shape-token.md @@ -0,0 +1,43 @@ +### Performance — generic property reads + +Cut six AArch64 instructions from the hit path of **every** generic +`obj.property` read, worth **−7.05% instructions on `interp`** (8.167 G → +7.591 G) and **−4.88% on `iso_miss`** (12.006 G → 11.420 G) — the two +tree-walking-interpreter benchmarks tracked in #8591. Against the pinned Node +26.5.1 oracle that moves `interp` from 1.34x to 1.25x and `iso_miss` from 1.87x +to 1.78x. Measured as instructions retired (`/usr/bin/time -l`, best of three, +same host, both arms built from the same target directory). The rest of the +CPU-bound corpus — `shapes`, `tree`, `churn`, `fib40`, `push_cls` — moved by at +most 0.05%, peak RSS is unchanged (32.8 MB / 33.0 MB), and every program still +prints byte-identical output to `node --experimental-strip-types`. + +Two independent removals, both in `perry-codegen`'s PIC dispatch +(`expr/property_get/generic_dispatch.rs`); no runtime change, no layout change. + +**The ShapeId range test was redundant against the cache compare.** The emitted +token used to be `is_stamp ? (pcid | 1<<62) : 0` followed by a separate +"token is non-zero" compare — a `select`, two constant materialisations and two +extra compares that the backend spent six instructions on, per read. The range +test is *implied*: `pic_prime_get` is the only writer of the cached token +(`js_put_value_set_ic_miss` writes a different, set-side cache), and it is only +ever handed `object_shape_stamp(obj) | PIC_ID_TOKEN_BIT`, which is zero outside +`SHAPE_ID_BASE..SHAPE_ID_END`. So an equal token already proves the receiver +carries that ShapeId. The one case the range test caught that equality does not +is a cached `0 | 1<<62`, primed by an unstamped receiver, aliasing a receiver +whose `parent_class_id` is 0 — a single `pcid != 0` compare replaces it, and +keeps the #809 behaviour (a keyless `Object.create(proto)` receiver still falls +through to the prototype-chain walk) exactly as it was. + +**The property key was materialised on the fast path but only used on cold +ones.** Every consumer of the pooled `StringHeader*` — the IC-miss handler, +both `js_object_get_field_by_name_f64` arms, the class-ref helper — sits on a +cold edge of the dispatch diamond, yet the load was emitted once up front, so +every hit paid a dependent load of a global it never read. It is now emitted +per consumer. Because the pool entry is a mutable global that GC evacuation +rewrites, re-reading it at each use is also the more correct reading. + +Measured and rejected along the way: folding each `(token, slot)` cache pair +into one `i128` load, to make the backend emit `ldp` and drop the duplicated +address materialisation. It does shrink `evalNode` by 1.3% of its instructions, +but the backend simply trades `adrp`+`ldr` for `adrp`+`add`+`ldr` — the +benchmarks moved by less than run-to-run noise, so the complexity was not kept. diff --git a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs index 10f9b46211..bd1dbf2057 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -34,6 +34,27 @@ pub(crate) const PIC_WAYS: usize = 4; /// both skip them. Mirrors the runtime's `PIC_WAY_STATE`. pub(crate) const PIC_WAY_STATE: usize = 3; +/// Materialise the pooled property-key `StringHeader*` in the CURRENT block. +/// +/// Every consumer of the key — `js_object_get_field_ic_miss`, the two +/// `js_object_get_field_by_name_f64` arms, the class-ref helper — sits on a +/// COLD edge of the dispatch diamond, but the load used to be emitted once up +/// front, in the entry block, so the hit path of every generic property read +/// paid a dependent load of a global it never used. Emitting it per consumer +/// duplicates dead-cheap code into blocks that are already making a call, and +/// takes the load off the fast path entirely. +/// +/// The pool entry is a *mutable* global — GC evacuation rewrites it — so +/// re-reading it at each consumer is not merely cheap, it is the correct +/// reading: every cold block sees the pool's current address rather than one +/// captured before whatever collected. +fn emit_key_handle(ctx: &mut FnCtx<'_>, key_handle_global: &str) -> String { + let blk = ctx.block(); + let key_box = blk.load(DOUBLE, key_handle_global); + let key_bits = blk.bitcast_double_to_i64(&key_box); + blk.and(I64, &key_bits, POINTER_MASK_I64) +} + /// The generic per-site monomorphic inline-cache dispatch for `obj.property`. /// This is the fall-through tail of the general catch-all arm: all earlier /// specializations have been ruled out. @@ -62,9 +83,12 @@ pub(crate) fn lower_generic_property_get( let blk = ctx.block(); let obj_bits = blk.bitcast_double_to_i64(&obj_box); let obj_handle = blk.and(I64, &obj_bits, POINTER_MASK_I64); - let key_box = blk.load(DOUBLE, &key_handle_global); - let key_bits = blk.bitcast_double_to_i64(&key_box); - let key_handle = blk.and(I64, &key_bits, POINTER_MASK_I64); + // The key handle is materialised per consumer (see `emit_key_handle`), all + // of which are cold. The one exception is the typed-feedback OBSERVE call, + // which sits in the hot `pget.pic` block — so under `--typed-feedback` the + // handle is still produced once, up front, exactly as before. + let key_handle_observed = crate::expr::typed_feedback_emission_enabled() + .then(|| emit_key_handle(ctx, &key_handle_global)); let feedback_site_id = emit_typed_feedback_register_site( ctx, TypedFeedbackKind::PropertyGet, @@ -91,6 +115,7 @@ pub(crate) fn lower_generic_property_get( .push((format!("__ic_decl_{}", cache_site), DOUBLE, vec![])); ctx.ic_globals.push(cache_name.clone()); let cache_ref = format!("@{}", cache_name); + let key_handle = emit_key_handle(ctx, &key_handle_global); let val = ctx.block().call( DOUBLE, "js_object_get_field_ic", @@ -214,6 +239,7 @@ pub(crate) fn lower_generic_property_get( // `constructor` lookup. Pass full obj_bits (NOT obj_handle — // the runtime needs the unmasked top16 to detect the tag). ctx.current_block = class_ref_idx; + let key_handle = emit_key_handle(ctx, &key_handle_global); let class_ref_result = ctx.block().call( DOUBLE, "js_typed_feedback_object_get_field_by_name_f64", @@ -227,13 +253,14 @@ pub(crate) fn lower_generic_property_get( ctx.block().br(&final_merge_label); ctx.current_block = pic_idx; + let observed_key = key_handle_observed.clone().unwrap_or_default(); crate::expr::emit_typed_feedback_record_call( ctx.block(), "js_typed_feedback_observe_property_get", &[ (I64, &feedback_site_id), (I64, &obj_handle), - (I64, &key_handle), + (I64, &observed_key), ], ); @@ -385,36 +412,55 @@ pub(crate) fn lower_generic_property_get( ctx.current_block = tok_idx; // The receiver token is derived solely from its authoritative ShapeId. - // Invalid/unstamped payloads produce zero and miss closed. + // Invalid/unstamped payloads miss closed. // #8113: the ShapeId word moved from header offset 8 to 4. let pcid_addr = ctx.block().add(I64, &obj_handle, "4"); let pcid_ptr = ctx.block().inttoptr(I64, &pcid_addr); let pcid = ctx.block().load(I32, &pcid_ptr); - // In-range test via wrapping add + ult: (pcid - 0x8000_0000) < 0x4000_0000. - // (-2147483648 is the i32 spelling of the 0x8000_0000 subtrahend.) - let pcid_rel = ctx.block().add(I32, &pcid, "-2147483648"); - let is_stamp = ctx.block().icmp_ult(I32, &pcid_rel, "1073741824"); let pcid64 = ctx.block().zext(I32, &pcid, I64); - // PIC_ID_TOKEN_BIT = 1 << 62. - let id_token = ctx.block().or(I64, &pcid64, "4611686018427387904"); - let token = ctx.block().select(I1, &is_stamp, I64, &id_token, "0"); + // PIC_ID_TOKEN_BIT = 1 << 62. The token is formed UNCONDITIONALLY — the + // in-range test the emitted code used to run first + // (`(pcid - 0x8000_0000)