From 2b3248f992378d93d2391bb2b403a16841c16064 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 04:47:44 +0200 Subject: [PATCH] perf(codegen): refresh rooted arrays, gate super-scope, and count store safepoints Lands four reviewed PRs as one squash. - #8670: refresh rooted arrays during iteration. - #8673: fix a `--report-size` false positive from std-internal crate names. - #8668: specialize dense Array-subclass indexing. - #8678 (#8583): count property/index STORES as GC safepoint sites in the spill estimate. `PropertySet`/`PropertyUpdate`/`IndexSet` lower to collecting runtime calls that rewrite-statepoints-for-gc gives a statepoint, but none were counted, so a closed-shape object literal's constructor -- one long run of `this.field = v` -- estimated ~0, was never spilled to the shadow frame, and RS4GC grew one `__AnonShape_*_constructor` from 34,009 to 2,280,128 instructions, overrunning the #8586 per-function budget and refusing the whole module. Reads are deliberately not counted: they frequently inline to a shape-cached load with no call, so counting them would over-spill read-heavy hot loops. Version bump stripped per maintainer policy; the Cargo.lock diff was verified version-only before stripping. --- changelog.d/8583-count-store-safepoints.md | 1 + changelog.d/8668-array-subclass-indexing.md | 12 + .../8670-refresh-rooted-arrays-iteration.md | 2 + .../8673-report-size-std-internal-filter.md | 3 + .../src/collectors/safepoint_sites.rs | 54 +++ .../expr/index_get/inline_dyn_typed_array.rs | 210 ++++++++- .../src/runtime_decls/strings.rs | 7 + .../perry-runtime/src/array/iter_methods.rs | 55 ++- crates/perry-runtime/src/array/mod.rs | 5 +- crates/perry-runtime/src/array/subclass.rs | 401 ++++++++++++++++++ .../perry-runtime/src/array/subclass_tests.rs | 54 ++- crates/perry-runtime/src/object/mod.rs | 6 +- .../src/object/polymorphic_index.rs | 11 +- .../perry-runtime/src/value/dynamic_object.rs | 4 + .../perry/src/commands/compile/size_report.rs | 237 +++++++++++ .../issue_8655_array_subclass_indexing.rs | 184 ++++++++ 16 files changed, 1228 insertions(+), 18 deletions(-) create mode 100644 changelog.d/8583-count-store-safepoints.md create mode 100644 changelog.d/8668-array-subclass-indexing.md create mode 100644 changelog.d/8670-refresh-rooted-arrays-iteration.md create mode 100644 changelog.d/8673-report-size-std-internal-filter.md create mode 100644 crates/perry/tests/issue_8655_array_subclass_indexing.rs diff --git a/changelog.d/8583-count-store-safepoints.md b/changelog.d/8583-count-store-safepoints.md new file mode 100644 index 0000000000..56e36504ea --- /dev/null +++ b/changelog.d/8583-count-store-safepoints.md @@ -0,0 +1 @@ +fix(codegen): the RS4GC root-spill estimate (`count_safepoint_sites`, #8583) now counts property/index STORES (`PropertySet`, `PropertyUpdate`, `IndexSet`) as GC safepoints, extending the earlier literal-counting fix. A closed-shape object literal compiles to a constructor that is one long run of `this.field = v` stores, each lowering to a collecting `js_class_field_set_ic` / `js_set_property` call that RS4GC gives a statepoint; with none counted, the constructor's estimate was ~0, it was never spilled, and RS4GC grew one such `__AnonShape_*_constructor` from 34k to 2.28M instructions — overrunning the #8586 per-function budget and refusing the whole module. Reads (`PropertyGet`/`IndexGet`) are deliberately not counted: they frequently inline to a shape-cached load with no call, and counting them would over-spill read-heavy hot loops. diff --git a/changelog.d/8668-array-subclass-indexing.md b/changelog.d/8668-array-subclass-indexing.md new file mode 100644 index 0000000000..59ae9245ab --- /dev/null +++ b/changelog.d/8668-array-subclass-indexing.md @@ -0,0 +1,12 @@ +### Performance — Array-subclass numeric indexing + +Numeric reads from a stable `class X extends Array` instance now use an exact +class-and-ShapeId inline cache and load dense own elements directly from their +object slots. The guarded path retains generic semantics for holes, accessors, +prototype changes, proxies, forwarding, and real-Array element-kind changes, +while removing per-index string creation and generic property lookup from hot +ECS loops. + +The Wolf-shaped #8655 reproducer (1,000 entities and 2,000 system iterations) +improves from a 1,064.0 ms median to 149.1 ms on Windows, a 7.1x speedup, with +identical output. diff --git a/changelog.d/8670-refresh-rooted-arrays-iteration.md b/changelog.d/8670-refresh-rooted-arrays-iteration.md new file mode 100644 index 0000000000..f697fbfd93 --- /dev/null +++ b/changelog.d/8670-refresh-rooted-arrays-iteration.md @@ -0,0 +1,2 @@ +Refresh rooted arrays during iteration so a collection that relocates the +backing store cannot leave the iterator reading a stale address. diff --git a/changelog.d/8673-report-size-std-internal-filter.md b/changelog.d/8673-report-size-std-internal-filter.md new file mode 100644 index 0000000000..b6aeff1b63 --- /dev/null +++ b/changelog.d/8673-report-size-std-internal-filter.md @@ -0,0 +1,3 @@ +### Fixed + +- `perry compile --report-size`: the "duplicate crate instance" finding no longer reports a false positive when a crate name collides with one Rust's own standard library vendors internally for `std::backtrace`/panic-unwinding support (`gimli`, `addr2line`, `miniz_oxide`, `object`, `rustc_demangle`). Root-caused by demangling the real symbols behind a suspicious second `gimli` instance: every one was `gimli::read::cfi::{EhFrame, CommonInformationEntry, Augmentation, ...}` — DWARF exception-handling-frame parsing, the narrow surface `std`'s own unwinder uses, baked into the prebuilt `std` shipped with the toolchain and never a resolvable Cargo dependency of the build at all (invisible to `cargo tree`/`cargo build --unit-graph`, both of which correctly show only one real `gimli` unit). A real application dependency on `gimli` reads debug info instead (`read::abbrev`, `read::line`, ...), so the report now classifies a hash as `std`'s internal copy when its symbols carry at least one CFI-specific marker and zero debug-info-specific ones, excludes it from the duplicate finding, and lists it separately under a new "Excluded: std-internal copies" section instead of silently dropping it. diff --git a/crates/perry-codegen/src/collectors/safepoint_sites.rs b/crates/perry-codegen/src/collectors/safepoint_sites.rs index 4cb67640f9..d96f39e0f5 100644 --- a/crates/perry-codegen/src/collectors/safepoint_sites.rs +++ b/crates/perry-codegen/src/collectors/safepoint_sites.rs @@ -71,6 +71,20 @@ fn is_safepoint(e: &Expr) -> bool { | Expr::ObjectAssign { .. } | Expr::Array(_) | Expr::ArraySpread(_) + // #8583 (`__AnonShape_*_constructor`): a property/index STORE lowers + // to an allocating, collecting runtime call (`js_class_field_set_ic` + // / `js_set_property` / the array-set helpers) that RS4GC gives a + // statepoint. A closed-shape object literal compiles to a constructor + // that is one long run of `this.field = v` stores (`PropertySet`); + // with none counted the estimate was ~0, the constructor was not + // spilled, and RS4GC grew it 34k -> 2.28M instructions, overrunning + // the #8586 budget. Count the stores (not the reads: + // `PropertyGet`/`IndexGet` frequently inline to a shape-cached load + // with no call, and counting them would over-spill read-heavy hot + // loops). `PropertyUpdate` (`x.f++`) is a read-modify-write store. + | Expr::PropertySet { .. } + | Expr::PropertyUpdate { .. } + | Expr::IndexSet { .. } ) } @@ -257,4 +271,44 @@ mod tests { // 1 outer + 10 inner = 11. assert_eq!(count_safepoint_sites(&[Stmt::Expr(Expr::Array(rows))]), 11); } + + #[test] + fn property_and_index_stores_are_safepoints() { + // #8583: `this.field = v` / `arr[i] = v` lower to a collecting runtime + // call and must count. A closed-shape object literal is a constructor of + // many `PropertySet` stores (the `__AnonShape_*_constructor` shape) — the + // pre-fix count saw none, so the constructor never spilled and RS4GC + // overran the #8586 budget. + let this = || Expr::LocalGet(0); + let set = |p: &str| Expr::PropertySet { + object: Box::new(this()), + property: p.to_string(), + value: Box::new(Expr::Number(1.0)), + }; + // Three field stores in the constructor body. + let body = vec![ + Stmt::Expr(set("a")), + Stmt::Expr(set("b")), + Stmt::Expr(set("c")), + ]; + assert_eq!(count_safepoint_sites(&body), 3); + + // An index store counts too; the value sub-expression still recurses + // (a call in the value is its own safepoint). + let idx_set = Expr::IndexSet { + object: Box::new(this()), + index: Box::new(Expr::Number(0.0)), + value: Box::new(call(vec![])), + }; + // 1 for the IndexSet + 1 for the call in `value`. + assert_eq!(count_safepoint_sites(&[Stmt::Expr(idx_set)]), 2); + + // A read (`PropertyGet`) is deliberately NOT a safepoint (it inlines). + let get = Expr::PropertyGet { + object: Box::new(this()), + property: "x".to_string(), + byte_offset: 0, + }; + assert_eq!(count_safepoint_sites(&[Stmt::Expr(get)]), 0); + } } diff --git a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs index 9ce38dfda7..f911454ef6 100644 --- a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs +++ b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs @@ -12,10 +12,10 @@ //! `expr::temp_root` symbol, so only the sabotage arm makes the line an //! assertion. The audit that earned it: the entry point receives the receiver //! and index already lowered, lowers no user expression, and emits only pure -//! IR (guards, GEPs, loads) plus the out-of-line `js_dyn_index_get` fallback — -//! so no register of a GC value spans a lowering here. +//! IR (guards, GEPs, loads) plus an out-of-line semantic fallback, so no +//! register of a GC value spans a lowering here. -use crate::types::{DOUBLE, F32, I1, I16, I32, I64, I8}; +use crate::types::{DOUBLE, F32, I1, I16, I32, I64, I8, PTR}; use super::FnCtx; @@ -149,7 +149,7 @@ pub(super) fn lower_inline_dyn_typed_array_get( // ---- load: per-kind direct element load (data = header + 16) ---- ctx.current_block = load_idx; // (value, end_label) for each per-kind load block, collected for the merge. - let kind_incoming: Vec<(String, String)>; + let mut kind_incoming: Vec<(String, String)>; { // Per-kind load blocks. Each computes the element address from // `data = raw + 16` and `off = idx * elem_size`, loads the native @@ -316,12 +316,205 @@ pub(super) fn lower_inline_dyn_typed_array_get( kind_incoming = incoming; } - // ---- slow: the unchanged runtime dispatcher ---- + // ---- typed-array miss: Array-subclass shape IC, then dispatcher ---- ctx.current_block = slow_idx; + let site_id = ctx.ic_site_counter; + ctx.ic_site_counter += 1; + let cache_name = super::super::inline_cache_global_name(ctx, site_id); + ctx.ic_globals.push(cache_name.clone()); + let cache_ref = format!("@{cache_name}"); + + let object_header_idx = ctx.new_block("arrlike.ic.header"); + let object_bounds_idx = ctx.new_block("arrlike.ic.bounds"); + let object_inline_idx = ctx.new_block("arrlike.ic.inline"); + let object_spill_idx = ctx.new_block("arrlike.ic.spill"); + let object_spill_ptr_idx = ctx.new_block("arrlike.ic.spill_ptr"); + let object_spill_load_idx = ctx.new_block("arrlike.ic.spill_load"); + let object_miss_idx = ctx.new_block("arrlike.ic.miss"); + let object_header_label = ctx.block_label(object_header_idx); + let object_bounds_label = ctx.block_label(object_bounds_idx); + let object_inline_label = ctx.block_label(object_inline_idx); + let object_spill_label = ctx.block_label(object_spill_idx); + let object_spill_ptr_label = ctx.block_label(object_spill_ptr_idx); + let object_spill_load_label = ctx.block_label(object_spill_load_idx); + let object_miss_label = ctx.block_label(object_miss_idx); + + // Reject every non-pointer / handle-band / noncanonical-index case before + // touching a managed header. The miss helper retains full ToPropertyKey, + // Proxy, string, descriptor, hole and prototype-chain semantics. + let heap_floor = + crate::target_layout::heap_addr_lower_bound_inclusive(ctx.target_triple).to_string(); + let heap_ceiling = + crate::target_layout::heap_addr_upper_bound_exclusive(ctx.target_triple).to_string(); + let (object_raw, object_entry_ok) = { + let blk = ctx.block(); + let bits = blk.bitcast_double_to_i64(obj_box); + let raw = blk.and(I64, &bits, pointer_mask); + let tag = blk.and(I64, &bits, &tag_mask); + let is_ptr = blk.icmp_eq(I64, &tag, pointer_tag); + let above_floor = blk.icmp_uge(I64, &raw, &heap_floor); + let below_ceiling = blk.icmp_ult(I64, &raw, &heap_ceiling); + let idx_ge0 = blk.fcmp("oge", idx_d, "0.0"); + let idx_lt = blk.fcmp("olt", idx_d, "4294967295.0"); + let valid_ptr = blk.and(I1, &is_ptr, &above_floor); + let valid_ptr = blk.and(I1, &valid_ptr, &below_ceiling); + let valid_idx = blk.and(I1, &idx_ge0, &idx_lt); + (raw, blk.and(I1, &valid_ptr, &valid_idx)) + }; + ctx.block() + .cond_br(&object_entry_ok, &object_header_label, &object_miss_label); + + // Exact class + semantic ShapeId identity. The runtime primes only a + // prototype-unmodified dense Array-subclass shape with no relevant + // accessors, and publishes no heap pointer in this cache. + ctx.current_block = object_header_idx; + let object_idx_i64 = ctx.block().fptosi(DOUBLE, idx_d, I64); + let object_idx_back = ctx.block().sitofp(I64, &object_idx_i64, DOUBLE); + let object_idx_is_int = ctx.block().fcmp("oeq", &object_idx_back, idx_d); + let gc_type_addr = ctx.block().sub(I64, &object_raw, "8"); + let gc_type_ptr = ctx.block().inttoptr(I64, &gc_type_addr); + let gc_type = ctx.block().load(I8, &gc_type_ptr); + let is_object = ctx.block().icmp_eq(I8, &gc_type, "2"); + let gc_flags_addr = ctx.block().sub(I64, &object_raw, "7"); + let gc_flags_ptr = ctx.block().inttoptr(I64, &gc_flags_addr); + let gc_flags = ctx.block().load(I8, &gc_flags_ptr); + let forwarded = ctx.block().and(I8, &gc_flags, "1"); + let not_forwarded = ctx.block().icmp_eq(I8, &forwarded, "0"); + let object_ptr = ctx.block().inttoptr(I64, &object_raw); + let class_id = ctx.block().load(I32, &object_ptr); + let shape_addr = ctx.block().add(I64, &object_raw, "4"); + let shape_ptr = ctx.block().inttoptr(I64, &shape_addr); + let shape_id = ctx.block().load(I32, &shape_ptr); + let class64 = ctx.block().zext(I32, &class_id, I64); + let shape64 = ctx.block().zext(I32, &shape_id, I64); + let class_high = ctx.block().shl(I64, &class64, "32"); + let live_key = ctx.block().or(I64, &class_high, &shape64); + let cached_key_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "0")]); + let cached_key = ctx.block().load(I64, &cached_key_ptr); + let key_matches = ctx.block().icmp_eq(I64, &live_key, &cached_key); + let key_nonzero = ctx.block().icmp_ne(I64, &cached_key, "0"); + let object_ok = ctx.block().and(I1, &object_idx_is_int, &is_object); + let object_ok = ctx.block().and(I1, &object_ok, ¬_forwarded); + let object_ok = ctx.block().and(I1, &object_ok, &key_matches); + let object_ok = ctx.block().and(I1, &object_ok, &key_nonzero); + ctx.block() + .cond_br(&object_ok, &object_bounds_label, &object_miss_label); + + // The exact shape proves the cached length slot is live and inline. Check + // its current value and the proved dense prefix on every hit; growing + // `length` without creating properties therefore cannot expose holes. + ctx.current_block = object_bounds_idx; + let length_slot_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "1")]); + let length_slot = ctx.block().load(I64, &length_slot_ptr); + let element_base_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "2")]); + let element_base = ctx.block().load(I64, &element_base_ptr); + let dense_prefix_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "3")]); + let dense_prefix = ctx.block().load(I64, &dense_prefix_ptr); + let inline_bound_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "4")]); + let inline_bound = ctx.block().load(I64, &inline_bound_ptr); + let object_header_size = + crate::target_layout::object_header_size_bytes(ctx.target_triple).to_string(); + let length_bytes = ctx.block().shl(I64, &length_slot, "3"); + let length_offset = ctx.block().add(I64, &length_bytes, &object_header_size); + let length_addr = ctx.block().add(I64, &object_raw, &length_offset); + let length_ptr = ctx.block().inttoptr(I64, &length_addr); + let live_length = ctx.block().load(DOUBLE, &length_ptr); + let below_length = ctx.block().fcmp("olt", idx_d, &live_length); + let below_prefix = ctx.block().icmp_ult(I64, &object_idx_i64, &dense_prefix); + let in_dense_range = ctx.block().and(I1, &below_length, &below_prefix); + let object_slot = ctx.block().add(I64, &element_base, &object_idx_i64); + let slot_is_inline = ctx.block().icmp_ult(I64, &object_slot, &inline_bound); + let inline_ok = ctx.block().and(I1, &in_dense_range, &slot_is_inline); + let slot_is_spilled = ctx.block().xor(I1, &slot_is_inline, "true"); + let range_but_spilled = ctx.block().and(I1, &in_dense_range, &slot_is_spilled); + let spill_or_miss_idx = ctx.new_block("arrlike.ic.spill_or_miss"); + let spill_or_miss_label = ctx.block_label(spill_or_miss_idx); + ctx.block() + .cond_br(&inline_ok, &object_inline_label, &spill_or_miss_label); + ctx.current_block = spill_or_miss_idx; + ctx.block() + .cond_br(&range_but_spilled, &object_spill_label, &object_miss_label); + + ctx.current_block = object_inline_idx; + let inline_bytes = ctx.block().shl(I64, &object_slot, "3"); + let inline_offset = ctx.block().add(I64, &inline_bytes, &object_header_size); + let inline_addr = ctx.block().add(I64, &object_raw, &inline_offset); + let inline_ptr = ctx.block().inttoptr(I64, &inline_addr); + let inline_raw = ctx.block().load(DOUBLE, &inline_ptr); + let inline_value = if coerce_slow_to_number { + ctx.block() + .call(DOUBLE, "js_number_coerce", &[(DOUBLE, &inline_raw)]) + } else { + inline_raw + }; + let inline_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + // Wide subclass instances store absolute field slots in the object-owned + // spill Array. Reload both moving pointers from the live receiver; the IC + // itself contains only scalar offsets. + ctx.current_block = object_spill_idx; + let meta_ptr_size: u64 = if crate::target_layout::target_is_ilp32(ctx.target_triple) { + 4 + } else { + 8 + }; + let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) + - meta_ptr_size) + .to_string(); + let meta_addr = ctx.block().add(I64, &object_raw, &meta_offset); + let meta_slot_ptr = ctx.block().inttoptr(I64, &meta_addr); + let meta_loaded = ctx + .block() + .load(if meta_ptr_size == 4 { I32 } else { I64 }, &meta_slot_ptr); + let meta_i64 = if meta_ptr_size == 4 { + ctx.block().zext(I32, &meta_loaded, I64) + } else { + meta_loaded + }; + let has_meta = ctx.block().icmp_ne(I64, &meta_i64, "0"); + ctx.block() + .cond_br(&has_meta, &object_spill_ptr_label, &object_miss_label); + + ctx.current_block = object_spill_ptr_idx; + let meta_ptr = ctx.block().inttoptr(I64, &meta_i64); + let spill_slot_ptr = ctx.block().gep(I64, &meta_ptr, &[(I64, "4")]); + let spill_i64 = ctx.block().load(I64, &spill_slot_ptr); + let has_spill = ctx.block().icmp_ne(I64, &spill_i64, "0"); + // Keep the hot path to one bounds branch without speculatively loading + // through a null spill pointer: ObjectMeta is live here and is a safe + // address for the ignored length load when `spill_i64 == 0`. + let safe_spill_i64 = ctx + .block() + .select(I1, &has_spill, I64, &spill_i64, &meta_i64); + let spill_ptr = ctx.block().inttoptr(I64, &safe_spill_i64); + let spill_len = ctx.block().load(I32, &spill_ptr); + let spill_len_i64 = ctx.block().zext(I32, &spill_len, I64); + let spill_in_bounds = ctx.block().icmp_ult(I64, &object_slot, &spill_len_i64); + let spill_ok = ctx.block().and(I1, &has_spill, &spill_in_bounds); + ctx.block() + .cond_br(&spill_ok, &object_spill_load_label, &object_miss_label); + + ctx.current_block = object_spill_load_idx; + let spill_element_word = ctx.block().add(I64, &object_slot, "1"); + let spill_element_ptr = + ctx.block() + .gep_inbounds(I64, &spill_ptr, &[(I64, &spill_element_word)]); + let spill_raw = ctx.block().load(DOUBLE, &spill_element_ptr); + let spill_value = if coerce_slow_to_number { + ctx.block() + .call(DOUBLE, "js_number_coerce", &[(DOUBLE, &spill_raw)]) + } else { + spill_raw + }; + let spill_end_label = ctx.block().label.clone(); + ctx.block().br(&merge_label); + + ctx.current_block = object_miss_idx; let slow_raw = ctx.block().call( DOUBLE, - "js_dyn_index_get", - &[(DOUBLE, obj_box), (DOUBLE, idx_d)], + "js_packed_arraylike_index_get", + &[(DOUBLE, obj_box), (DOUBLE, idx_d), (PTR, &cache_ref)], ); // In a number context, coerce the (possibly boxed) slow result here so the // merge phi is uniformly a Number and the arithmetic caller skips its own @@ -337,6 +530,9 @@ pub(super) fn lower_inline_dyn_typed_array_get( let slow_end_label = ctx.block().label.clone(); ctx.block().br(&merge_label); + kind_incoming.push((inline_value, inline_end_label)); + kind_incoming.push((spill_value, spill_end_label)); + // ---- final merge: one phi over every per-kind fast end + the slow end ---- ctx.current_block = merge_idx; let mut incoming_refs: Vec<(&str, &str)> = kind_incoming diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index c8a1ff4a19..9684d2533a 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -686,6 +686,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // based on the receiver's NaN-box tag at runtime. Used by IndexGet's // fallback path when codegen can't statically prove the receiver type. module.declare_function("js_dyn_index_get", DOUBLE, &[DOUBLE, DOUBLE]); + // #8655: guarded packed-array / dense Array-subclass read before the + // fully generic dynamic dispatcher. Used by unknown-receiver loop reads. + module.declare_function( + "js_packed_arraylike_index_get", + DOUBLE, + &[DOUBLE, DOUBLE, PTR], + ); // Issue #957: tag-aware dynamic index write. Used by `Expr::IndexUpdate` // codegen to write back the incremented value without rebuilding the // IndexSet dispatch tree. Routes to `js_array_set_index_or_string` for diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index 4afc2b90cb..1fc706fd99 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -63,22 +63,69 @@ impl<'s> RootedIterArray<'s> { /// observes the original receiver object — at its CURRENT address). #[inline(always)] fn receiver(&self) -> f64 { - self.handle.get_nanbox_f64() + array_receiver_value(self.arr()) } #[inline(always)] fn arr(&self) -> *const ArrayHeader { - (self.handle.get_nanbox_u64() & crate::value::POINTER_MASK) as *const ArrayHeader + let rooted = + (self.handle.get_nanbox_u64() & crate::value::POINTER_MASK) as *const ArrayHeader; + let live = clean_arr_ptr(rooted); + if live != rooted { + // Array growth and moving GC leave forwarding stubs behind. Keep + // the root current so subsequent loop iterations do not inspect + // the stub's overwritten length/capacity word as an ArrayHeader. + self.handle.set_nanbox_f64(array_receiver_value(live)); + } + live } #[inline(always)] unsafe fn present(&self, index: usize) -> Option { - present_array_element(array_elements_ptr(self.arr()), index) + let arr = self.arr(); + if index >= (*arr).length as usize { + return None; + } + present_array_element(array_elements_ptr(arr), index) } #[inline(always)] unsafe fn get_or_undefined(&self, index: usize) -> f64 { - array_element_get_value(array_elements_ptr(self.arr()), index) + let arr = self.arr(); + if index >= (*arr).length as usize { + return undefined_value(); + } + array_element_get_value(array_elements_ptr(arr), index) + } +} + +#[cfg(test)] +mod rooted_iter_array_tests { + use super::*; + + #[test] + fn forwarded_array_observes_shrunk_length_during_callback_iteration() { + unsafe { + let mut arr = js_array_alloc(3); + arr = js_array_push_f64(arr, 1.0); + arr = js_array_push_f64(arr, 2.0); + arr = js_array_push_f64(arr, 3.0); + + let scope = crate::gc::RuntimeHandleScope::new(); + let rooted = RootedIterArray::new(&scope, arr); + let mut live_arr = js_array_grow(arr, (*arr).capacity + 1); + assert_ne!(live_arr, arr); + let _removed = js_array_splice(live_arr, 1, 1, ptr::null(), 0, &mut live_arr); + + assert_eq!((*live_arr).length, 2); + assert_eq!(rooted.arr(), clean_arr_ptr(live_arr)); + assert_eq!(rooted.get_or_undefined(1), 3.0); + assert_eq!( + rooted.get_or_undefined(2).to_bits(), + crate::value::TAG_UNDEFINED + ); + assert_eq!(rooted.present(2), None); + } } } diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 5447f78dbf..ace0567d54 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -171,8 +171,9 @@ pub(crate) use indexing::test_swap_array_index_fast_path_invalidated; // points, plus the Array-exotic `length` maintenance the generic OBJECT index // store needs for a `class X extends Array` receiver. pub(crate) use self::subclass::{ - array_object_set_length, is_array_subclass_class_id, is_array_subclass_value, - maintain_array_exotic_length, note_array_subclass_index_write, + array_object_set_length, array_subclass_fast_index_get, array_subclass_fast_length, + is_array_subclass_class_id, is_array_subclass_value, maintain_array_exotic_length, + note_array_subclass_index_write, }; // Issue #1572 — flatten helpers reused by `node_stream::ns_iter_flat_map` // so an `async function*` mapper return is driven through the iterator diff --git a/crates/perry-runtime/src/array/subclass.rs b/crates/perry-runtime/src/array/subclass.rs index 15a1714be7..812805cadc 100644 --- a/crates/perry-runtime/src/array/subclass.rs +++ b/crates/perry-runtime/src/array/subclass.rs @@ -7,12 +7,413 @@ //! Kept out of `generic.rs` so that module stays under the file-size gate. use std::ptr; +use std::sync::atomic::{AtomicU64, Ordering}; use super::generic::{al_get, al_length, nanbox_arr}; use crate::array::{js_array_alloc_with_length, note_array_slot, ArrayHeader}; use crate::object::ObjectHeader; use crate::value::JSValue; +// #8655: Array-subclass instances use ordinary ObjectHeader property slots, +// but their hot numeric reads have a much stronger invariant than a generic +// object lookup can exploit: `push` appends the own keys `"0"`, `"1"`, ... in +// order, and every structural/descriptor/prototype mutation publishes a new +// ShapeId before it becomes observable. Cache that dense prefix per exact +// (class, shape) pair so a stable `sub[i]` is two field-slot reads (`length` +// and the element) instead of number -> String allocation + hash lookup. +// +// The cache stores no heap pointer, so it is not a GC root. ShapeIds are never +// reused, and the class id prevents an unrelated class with the same ordered +// keys from borrowing the Array-subclass proof. +const DENSE_SUBCLASS_CACHE_SLOTS: usize = 256; + +struct DenseSubclassCacheEntry { + /// Even while stable, odd while a colliding writer publishes a payload. + sequence: AtomicU64, + /// `(class_id << 32) | shape_id`. + key: AtomicU64, + /// `(length_slot << 32) | element_base`. + slots: AtomicU64, + /// `(live_inline_slots << 32) | dense_prefix_len`. + bounds: AtomicU64, +} + +impl DenseSubclassCacheEntry { + const fn new() -> Self { + Self { + sequence: AtomicU64::new(0), + key: AtomicU64::new(0), + slots: AtomicU64::new(0), + bounds: AtomicU64::new(0), + } + } +} + +static DENSE_SUBCLASS_CACHE: [DenseSubclassCacheEntry; DENSE_SUBCLASS_CACHE_SLOTS] = + [const { DenseSubclassCacheEntry::new() }; DENSE_SUBCLASS_CACHE_SLOTS]; + +#[derive(Clone, Copy)] +struct DenseSubclassLayout { + length_slot: u32, + element_base: u32, + dense_prefix_len: u32, + live_inline_slots: u32, +} + +#[inline(always)] +fn dense_cache_key(class_id: u32, shape_id: u32) -> u64 { + ((class_id as u64) << 32) | shape_id as u64 +} + +#[inline(always)] +fn dense_cache_entry(key: u64) -> &'static DenseSubclassCacheEntry { + let mixed = key ^ (key >> 33) ^ (key >> 17); + &DENSE_SUBCLASS_CACHE[mixed as usize & (DENSE_SUBCLASS_CACHE_SLOTS - 1)] +} + +#[inline] +fn cached_dense_layout(key: u64) -> Option { + let entry = dense_cache_entry(key); + let sequence = entry.sequence.load(Ordering::Acquire); + if sequence & 1 != 0 || entry.key.load(Ordering::Relaxed) != key { + return None; + } + let slots = entry.slots.load(Ordering::Relaxed); + let bounds = entry.bounds.load(Ordering::Relaxed); + // Recheck the seqlock before interpreting either word so readers never + // combine payloads from two colliding publishers. + if entry.sequence.load(Ordering::Acquire) != sequence { + return None; + } + Some(DenseSubclassLayout { + length_slot: (slots >> 32) as u32, + element_base: slots as u32, + dense_prefix_len: bounds as u32, + live_inline_slots: (bounds >> 32) as u32, + }) +} + +#[inline] +fn publish_dense_layout(key: u64, layout: DenseSubclassLayout) { + let entry = dense_cache_entry(key); + let mut sequence = entry.sequence.load(Ordering::Relaxed); + loop { + if sequence & 1 != 0 { + std::hint::spin_loop(); + sequence = entry.sequence.load(Ordering::Relaxed); + continue; + } + match entry.sequence.compare_exchange_weak( + sequence, + sequence.wrapping_add(1), + Ordering::Acquire, + Ordering::Relaxed, + ) { + Ok(_) => break, + Err(observed) => sequence = observed, + } + } + entry.slots.store( + ((layout.length_slot as u64) << 32) | layout.element_base as u64, + Ordering::Relaxed, + ); + entry.bounds.store( + ((layout.live_inline_slots as u64) << 32) | layout.dense_prefix_len as u64, + Ordering::Relaxed, + ); + entry.key.store(key, Ordering::Relaxed); + entry + .sequence + .store(sequence.wrapping_add(2), Ordering::Release); +} + +fn decimal_u32<'a>(mut value: u32, buf: &'a mut [u8; 10]) -> &'a [u8] { + let mut start = buf.len(); + loop { + start -= 1; + buf[start] = b'0' + (value % 10) as u8; + value /= 10; + if value == 0 { + return &buf[start..]; + } + } +} + +/// Establish the dense-prefix invariant once for an exact semantic ShapeId. +/// This path may scan the keys array, but it runs only on a cache miss. It +/// allocates nothing and keeps no address into the moving heap. +unsafe fn build_dense_layout(obj: *const ObjectHeader) -> Option { + let class_id = (*obj).class_id; + if class_id == 0 + || !is_array_subclass_class_id(class_id) + || crate::object::prototype_chain::object_has_prototype_override(obj as usize) + { + return None; + } + let shape = crate::object::shapes::object_shape_descriptor(obj)?; + if shape.object_kind != crate::object::shapes::ShapeObjectKind::Ordinary { + return None; + } + let keys = shape.keys as usize as *const crate::array::ArrayHeader; + if keys.is_null() { + return None; + } + let (key_slots, physical_len) = crate::object::keys_array_dense_slots(keys); + let key_count = (shape.logical_key_count as usize).min(physical_len); + if key_slots.is_null() || key_count == 0 { + return None; + } + + let mut length_slot = None; + let mut element_base = None; + for slot in 0..key_count { + let stored = JSValue::from_bits((*key_slots.add(slot)).to_bits()); + if length_slot.is_none() && crate::string::js_string_key_matches_bytes(stored, b"length") { + length_slot = Some(slot as u32); + } + if element_base.is_none() && crate::string::js_string_key_matches_bytes(stored, b"0") { + element_base = Some(slot as u32); + } + } + let length_slot = length_slot?; + // A length-only empty subclass has no `"0"` key yet. Cache its length + // read, while leaving the numeric prefix empty so every index side-exits. + let has_element_zero = element_base.is_some(); + let element_base = element_base.unwrap_or(0); + let mut dense_prefix_len = 0u32; + if has_element_zero { + while (element_base as usize + dense_prefix_len as usize) < key_count { + let slot = element_base as usize + dense_prefix_len as usize; + let stored = JSValue::from_bits((*key_slots.add(slot)).to_bits()); + let mut decimal = [0u8; 10]; + if !crate::string::js_string_key_matches_bytes( + stored, + decimal_u32(dense_prefix_len, &mut decimal), + ) { + break; + } + dense_prefix_len += 1; + } + } + + // Class construction installs descriptors for unrelated methods, so the + // object-wide descriptor bit is too coarse for this proof. Data + // descriptors do not alter [[Get]]; reject only accessors for the slots the + // fast path will read. Descriptor mutations publish a new semantic + // ShapeId, which makes this one-time scan part of the exact-shape proof. + if crate::object::object_has_descriptors(obj as usize) { + if crate::object::get_accessor_descriptor(obj as usize, "length").is_some() { + return None; + } + for index in 0..dense_prefix_len { + let mut decimal = [0u8; 10]; + let bytes = decimal_u32(index, &mut decimal); + // `decimal_u32` emits ASCII digits only. + let key = unsafe { std::str::from_utf8_unchecked(bytes) }; + if crate::object::get_accessor_descriptor(obj as usize, key).is_some() { + return None; + } + } + } + + Some(DenseSubclassLayout { + length_slot, + element_base, + dense_prefix_len, + live_inline_slots: shape.live_inline_slot_count, + }) +} + +/// Resolve a live Array-subclass object and its cached dense layout. Every +/// rejected brand, forwarding, descriptor, hole, or prototype case returns +/// `None`; callers retain their existing fully generic fallback. +#[inline] +fn dense_layout_for_value(value: f64) -> Option<(*const ObjectHeader, DenseSubclassLayout)> { + let js = JSValue::from_bits(value.to_bits()); + if !js.is_pointer() { + return None; + } + let obj = js.as_pointer::(); + if obj.is_null() || !crate::object::is_valid_obj_ptr(obj.cast::()) { + return None; + } + let header = unsafe { crate::value::addr_class::try_read_gc_header(obj as usize)? }; + if header.obj_type != crate::gc::GC_TYPE_OBJECT + || header.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + { + return None; + } + let (class_id, shape_id) = unsafe { ((*obj).class_id, (*obj).parent_class_id) }; + let key = dense_cache_key(class_id, shape_id); + let layout = cached_dense_layout(key).or_else(|| { + let layout = unsafe { build_dense_layout(obj) }?; + publish_dense_layout(key, layout); + Some(layout) + })?; + Some((obj, layout)) +} + +#[inline] +fn layout_length_value(obj: *const ObjectHeader, layout: DenseSubclassLayout) -> JSValue { + layout_field_value(obj, layout.length_slot, layout.live_inline_slots) +} + +/// Read a slot already proved live by an exact ShapeId. Wide dynamic objects +/// keep post-inline fields in the object-owned spill Array. Reaching that +/// buffer directly is the essential #8655 hot path: the general field helper +/// reclassifies the owner and probes the overflow abstraction for every ECS +/// element even though the shape proof already established all of it. +#[inline(always)] +fn layout_field_value(obj: *const ObjectHeader, slot: u32, live_inline_slots: u32) -> JSValue { + unsafe { + if slot < live_inline_slots { + let fields = + (obj as *const u8).add(std::mem::size_of::()) as *const JSValue; + return *fields.add(slot as usize); + } + + if crate::object::object_spill_enabled() { + let meta = (*obj).meta; + if !meta.is_null() { + let spill = (*meta).spill as *const ArrayHeader; + if !spill.is_null() && slot < (*spill).length { + let elements = + (spill as *const u8).add(std::mem::size_of::()) as *const u64; + return JSValue::from_bits(*elements.add(slot as usize)); + } + } + return JSValue::undefined(); + } + + crate::object::overflow_get(obj as usize, slot as usize) + .map(JSValue::from_bits) + .unwrap_or_else(JSValue::undefined) + } +} + +fn nonnegative_u32_length(value: JSValue) -> Option { + let number = if value.is_int32() { + value.as_int32() as f64 + } else if value.is_number() { + value.as_number() + } else { + return None; + }; + (number.is_finite() && number >= 0.0 && number.fract() == 0.0 && number <= u32::MAX as f64) + .then_some(number as u32) +} + +/// Fast own `length` read for an object-backed Array subclass. Returning the +/// stored JSValue (rather than coercing it) preserves source property-read +/// semantics; descriptor/prototype-divergent shapes decline above. +#[inline] +pub(crate) fn array_subclass_fast_length(value: f64) -> Option { + let (obj, layout) = dense_layout_for_value(value)?; + Some(f64::from_bits(layout_length_value(obj, layout).bits())) +} + +/// Guarded dense numeric read for an object-backed Array subclass. The live +/// `length` value is checked on every hit, while `dense_prefix_len` caps the +/// proof when a length-only grow created holes without changing the shape. +#[inline] +pub(crate) fn array_subclass_fast_index_get(value: f64, index: u32) -> Option { + let (obj, layout) = dense_layout_for_value(value)?; + dense_index_get_with_layout(obj, layout, index) +} + +#[inline(always)] +fn dense_index_get_with_layout( + obj: *const ObjectHeader, + layout: DenseSubclassLayout, + index: u32, +) -> Option { + let length = nonnegative_u32_length(layout_length_value(obj, layout))?; + if index >= length || index >= layout.dense_prefix_len { + return None; + } + let slot = layout.element_base.checked_add(index)?; + let value = layout_field_value(obj, slot, layout.live_inline_slots); + Some(f64::from_bits(value.bits())) +} + +fn canonical_u32_index(value: f64) -> Option { + let js = JSValue::from_bits(value.to_bits()); + if js.is_int32() { + return (js.as_int32() >= 0).then_some(js.as_int32() as u32); + } + (js.is_number() + && value.is_finite() + && value >= 0.0 + && value.fract() == 0.0 + && value <= (u32::MAX - 1) as f64) + .then_some(value as u32) +} + +/// Unknown-receiver numeric read used by codegen's guarded typed-array miss +/// block. Stable real arrays and Array subclasses terminate here; every other +/// receiver/key keeps the established tag-aware dispatcher as a cold side +/// exit. Keeping that call behind this ABI boundary removes `js_dyn_index_get` +/// from the emitted hot-loop artifact without weakening its semantics. +#[no_mangle] +/// The five optional IC words are +/// scalar layout facts, never heap pointers: +/// `(class_id, ShapeId)`, length slot, element base, dense prefix, inline bound. +/// The emitted hit path reloads the live object/meta/spill pointers, so moving +/// GC never has to trace or rewrite this cache. +pub extern "C" fn js_packed_arraylike_index_get(receiver: f64, index: f64, cache: *mut u64) -> f64 { + if let Some(index_u32) = canonical_u32_index(index) { + let js = JSValue::from_bits(receiver.to_bits()); + if js.is_pointer() { + let raw = js.as_pointer::(); + if let Some(header) = + unsafe { crate::value::addr_class::try_read_gc_header(raw as usize) } + { + if matches!( + header.obj_type, + crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY + ) { + return crate::array::js_array_get_f64( + raw as *const crate::array::ArrayHeader, + index_u32, + ); + } + if header.obj_type == crate::gc::GC_TYPE_OBJECT { + if let Some((obj, layout)) = dense_layout_for_value(receiver) { + // The codegen hit path reads length inline and wide + // slots through ObjectMeta::spill. Decline to prime in + // the legacy side-table mode or for a pathological + // layout whose length itself spilled. + if !cache.is_null() + && crate::object::object_spill_enabled() + && layout.length_slot < layout.live_inline_slots + { + unsafe { + cache.add(1).write(layout.length_slot as u64); + cache.add(2).write(layout.element_base as u64); + cache.add(3).write(layout.dense_prefix_len as u64); + cache.add(4).write(layout.live_inline_slots as u64); + cache.write(dense_cache_key( + (*obj).class_id, + (*obj).parent_class_id, + )); + } + } + if let Some(value) = dense_index_get_with_layout(obj, layout, index_u32) { + return value; + } + } + } + } + } + } + crate::value::js_dyn_index_get(receiver, index) +} + +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_PACKED_ARRAYLIKE_INDEX_GET: extern "C" fn(f64, f64, *mut u64) -> f64 = + js_packed_arraylike_index_get; + /// True when `class_id` is a user class that extends `Array` (the reserved /// parent id `0xFFFF0024` appears in its class chain), i.e. `class X extends /// Array`. Such instances are plain `ObjectHeader`s, so the array-like engines diff --git a/crates/perry-runtime/src/array/subclass_tests.rs b/crates/perry-runtime/src/array/subclass_tests.rs index 815411e0b9..980cf338aa 100644 --- a/crates/perry-runtime/src/array/subclass_tests.rs +++ b/crates/perry-runtime/src/array/subclass_tests.rs @@ -18,7 +18,8 @@ //! vacuous — which is exactly the failure mode the module is written to avoid. use super::subclass::{ - array_object_receiver, is_array_subclass_class_id, raw_receiver_is_heap_object, + array_object_receiver, array_subclass_fast_index_get, array_subclass_fast_length, + is_array_subclass_class_id, js_packed_arraylike_index_get, raw_receiver_is_heap_object, }; use crate::array::{clean_arr_ptr, js_array_alloc, ArrayHeader}; use crate::object::{js_object_alloc, ObjectHeader}; @@ -164,3 +165,54 @@ fn array_object_receiver_is_safe_for_non_pointers_and_handle_band_ids() { assert!(array_object_receiver(hdr).is_none(), "id {id:#x}"); } } + +/// #8655: the object-backed representation still stores dense Array-subclass +/// elements in ordinary property slots. Pin the shape proof and, importantly, +/// its side exit after a structural mutation. +#[test] +fn dense_array_subclass_reads_slots_until_its_shape_changes() { + let class_id = 0x0074_8655; + crate::object::js_register_class_parent(class_id, CLASS_ID_ARRAY); + let obj = js_object_alloc(class_id, 2); + assert!(!obj.is_null()); + let receiver = crate::value::js_nanbox_pointer(obj as i64); + crate::node_stream::js_array_subclass_init(receiver, 0.0); + + for (index, value) in [11.0, 22.0, 33.0].into_iter().enumerate() { + crate::object::js_object_set_index_polymorphic(obj as i64, index as f64, value); + } + + assert_eq!(array_subclass_fast_length(receiver), Some(3.0)); + assert_eq!(array_subclass_fast_index_get(receiver, 1), Some(22.0)); + assert_eq!( + js_packed_arraylike_index_get(receiver, 2.0, std::ptr::null_mut()), + 33.0 + ); + + crate::object::js_object_delete_dynamic(obj, 1.0); + assert_eq!( + array_subclass_fast_index_get(receiver, 1), + None, + "deleting an indexed property must mint a shape whose dense proof side-exits" + ); + assert_eq!( + js_packed_arraylike_index_get(receiver, 1.0, std::ptr::null_mut()).to_bits(), + crate::value::TAG_UNDEFINED, + "the wrapper must preserve the generic hole result" + ); +} + +#[test] +fn dense_array_subclass_guard_rejects_other_object_brands() { + let obj = js_object_alloc(0x0074_8656, 2); + let receiver = crate::value::js_nanbox_pointer(obj as i64); + let key = crate::string::js_string_from_bytes(b"0".as_ptr(), 1); + crate::object::js_object_set_field_by_name(obj, key, 17.0); + + assert_eq!(array_subclass_fast_length(receiver), None); + assert_eq!(array_subclass_fast_index_get(receiver, 0), None); + assert_eq!( + js_packed_arraylike_index_get(receiver, 0.0, std::ptr::null_mut()), + 17.0 + ); +} diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 28b2d034d8..fb6d67850d 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -145,11 +145,11 @@ mod regex_proto_thunks; // names they use (the rest stay internal to `spill`). mod spill; pub(crate) use spill::{ - learned_inline_field_count, learned_inline_fields_hot_addr, overflow_get, overflow_set, - reserve_object_spill, + learned_inline_field_count, learned_inline_fields_hot_addr, object_spill_enabled, overflow_get, + overflow_set, reserve_object_spill, }; #[cfg(test)] -use spill::{object_spill_enabled, spill_capable_owner, spill_get, SPILL_MAX_FIELD_INDEX}; +use spill::{spill_capable_owner, spill_get, SPILL_MAX_FIELD_INDEX}; #[cfg(test)] pub(crate) use spill::{test_set_spill_safepoint_hook, SpillSafepointHook}; mod string_proto_thunks; diff --git a/crates/perry-runtime/src/object/polymorphic_index.rs b/crates/perry-runtime/src/object/polymorphic_index.rs index c1cedb987a..b54813cdc4 100644 --- a/crates/perry-runtime/src/object/polymorphic_index.rs +++ b/crates/perry-runtime/src/object/polymorphic_index.rs @@ -291,7 +291,16 @@ pub extern "C" fn js_object_get_index_polymorphic(obj_handle: i64, idx: f64) -> return unsafe { rooted_property_key_get(raw, idx) }; } } - if gc_type == crate::gc::GC_TYPE_OBJECT || gc_type == crate::gc::GC_TYPE_CLOSURE { + if gc_type == crate::gc::GC_TYPE_OBJECT { + if let Some(index) = numeric_key_u32_index(idx) { + let receiver = f64::from_bits(crate::value::POINTER_TAG | raw); + if let Some(value) = crate::array::array_subclass_fast_index_get(receiver, index) { + return value; + } + } + return unsafe { rooted_property_key_get(raw, idx) }; + } + if gc_type == crate::gc::GC_TYPE_CLOSURE { return unsafe { rooted_property_key_get(raw, idx) }; } if crate::set::is_registered_set(raw as usize) || crate::map::is_registered_map(raw as usize) { diff --git a/crates/perry-runtime/src/value/dynamic_object.rs b/crates/perry-runtime/src/value/dynamic_object.rs index f49cf8dc40..b544879a71 100644 --- a/crates/perry-runtime/src/value/dynamic_object.rs +++ b/crates/perry-runtime/src/value/dynamic_object.rs @@ -275,6 +275,10 @@ pub extern "C" fn js_value_length_property_f64(value: f64) -> f64 { return crate::string::js_string_length(string) as f64; } + if let Some(length) = crate::array::array_subclass_fast_length(value) { + return length; + } + unsafe { js_dynamic_object_get_property(value, b"length".as_ptr() as *const i8, 6) } } diff --git a/crates/perry/src/commands/compile/size_report.rs b/crates/perry/src/commands/compile/size_report.rs index 9a7874878a..00a2cda757 100644 --- a/crates/perry/src/commands/compile/size_report.rs +++ b/crates/perry/src/commands/compile/size_report.rs @@ -20,6 +20,11 @@ //! made it into the final link — code/data attribution, duplicate function //! bodies, duplicate crate instances, generic-monomorphization cost, and a //! few named cost patterns (panics, `Debug`/`Display` formatting, vtables). +//! The duplicate-crate-instance check also screens out a real false-positive +//! class: a coincidental name collision with a crate `std` itself vendors +//! internally for backtrace support (see `STD_INTERNAL_BACKTRACE_CRATES`) +//! is not a build duplication Perry produced, so it is excluded from the +//! finding and reported separately instead. use std::collections::BTreeMap; use std::fs; @@ -83,6 +88,18 @@ struct DuplicateCrateInstance { total_bytes: u64, } +/// A crate instance excluded from `duplicate_crate_instances` because its +/// symbols look like Rust's own standard library's internal, toolchain- +/// baked copy of that crate name (see `STD_INTERNAL_BACKTRACE_CRATES`) — +/// not a real second build Perry's own compilation produced. +#[derive(Serialize)] +struct ExcludedStdInternalCopy { + crate_name: String, + hash: String, + bytes: u64, + symbol_count: usize, +} + #[derive(Serialize)] struct PatternTotal { name: &'static str, @@ -111,6 +128,7 @@ struct SizeReport { generic_families: Vec, duplicate_bodies: Vec, duplicate_crate_instances: Vec, + std_internal_excluded: Vec, patterns: Vec, suggestions: Vec, } @@ -277,6 +295,10 @@ fn build_report(exe_path: &Path) -> anyhow::Result { let mut family_totals: BTreeMap<(String, String), (usize, u64)> = BTreeMap::new(); let mut crate_hashes: BTreeMap> = BTreeMap::new(); let mut crate_hash_bytes: BTreeMap<(String, String), u64> = BTreeMap::new(); + // Only populated for `STD_INTERNAL_BACKTRACE_CRATES` — the full symbol + // list per (crate, hash) is only needed to classify those few crate + // names, so this stays cheap regardless of binary size. + let mut crate_hash_symbols: BTreeMap<(String, String), Vec> = BTreeMap::new(); let mut body_bytes: BTreeMap<&[u8], Vec<(String, u64)>> = BTreeMap::new(); // exact bytes -> [(symbol, size)] let mut pattern_totals: BTreeMap<&'static str, (u64, usize)> = BTreeMap::new(); @@ -303,6 +325,12 @@ fn build_report(exe_path: &Path) -> anyhow::Result { *crate_hash_bytes .entry((crate_name.clone(), hash.clone())) .or_insert(0) += sym.size; + if STD_INTERNAL_BACKTRACE_CRATES.contains(&crate_name.as_str()) { + crate_hash_symbols + .entry((crate_name.clone(), hash.clone())) + .or_default() + .push(demangled.clone()); + } } if crate_name != "native/other" { @@ -380,6 +408,45 @@ fn build_report(exe_path: &Path) -> anyhow::Result { duplicate_bodies.sort_by_key(|a| std::cmp::Reverse(a.wasted_bytes)); duplicate_bodies.truncate(REPORT_TOP_DUPLICATES); + // Exclude hash-variants that look like `std`'s own internal, toolchain- + // baked copy of a crate it vendors for backtrace support, before + // deciding whether a crate name has more than one REAL instance — + // otherwise a coincidental name collision with a std-internal component + // (confirmed for `gimli`: std's own DWARF unwinder) gets reported as a + // fixable build duplication it is not. + let mut std_internal_excluded: Vec = Vec::new(); + for (crate_name, hashes) in crate_hashes.iter_mut() { + if !STD_INTERNAL_BACKTRACE_CRATES.contains(&crate_name.as_str()) { + continue; + } + let suspect: Vec = hashes + .iter() + .filter(|h| { + crate_hash_symbols + .get(&(crate_name.clone(), (*h).clone())) + .is_some_and(|syms| looks_like_std_internal_backtrace_copy(crate_name, syms)) + }) + .cloned() + .collect(); + for hash in suspect { + hashes.remove(&hash); + std_internal_excluded.push(ExcludedStdInternalCopy { + crate_name: crate_name.clone(), + bytes: crate_hash_bytes + .get(&(crate_name.clone(), hash.clone())) + .copied() + .unwrap_or(0), + symbol_count: crate_hash_symbols + .get(&(crate_name.clone(), hash.clone())) + .map(Vec::len) + .unwrap_or(0), + hash, + }); + } + } + crate_hashes.retain(|_, hashes| !hashes.is_empty()); + std_internal_excluded.sort_by_key(|e| std::cmp::Reverse(e.bytes)); + let mut duplicate_crate_instances: Vec = crate_hashes .into_iter() .filter(|(_, hashes)| hashes.len() > 1) @@ -439,6 +506,7 @@ fn build_report(exe_path: &Path) -> anyhow::Result { generic_families, duplicate_bodies, duplicate_crate_instances, + std_internal_excluded, patterns, suggestions, }) @@ -569,6 +637,92 @@ const PATTERNS: &[PatternMatcher] = &[ }), ]; +/// Crate names Rust's own standard library vendors internally for +/// `std::backtrace`/panic-unwinding support (`library/std/Cargo.toml` in +/// rust-lang/rust). A build can end up with a second, unrelated instance of +/// one of these names: the real Cargo-resolved dependency (if the program +/// also uses it directly, e.g. for its own symbolication), plus a +/// completely separate copy baked into the prebuilt `std` rlib shipped with +/// the toolchain. That second copy is invisible to `cargo tree`/the unit +/// graph (it was never a resolvable dependency of THIS build at all — Rust +/// itself's release process built and shipped it), so there is nothing +/// Perry's build can deduplicate. +const STD_INTERNAL_BACKTRACE_CRATES: &[&str] = &[ + "gimli", + "addr2line", + "miniz_oxide", + "object", + "rustc_demangle", +]; + +/// Symbol-path markers specific to DWARF CFI/EH-frame unwinding — the +/// narrow slice of `gimli`'s API surface `std`'s own unwinder uses +/// (confirmed by demangling real symbols under a suspect hash: every +/// gimli-rooted one was `gimli::read::cfi::{EhFrame, +/// CommonInformationEntry, Augmentation, PartialFrameDescriptionEntry, +/// UnwindSection, UnwindTable, ...}` or the `common`/`eh_walker` types that +/// exist only to support that path). +const CFI_MARKERS: &[&str] = &[ + "::cfi::", + "EhFrame", + "CommonInformationEntry", + "PartialFrameDescriptionEntry", + "UnwindSection", + "UnwindTable", + "UnwindContext", + "FrameDescriptionEntry", + "Augmentation", + "CfaRule", + "RegisterRule", + "EhFrameOffset", + "eh_walker", +]; + +/// Symbol-path markers specific to ordinary DWARF debug-info reading — the +/// surface a real application dependency on `gimli` actually uses +/// (confirmed against the OTHER, real hash in the same binary: every +/// symbol was `gimli::read::abbrev::Abbreviation`). `std`'s CFI-only +/// internal copy never touches this surface, so any of these appearing +/// under a hash rules out "this is std's internal copy" even though that +/// hash's OWN shared reader/utility code (`EndianSlice`, `Reader:: +/// read_uleb128`, …) has no CFI marker of its own. +const DEBUG_INFO_MARKERS: &[&str] = &[ + "::abbrev::", + "Abbreviation", + "::line::", + "LineProgram", + "::rnglists::", + "::loclists::", + "::unit::", + "UnitHeader", + "::dwarf::", + "DebugAbbrev", + "DebugInfo", + "DebugLine", + "DebugStr", + "DebugRanges", +]; + +/// Whether a (crate, hash) group's symbols look like `std`'s own internal +/// backtrace-support copy rather than a real second build of the +/// application's dependency: at least one CFI-specific marker present, and +/// zero debug-info-specific markers. Requiring "at least one" rather than +/// "all" matters because the CFI path's own shared reader/utility code +/// (`EndianSlice`, `Reader::read_uleb128`, error types, …) carries no CFI- +/// specific name of its own but is compiled alongside it in the same unit. +fn looks_like_std_internal_backtrace_copy(crate_name: &str, symbols: &[String]) -> bool { + if !STD_INTERNAL_BACKTRACE_CRATES.contains(&crate_name) || symbols.is_empty() { + return false; + } + let has_cfi_marker = symbols + .iter() + .any(|s| CFI_MARKERS.iter().any(|marker| s.contains(marker))); + let has_debug_info_marker = symbols + .iter() + .any(|s| DEBUG_INFO_MARKERS.iter().any(|marker| s.contains(marker))); + has_cfi_marker && !has_debug_info_marker +} + /// Demangle a Rust symbol name. `rustc_demangle` returns non-Rust input /// unchanged — the normal case for libc/system symbols — and `crate_of` /// below buckets those as `native/other`. @@ -736,6 +890,28 @@ fn render_markdown(report: &SizeReport) -> String { } } + if !report.std_internal_excluded.is_empty() { + out.push_str("\n## Excluded: std-internal copies\n\n"); + out.push_str( + "Crate instances that looked like a duplicate above but were excluded: their \ + symbols matched the narrow API surface Rust's own standard library uses \ + internally for `std::backtrace`/panic-unwinding support (e.g. `gimli`'s DWARF \ + CFI/EH-frame reader). That copy is baked into the prebuilt `std` shipped with the \ + toolchain — it was never a resolvable dependency of this build, so there is \ + nothing here for Perry (or Cargo) to deduplicate; the name collision alone would \ + otherwise misreport it as a fixable duplicate.\n\n", + ); + out.push_str("| Bytes | Symbols | Crate |\n|---|---|---|\n"); + for excluded in &report.std_internal_excluded { + out.push_str(&format!( + "| {} | {} | `{}` |\n", + human_bytes(excluded.bytes), + excluded.symbol_count, + excluded.crate_name, + )); + } + } + if !report.generic_families.is_empty() { out.push_str("\n## Generic monomorphization\n\n"); out.push_str("| Total | Instantiations | Crate | Family |\n|---|---|---|---|\n"); @@ -809,6 +985,67 @@ fn human_bytes(bytes: u64) -> String { mod tests { use super::*; + #[test] + fn std_internal_backtrace_copy_detected_from_real_cfi_symbols() { + // Real demangled symbols pulled from a compiled binary's second + // gimli instance — all gimli::read::cfi::*. + let symbols = vec![ + "> as gimli::read::cfi::UnwindSection>>::cie_from_offset".to_string(), + ", usize>>::parse".to_string(), + "::parse".to_string(), + ]; + assert!(looks_like_std_internal_backtrace_copy("gimli", &symbols)); + } + + #[test] + fn std_internal_backtrace_copy_not_detected_for_real_dependency_usage() { + // A real application dependency on gimli reads debug info + // (abbrev/line/rnglists), not CFI/EH-frame data. + let symbols = vec![ + "gimli::read::abbrev::Abbreviation::new_internal".to_string(), + "gimli::read::line::LineProgram::header".to_string(), + ]; + assert!(!looks_like_std_internal_backtrace_copy("gimli", &symbols)); + } + + #[test] + fn std_internal_backtrace_copy_not_detected_outside_the_known_crate_list() { + // A crate outside STD_INTERNAL_BACKTRACE_CRATES never gets excluded, + // even if its symbols happen to mention "EhFrame" in passing. + let symbols = vec!["some_crate::EhFrame::wrapper".to_string()]; + assert!(!looks_like_std_internal_backtrace_copy( + "some_crate", + &symbols + )); + } + + #[test] + fn std_internal_backtrace_copy_tolerates_shared_reader_utility_symbols() { + // Real finding: the CFI build's own shared reader/utility code + // (EndianSlice, Reader::read_uleb128, the CapacityFull error type) + // carries no CFI-specific name of its own, but is compiled + // alongside gimli::read::cfi in the SAME hash group — "at least one + // CFI marker" (not "all symbols") is what correctly includes it. + let symbols = vec![ + "gimli::read::cfi::EhFrame::parse".to_string(), + " as gimli::read::reader::Reader>::read_uleb128".to_string(), + "::fmt".to_string(), + ]; + assert!(looks_like_std_internal_backtrace_copy("gimli", &symbols)); + } + + #[test] + fn std_internal_backtrace_copy_a_single_debug_info_marker_vetoes_exclusion() { + // A debug-info marker anywhere in the group is enough to keep it as + // a (possibly real) finding rather than excluding it, even + // alongside a CFI-looking symbol. + let symbols = vec![ + "gimli::read::cfi::EhFrame::parse".to_string(), + "gimli::read::abbrev::Abbreviation::new_internal".to_string(), + ]; + assert!(!looks_like_std_internal_backtrace_copy("gimli", &symbols)); + } + #[test] fn crate_of_extracts_the_first_path_segment() { assert_eq!( diff --git a/crates/perry/tests/issue_8655_array_subclass_indexing.rs b/crates/perry/tests/issue_8655_array_subclass_indexing.rs new file mode 100644 index 0000000000..0d52c50420 --- /dev/null +++ b/crates/perry/tests/issue_8655_array_subclass_indexing.rs @@ -0,0 +1,184 @@ +//! Regression coverage for #8655. Numeric indexing on an object-backed +//! `class X extends Array` used to stringify every index and perform a generic +//! property lookup inside the Wolf ECS inner loop, leaving the native binary +//! 191x behind Node in the issue report. +//! +//! The runtime now caches the exact dense property-slot layout by class and +//! semantic ShapeId. These tests pin both halves of the contract: emitted hot +//! loops call the guarded packed-arraylike helper instead of +//! `js_dyn_index_get`, and every shape the guard must reject keeps ordinary JS +//! semantics. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str, keep_ir: bool) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let mut cmd = Command::new(perry_bin()); + cmd.current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1"); + if keep_ir { + cmd.env("PERRY_LLVM_KEEP_IR", "1"); + } + let compile = cmd.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) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn issue_repro_source() -> &'static str { + r#" +class Query extends Array { archetypes = this; } +class Archetype extends Array { entities = this; } + +const query = new Query(); +const archetype = new Archetype(); +for (let i = 0; i < 1000; i++) archetype.push(i); +query.push(archetype); +const values = new Uint32Array(1000); + +function system(values: Uint32Array) { + for (let i = 0; i < query.length; i++) { + const current = query[i]; + for (let j = 0; j < current.length; j++) values[current[j]] += 1; + } +} + +for (let i = 0; i < 4; i++) system(values); +console.log(values[0] + "," + values[999]); +"# +} + +fn function_ir<'a>(ir: &'a str, function_fragment: &str) -> &'a str { + let start = ir + .find(function_fragment) + .unwrap_or_else(|| panic!("missing function `{function_fragment}` in emitted IR")); + let body_start = ir[..start] + .rfind("\ndefine ") + .unwrap_or_else(|| panic!("missing definition before `{function_fragment}`")); + let tail = &ir[body_start + 1..]; + let end = tail + .find("\n}\n") + .unwrap_or_else(|| panic!("unterminated definition for `{function_fragment}`")); + &tail[..end + 2] +} + +#[test] +fn wolf_ecs_loop_has_no_generic_dynamic_index_get() { + let dir = tempfile::tempdir().expect("tempdir"); + let (_bin, stderr) = compile(dir.path(), issue_repro_source(), true); + let ll_path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + let ir = std::fs::read_to_string(&ll_path).expect("read kept LLVM IR"); + let _ = std::fs::remove_file(&ll_path); + let system = function_ir(&ir, "__system(double"); + + assert!( + system.contains("call double @js_packed_arraylike_index_get("), + "the unknown Array-subclass receiver must use the guarded packed-arraylike read" + ); + assert!( + !system.contains("call double @js_dyn_index_get("), + "the Wolf ECS hot loop must not call the generic dynamic index dispatcher" + ); + assert!( + !system.contains("call i64 @js_string_from_bytes("), + "the hot loop must not construct numeric or length property keys" + ); +} + +#[test] +fn guarded_arraylike_reads_preserve_side_exit_semantics() { + let dir = tempfile::tempdir().expect("tempdir"); + let source = r#" +class Dense extends Array {} + +function readParam(a: any): string { + let out = ""; + for (let i = 0; i < a.length; i++) out += String(a[i]) + ";"; + return out; +} + +const captured: any = new Dense(); +captured.push(10); captured.push(20); captured.push(30); +function readCaptured(): string { + let out = ""; + for (let i = 0; i < captured.length; i++) out += String(captured[i]) + ";"; + return out; +} +console.log("dense=" + readCaptured() + "|" + readParam(captured)); + +// Installing an own accessor after warming the exact old shape must retire +// its cached layout and invoke the getter. +Object.defineProperty(captured, "1", { get() { return 41; }, configurable: true }); +console.log("descriptor=" + readCaptured()); + +// A hole must fall through an indexed custom prototype accessor. +const hole: any = new Dense(); +hole.push(1); hole.push(2); hole.push(3); +delete hole[1]; +const proto: any = {}; +Object.defineProperty(proto, "1", { get() { return 77; }, configurable: true }); +Object.setPrototypeOf(hole, proto); +console.log("hole-proto=" + readParam(hole)); + +// A Proxy must remain wholly observable, including numeric get traps. +const proxied: any = new Proxy(captured, { + get(target: any, key: any) { + if (String(key) === "2") return 99; + return target[key]; + } +}); +console.log("proxy=" + readParam(proxied)); + +// The same unknown-receiver helper also sees real Arrays. A transition from +// packed numeric to mixed elements must read the live boxed value. +const ordinary: any[] = [4, 5, 6]; +console.log("array-before=" + readParam(ordinary)); +ordinary[1] = "mixed"; +console.log("array-after=" + readParam(ordinary)); +"#; + let (bin, _stderr) = compile(dir.path(), source, false); + let run = Command::new(&bin) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&run.stdout), + "dense=10;20;30;|10;20;30;\n\ + descriptor=10;41;30;\n\ + hole-proto=1;77;3;\n\ + proxy=10;41;99;\n\ + array-before=4;5;6;\n\ + array-after=4;mixed;6;\n" + ); +}