diff --git a/changelog.d/9287-overflow-slot-ic.md b/changelog.d/9287-overflow-slot-ic.md new file mode 100644 index 0000000000..cd197017ae --- /dev/null +++ b/changelog.d/9287-overflow-slot-ic.md @@ -0,0 +1,11 @@ +### Performance + +- **Property access past an object's first two slots is now cached.** A plain + object keeps its first two properties in inline storage and the rest in an + overflow buffer — and the constant-key inline caches refused to prime for + the overflow region, so `obj["field_x"]` on any wider object missed its + cache on every single access, forever. Measured at 27 ms against 3 ms for + the identical loop, decided entirely by whether the property was added + second or third. Overflow slots now prime with the same encoding the + dynamic-key cache has always used, and hot access to a wide object's + fields runs ~5× faster (27 → 5 ms; node: 2 ms). 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 5c8e6e8202..1cdb2c8424 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -550,6 +550,41 @@ pub(crate) fn lower_generic_property_get( ctx.current_block = hit_idx; let cache_slot_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "1")]); let slot = ctx.block().load(I64, &cache_slot_ptr); + + // #9287: the primed slot word may carry IC_SLOT_OVERFLOW_BIT (1 << 30) — + // the field lives past the inline region, in the object's spill buffer, + // and the inline `obj + header + slot*8` arithmetic below must not run on + // it. Such hits route through `js_object_get_field_ic_overflow_load`, + // which loads through `overflow_get` and falls back to the full miss + // handler on a tombstoned slot. Sites whose field is inline never see the + // bit, so this branch predicts perfectly for them. The polymorphic WAYS + // never hold an encoded slot (`pic_prime_get` refuses to cascade one), so + // only this MRU path needs the check. + let ovf_idx = ctx.new_block("pic.hit.overflow"); + let inline_hit_idx = ctx.new_block("pic.hit.inline"); + let ovf_label = ctx.block_label(ovf_idx); + let inline_hit_label = ctx.block_label(inline_hit_idx); + let ovf_bits = ctx.block().and(I64, &slot, "1073741824"); // 1 << 30 + let is_ovf = ctx.block().icmp_ne(I64, &ovf_bits, "0"); + ctx.block().cond_br(&is_ovf, &ovf_label, &inline_hit_label); + + ctx.current_block = ovf_idx; + let ovf_key_handle = emit_key_handle(ctx, &key_handle_global); + let ovf_slot_i32 = ctx.block().trunc(I64, &slot, I32); + let val_ovf = ctx.block().call( + DOUBLE, + "js_object_get_field_ic_overflow_load", + &[ + (I64, &obj_handle), + (I64, &ovf_key_handle), + (I32, &ovf_slot_i32), + (PTR, &cache_ref), + ], + ); + let ovf_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = inline_hit_idx; let offset = ctx.block().shl(I64, &slot, "3"); // arm64_32 watchOS: the object fields region begins at // `size_of::()` past the user pointer — 16 on LP64 and @@ -908,6 +943,7 @@ pub(crate) fn lower_generic_property_get( DOUBLE, &[ (&val_hit, &hit_end_label), + (&val_ovf, &ovf_end_label), (&val_prefix, &prefix_end_label), (&val_desc_prefix, &desc_prefix_end_label), (&val_way, &way_end_label), diff --git a/crates/perry-codegen/src/expr/proxy_reflect.rs b/crates/perry-codegen/src/expr/proxy_reflect.rs index 05a63ca337..03c686bcb2 100644 --- a/crates/perry-codegen/src/expr/proxy_reflect.rs +++ b/crates/perry-codegen/src/expr/proxy_reflect.rs @@ -710,6 +710,40 @@ fn lower_put_value_static_write_ic( ], ); + // #9287: a primed slot word may carry IC_SLOT_OVERFLOW_BIT (1 << 30) — + // the property lives past the inline region, in the object's spill + // buffer. The inline address arithmetic below is inline-region-only, so + // such hits route through `js_put_value_set_ic_overflow_store`, which is + // the dynamic-key IC's audited validate-and-store (spill store, + // stable-tombstone hole check, barriers). One branch that predicts + // perfectly for sites whose property is inline: their slot words never + // have the bit. Helper failure (revoked between prime and hit) falls back + // to the full miss call, which re-primes way 1. + let ovf_idx = ctx.new_block("put.pic.hit.overflow"); + let inline_hit_idx = ctx.new_block("put.pic.hit.inline"); + let ovf_label = ctx.block_label(ovf_idx); + let inline_hit_label = ctx.block_label(inline_hit_idx); + let ovf_bits = ctx.block().and(I64, &selected_slot, "1073741824"); // 1 << 30 + let is_ovf = ctx.block().icmp_ne(I64, &ovf_bits, "0"); + ctx.block().cond_br(&is_ovf, &ovf_label, &inline_hit_label); + + ctx.current_block = ovf_idx; + let ovf_slot_i32 = ctx.block().trunc(I64, &selected_slot, I32); + let ovf_ok = ctx.block().call( + I32, + "js_put_value_set_ic_overflow_store", + &[ + (DOUBLE, &target_value), + (I64, &shape_token), + (I32, &ovf_slot_i32), + (DOUBLE, &stored_value), + ], + ); + let ovf_hit = ctx.block().icmp_ne(I32, &ovf_ok, "0"); + let ovf_end_label = ctx.block().label.clone(); + ctx.block().cond_br(&ovf_hit, &merge_label, &miss_label); + + ctx.current_block = inline_hit_idx; // `pointer_possible` is a COMPILE-TIME claim about the RHS, so it is true // for every `o.x = v` whose RHS is an untyped local — which is most of // them. Before #8184 that arm paid three unconditional `gc-leaf` calls @@ -906,6 +940,7 @@ fn lower_put_value_static_write_ic( DOUBLE, &[ (&stored_value, &hit_end_label), + (&stored_value, &ovf_end_label), (&deleted_value, &deleted_end_label), (&miss_value, &miss_end_label), (&miss2_value, &miss2_end_label), diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index eb7a82320e..ec942b61a8 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -520,6 +520,19 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { DOUBLE, &[DOUBLE, I64, DOUBLE, I32, PTR], ); + // #9287: validate-and-store for a constant-key IC hit whose slot word + // carries the overflow bit (property lives in the spill buffer). + module.declare_function( + "js_put_value_set_ic_overflow_store", + I32, + &[DOUBLE, I64, I32, DOUBLE], + ); + // #9287: MRU-hit load for a get-IC slot word carrying the overflow bit. + module.declare_function( + "js_object_get_field_ic_overflow_load", + DOUBLE, + &[I64, I64, I32, PTR], + ); module.declare_function( "js_put_value_set_ic_poly_tail", DOUBLE, diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index 913ee33409..7e0ae08e61 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -326,7 +326,16 @@ pub(crate) unsafe fn pic_prime_get(cache: *mut PicCache, token: i64, slot: i64) c[PIC_WAY_STATE] = state + 1; return; } - let cascade = prev_tok != 0 && prev_tok != token; + // #9287: an overflow-encoded slot (IC_SLOT_OVERFLOW_BIT) may live in the + // MRU entry — the emitted MRU hit path tests the bit and routes through + // `js_object_get_field_ic_overflow_load`. The emitted WAY path does not: + // it computes `obj + header + slot*8` directly, and an encoded slot there + // would be a wild load. Keep encoded slots out of the ways entirely; a + // polymorphic site rotating overflow shapes re-primes the MRU per shape, + // which is exactly the pre-#7753 behaviour. + let prev_is_overflow = + (prev_slot as u64) & u64::from(crate::proxy::IC_SLOT_OVERFLOW_BIT) != 0; + let cascade = prev_tok != 0 && prev_tok != token && !prev_is_overflow; // One pass over the ways does three things: // * evicts `token` from a way if it has one — it now lives in the MRU // entry, and leaving the stale copy behind would permanently cost a way @@ -413,6 +422,33 @@ unsafe fn key_bytes_are(key: *const crate::StringHeader, want: &[u8]) -> bool { let p = (key as *const u8).add(std::mem::size_of::()); std::slice::from_raw_parts(p, want.len()) == want } +/// #9287: MRU-hit load for a get-IC slot word carrying +/// `IC_SLOT_OVERFLOW_BIT` — the field lives past the inline region. The +/// emitted guards already matched the receiver's shape token in the same +/// straight-line region, so the slot is the one the prime validated; the load +/// still goes through `overflow_get` (spill buffer first, side table behind +/// the kill switch), and a hole — the field was tombstoned since priming — +/// falls back to the full miss handler, which honours deletion and the +/// prototype chain. +#[no_mangle] +pub extern "C" fn js_object_get_field_ic_overflow_load( + obj: *const ObjectHeader, + key: *const crate::StringHeader, + slot: i32, + cache: *mut PicCache, +) -> f64 { + let idx = (slot as u32 & !crate::proxy::IC_SLOT_OVERFLOW_BIT) as usize; + if !obj.is_null() { + if let Some(bits) = crate::object::overflow_get(obj as usize, idx) { + if bits != crate::value::TAG_HOLE { + return f64::from_bits(bits); + } + } + } + js_object_get_field_ic_miss(obj, key, cache) +} + + /// Monomorphic inline cache miss handler (issue #51). /// @@ -741,6 +777,40 @@ pub extern "C" fn js_object_get_field_ic_miss( let k_ptr = (k_bits & 0x0000_FFFF_FFFF_FFFF) as *const crate::StringHeader; if !k_ptr.is_null() && crate::string::js_string_equals(k_ptr, key) != 0 { if i >= alloc_limit { + // #9287: a field past the inline region primes too, + // with IC_SLOT_OVERFLOW_BIT — the emitted MRU hit path + // tests the bit and routes through + // `js_object_get_field_ic_overflow_load`. Before this, + // the break below meant an overflow field missed this + // cache on EVERY read. Descriptor-bearing receivers + // keep falling through (the slow path honours + // accessors), and the value must be readable through + // `overflow_get` right now — if it is not, priming + // would cache a lie. + if !has_own_descriptors + && (i as u32) < crate::proxy::IC_SLOT_OVERFLOW_BIT + { + if let Some(bits) = crate::object::overflow_get(obj as usize, i) { + if bits != crate::value::TAG_HOLE { + let stamp = crate::object::shapes::object_shape_stamp(obj); + let token = (stamp as u64 + | crate::object::shapes::PIC_ID_TOKEN_BIT) + as i64; + // Word 2 (named-prefix identity) stays 0: + // the prefix paths compute inline + // addresses and must never fire from an + // overflow-primed entry. + (*cache)[2] = 0; + pic_prime_get( + cache, + token, + (i as u32 | crate::proxy::IC_SLOT_OVERFLOW_BIT) + as i64, + ); + return f64::from_bits(bits); + } + } + } // Field is in the overflow map — fall through to the // slow path which handles overflow correctly. break; diff --git a/crates/perry-runtime/src/proxy/put_value.rs b/crates/perry-runtime/src/proxy/put_value.rs index 027bad2259..bf87ef7db1 100644 --- a/crates/perry-runtime/src/proxy/put_value.rs +++ b/crates/perry-runtime/src/proxy/put_value.rs @@ -328,16 +328,6 @@ pub extern "C" fn js_put_value_set( } value_handle.get_nanbox_f64() } - -/// Miss path for one way of the codegen-emitted polymorphic PutValue cache. -/// -/// The full strict/sloppy `[[Set]]` semantics run first. Only a successful -/// ordinary class-instance own-data overwrite may prime `[shape_token, slot]`; -/// every exotic, descriptor-bearing, frozen, class-object, plain-class-zero, -/// overflow, or typed-layout-intact receiver remains permanently on the miss -/// path. The token mirrors the read PIC: a stamped runtime ShapeId is lifted -/// above the pointer range with bit 62; otherwise the shared keys pointer is -/// used. The generated hit path repeats all mutable per-object guards. #[no_mangle] pub extern "C" fn js_put_value_set_ic_miss( target: f64, @@ -446,10 +436,26 @@ pub extern "C" fn js_put_value_set_ic_miss( let Some(idx) = own_idx else { return result; }; + // #9287: a slot past the inline region primes too, carrying + // IC_SLOT_OVERFLOW_BIT exactly like the dynamic-key IC's stub entries. + // The emitted hit path routes such slots through + // `js_put_value_set_ic_overflow_store`, which is `dyn_ic_try_store` — + // the same validate-and-store the dynamic IC has always used for + // overflow properties (spill store, tombstone check, barriers). Before + // this, `idx >= alloc_limit` returned without priming, so a property + // in overflow storage missed this cache on EVERY access: 3 ms vs + // 27 ms for the identical loop with the identical object, decided by + // whether the property sat at index 1 or index 2. let alloc_limit = shape.live_inline_slot_count as usize; - if idx as usize >= alloc_limit { + let slot_word: u32 = if (idx as usize) < alloc_limit { + idx + } else if (idx as usize) < shape.logical_key_count as usize + && idx < IC_SLOT_OVERFLOW_BIT + { + idx | IC_SLOT_OVERFLOW_BIT + } else { return result; - } + }; // The descriptor above already proves this stamp is live, so the // token comes from the header word rather than from a second full @@ -460,7 +466,7 @@ pub extern "C" fn js_put_value_set_ic_miss( // Publish the token last conceptually: a zero-initialized or stale // token cannot hit this slot until it matches this receiver's current // discriminated shape token. Perry's read PIC uses the same format. - (*cache)[1] = idx as i64; + (*cache)[1] = slot_word as i64; (*cache)[0] = shape_token as i64; // Rotating-key sites overflow the single-slot site cache immediately; // the global stub is what lets them hit. Key bits from the rooted key. @@ -474,7 +480,7 @@ pub extern "C" fn js_put_value_set_ic_miss( let boxed = f64::from_bits(crate::value::js_nanbox_string(key_now as i64).to_bits()); if let Some(kb) = stub_key_bits(boxed) { - write_stub_insert(shape_token, kb, idx); + write_stub_insert(shape_token, kb, slot_word); } } }); @@ -738,6 +744,30 @@ fn write_stub_insert(token: u64, key_bits: u64, slot: u32) { /// and bounds. What remains mutable per object is the GC header — type, /// forwarded, and the blocking flags `Object.freeze`-family operations set — /// and the hit still checks those on every store. +/// #9287: validate-and-store for a constant-key IC hit whose slot word +/// carries [`IC_SLOT_OVERFLOW_BIT`]. The emitted inline path has already +/// matched the receiver's shape token against the cache, but this helper +/// re-validates everything through [`dyn_ic_try_store`] anyway — the checks +/// are ~10 instructions, and reusing the dynamic IC's audited path means the +/// overflow store (spill buffer, stable-tombstone hole check, barriers, +/// layout notes) has exactly one implementation. Returns 1 and performs the +/// store on success; returns 0 without side effects when validation fails, +/// and the caller falls back to `js_put_value_set_ic_miss`. +#[no_mangle] +pub extern "C" fn js_put_value_set_ic_overflow_store( + target: f64, + token: i64, + slot: i32, + value: f64, +) -> i32 { + unsafe { + match dyn_ic_try_store(target, token as u64, slot as u32, value) { + Some(_) => 1, + None => 0, + } + } +} + pub(crate) const IC_SLOT_OVERFLOW_BIT: u32 = 1 << 30; /// Validated fast store for a cached `(token, slot)` hit. Header checks and diff --git a/crates/perry/tests/issue_9287_overflow_slot_ic.rs b/crates/perry/tests/issue_9287_overflow_slot_ic.rs new file mode 100644 index 0000000000..8d8ed450cd --- /dev/null +++ b/crates/perry/tests/issue_9287_overflow_slot_ic.rs @@ -0,0 +1,191 @@ +//! #9287: constant-key property access to a slot past the inline region +//! (overflow / spill storage) primes and hits the inline caches. +//! +//! Before the fix, `js_put_value_set_ic_miss` bailed with `idx >= alloc_limit` +//! and `js_object_get_field_ic_miss` broke out of its keys walk at the same +//! condition — so a property at index >= `live_inline_slot_count` (2 for a +//! plain `{}`) missed both ICs on EVERY access, forever: 3 ms vs 27 ms for +//! the identical loop, decided by whether the property sat at index 1 or 2. +//! A miss-handler probe put 99.9998% of 2.4M declines on that one bail. +//! +//! The fix primes such slots with `IC_SLOT_OVERFLOW_BIT` and routes emitted +//! hits through `js_put_value_set_ic_overflow_store` / +//! `js_object_get_field_ic_overflow_load` — the dynamic-key IC's audited +//! overflow path. What these tests guard is the INVALIDATION story of that +//! new hit path: every case below primes the cache hot and then changes the +//! world (delete, accessor, freeze, shape change, GC move) in a way the hit +//! must notice. A wrong answer here is the cache returning stale state — the +//! exact bug class a primed-but-unvalidated hit would ship. +//! +//! The polymorphic-rotation case pins `pic_prime_get`'s cascade guard: an +//! overflow-encoded slot must never enter the ways, whose emitted path +//! computes a raw inline address from the slot word (a wild load if the bit +//! ever reached it). + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run_with_env(source: &str, envs: &[(&str, &str)]) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let mut run_cmd = Command::new(&output); + run_cmd.current_dir(dir.path()); + for (k, v) in envs { + run_cmd.env(k, v); + } + let run = run_cmd.output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed (exit {:?})\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).trim().to_owned() +} + +fn compile_and_run(source: &str) -> String { + compile_and_run_with_env(source, &[]) +} + +/// Six properties: indices 0-1 inline, 2-5 in overflow for a plain `{}`. +const MK: &str = r#" +function mk(): any { const o: any = {}; for (let j = 0; j < 6; j++) o["f" + j] = j; return o; } +"#; + +#[test] +fn overflow_reads_and_writes_stay_correct_when_hot() { + let out = compile_and_run(&format!( + r#"{MK} +const o = mk(); let s = 0; +for (let i = 0; i < 10000; i++) {{ o["f4"] = i; s += o["f4"]; }} +console.log(s + " " + o["f0"] + " " + o["f5"]); +"# + )); + assert_eq!(out, "49995000 0 5"); +} + +#[test] +fn a_deleted_overflow_property_is_not_served_from_the_cache() { + // Primes the read+write caches hot, deletes the property, then reads. + // A hit path that skipped the tombstone check would return the last + // written value instead of undefined. + let out = compile_and_run(&format!( + r#"{MK} +const o = mk(); let s = 0; +for (let i = 0; i < 1000; i++) {{ o["f4"] = i; s += o["f4"]; }} +delete o["f4"]; +console.log(o["f4"] + " " + (s === 499500 ? "sum_ok" : "sum_" + s)); +"# + )); + assert_eq!(out, "undefined sum_ok"); +} + +#[test] +fn an_accessor_installed_after_priming_fires_on_the_next_read() { + // OBJ_FLAG_HAS_DESCRIPTORS must force the primed site back to the miss + // handler; a stale hit would read the raw slot and bypass the getter. + let out = compile_and_run(&format!( + r#"{MK} +const o = mk(); let s = 0; +for (let i = 0; i < 1000; i++) s += o["f4"]; +Object.defineProperty(o, "f4", {{ get() {{ return 777; }} }}); +console.log(o["f4"] + " " + (s === 4000 ? "ok" : "" + s)); +"# + )); + assert_eq!(out, "777 ok"); +} + +#[test] +fn freeze_after_priming_blocks_the_cached_write() { + let out = compile_and_run(&format!( + r#"{MK} +const o = mk(); +for (let i = 0; i < 1000; i++) o["f4"] = i; +Object.freeze(o); +o["f4"] = 12345; +console.log(o["f4"]); +"# + )); + assert_eq!(out, "999"); +} + +#[test] +fn a_deleted_then_rewritten_overflow_property_revives_through_the_cache() { + let out = compile_and_run(&format!( + r#"{MK} +const o = mk(); let s = 0; +for (let i = 0; i < 500; i++) {{ o["f4"] = i; s += o["f4"]; }} +delete o["f4"]; +o["f4"] = 999; +for (let i = 0; i < 500; i++) s += o["f4"]; +console.log(o["f4"] + " " + s); +"# + )); + assert_eq!(out, "999 624250"); +} + +#[test] +fn rotating_overflow_shapes_at_one_site_stays_correct() { + // Pins `pic_prime_get`'s cascade guard: three shapes rotate through one + // site whose property is in overflow on each of them. If an encoded slot + // ever cascaded into a way, the emitted way path would compute an + // inline address from it — index | (1 << 30) scaled by 8 — and the read + // would be garbage or a crash, not a number. + let out = compile_and_run(&format!( + r#"{MK} +const a = mk(); +const b = mk(); b["extra"] = 0; +const c = mk(); c["e1"] = 0; c["e2"] = 0; +function site(o: any): number {{ let t = 0; for (let i = 0; i < 500; i++) {{ o["f3"] = i; t += o["f3"]; }} return t; }} +let s = 0; +for (let r = 0; r < 4; r++) {{ s += site(a); s += site(b); s += site(c); }} +console.log(s); +"# + )); + assert_eq!(out, "1497000"); +} + +#[test] +fn pointer_values_through_the_overflow_write_survive_evacuating_gc() { + // String values exercise the write barrier inside the spill store; the + // heap limit plus forced evacuation makes the collector actually move + // things while the site is hot. + let out = compile_and_run_with_env( + &format!( + r#"{MK} +const o = mk(); const keep: string[] = []; +for (let i = 0; i < 5000; i++) {{ + o["f5"] = "str_" + (i % 7); + keep.push("g" + i); + if (keep.length > 64) keep.length = 0; +}} +console.log(o["f5"] + " " + o["f0"]); +"# + ), + &[("PERRY_GC_HEAP_LIMIT", "8"), ("PERRY_GC_FORCE_EVACUATE", "1")], + ); + assert_eq!(out, "str_1 0"); +}