diff --git a/changelog.d/8921-proven-this-call-args-subclass-push.md b/changelog.d/8921-proven-this-call-args-subclass-push.md new file mode 100644 index 0000000000..635021a18f --- /dev/null +++ b/changelog.d/8921-proven-this-call-args-subclass-push.md @@ -0,0 +1,8 @@ +Compiler and Array performance: a method that passes a declared field's value +to a sibling method (`this.m(this.items[i])`) keeps its proven-receiver clone +instead of re-proving `this` at every site of its public body; and the +generic and spec Array push entries append to an object-backed Array +subclass through its dense fast path before consulting the tracked +allocator resolver. wolf-ecs (noctjs/ecs-benchmark) on the Mac mini +reference box, 2-second window: add/remove -11.2%, entity-cycle -11.9%, +11/11 paired wins each; semantics probes byte-identical to Node. diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index c36abe22e5..67be82a1af 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -1014,6 +1014,90 @@ fn guarded_pshape_call_site_is_preceded_by_a_shape_id_guard() { } } +/// A method that hands a declared field's VALUE to a sibling method +/// (`this.scale(this.value)`) passes nothing but that value: the receiver is +/// not leaked, so the caller keeps its proven-`this` clone. Before, any +/// mention of `this` inside an internal call's arguments rejected the caller +/// outright — wolf-ecs `addComponent(this._ent[id], i)`-style calls lost their +/// clone and re-proved `this` at every site of the public body. +#[test] +fn field_value_arguments_to_sibling_methods_keep_the_proven_this_clone() { + let mut counter = counter_class(); + counter.methods.push(func( + 92, + "scaleByValue", + Vec::new(), + Type::Number, + vec![Stmt::Return(Some(call( + Expr::This, + "scale", + vec![this_get("value")], + )))], + )); + let mut m = Module::new("pshape_field_value_args.ts"); + m.classes = vec![counter]; + m.functions = vec![func( + 1, + "probe", + vec![param(2, "c", Type::Named("Counter".to_string()))], + Type::Number, + vec![Stmt::Return(Some(call( + Expr::LocalGet(2), + "scaleByValue", + Vec::new(), + )))], + )]; + m.init_kind = ModuleInitKind::Eager; + let ir = emit(&m, false); + let clones = pshape_definitions(&ir); + assert!( + clones.iter().any(|d| d.contains("__scaleByValue$pshape")), + "a field-value argument to a sibling method must not reject the clone:\n{clones:#?}" + ); + + // A bare `this` argument still leaks the receiver and must still reject. + let mut counter = counter_class(); + counter.methods.push(func( + 93, + "leak", + vec![param(94, "other", Type::Any)], + Type::Number, + vec![Stmt::Return(Some(Expr::Number(1.0)))], + )); + counter.methods.push(func( + 95, + "leakSelf", + Vec::new(), + Type::Number, + vec![Stmt::Return(Some(call( + Expr::This, + "leak", + vec![Expr::This], + )))], + )); + let mut m = Module::new("pshape_this_value_arg.ts"); + m.classes = vec![counter]; + m.functions = vec![func( + 1, + "probeLeak", + vec![param(2, "c", Type::Named("Counter".to_string()))], + Type::Number, + vec![Stmt::Return(Some(call( + Expr::LocalGet(2), + "leakSelf", + Vec::new(), + )))], + )]; + m.init_kind = ModuleInitKind::Eager; + let ir = emit(&m, false); + assert!( + !pshape_definitions(&ir) + .iter() + .any(|d| d.contains("__leakSelf$pshape")), + "a bare `this` argument leaks the receiver and must reject the clone:\n{ir}" + ); +} + /// The single-pair shape-only arm is small enough to inline at the call site. /// Pin the complete safety gate: acquire both the all-method escape latch and /// the FNV-indexed method-name latch, accept both the boxed-pointer and diff --git a/crates/perry-codegen/src/collectors/ptr_shape.rs b/crates/perry-codegen/src/collectors/ptr_shape.rs index b153919e2b..cb8ba1a1b0 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape.rs @@ -1758,8 +1758,15 @@ impl<'a, 'b> ThisFlowAnalysis<'a, 'b> { if !self.function_this_safe(&owner, property, func, false) { return false; } - args.iter() - .all(|a| !expr_mentions_this(a) && self.expr_this_safe(a, ctx)) + // Arguments are vetted as ordinary expressions: a bare `this` + // in value position, a `this`-capturing closure and a + // non-field `this.x` read all reject there already. A declared + // field READ passed along (`this.m(this.ents[id])`) hands the + // callee a field's value, never the receiver, and must not + // disqualify the caller — wolf-ecs `addComponent` / + // `removeComponent` / `createEntity` each call a sibling + // method with such an argument. + args.iter().all(|a| self.expr_this_safe(a, ctx)) } // `super(...)`: the parent constructor body was already vetted by // `ctor_chain_safe` (whole chain). Args must not leak `this`; in @@ -1788,8 +1795,7 @@ impl<'a, 'b> ThisFlowAnalysis<'a, 'b> { if !self.function_this_safe(&owner, method, func, false) { return false; } - args.iter() - .all(|a| !expr_mentions_this(a) && self.expr_this_safe(a, ctx)) + args.iter().all(|a| self.expr_this_safe(a, ctx)) } // Shape barriers on `this` inside a method body (the module-wide // kill already covers these; kept as defense in depth). diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 5f03bd0922..ffa4050a81 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -670,6 +670,15 @@ pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut A } return arr; } + // An object-backed Array subclass carries `GC_TYPE_OBJECT`, so the tracked + // resolver below is a guaranteed miss for it. Ask the dense subclass append + // first, off the header tag the guarded element tiers already read; every + // rejected case (no dense proof, integrity flags, tail not learned) keeps + // the complete route below. wolf-ecs `packed.push(id)` on an `Archetype` + // paid the resolver twice per push once #8897 routed field pushes here. + if object_backed_push_fast(arr, value) { + return arr; + } let cleaned = clean_arr_ptr_mut(arr); if cleaned.is_null() { // #7574: a `class X extends Array` instance (or any array-like object) @@ -692,6 +701,17 @@ pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut A unsafe { js_array_push_f64_resolved(cleaned, value) } } +/// The dense object-backed Array-subclass append, dispatched off the receiver's +/// `GC_TYPE_OBJECT` header tag before any allocator/registry resolution. The +/// caller has already demoted a uniquely-owned heap string in `value`. +#[inline] +fn object_backed_push_fast(arr: *mut ArrayHeader, value: f64) -> bool { + if crate::array::array_receiver_gc_tag(arr).0 != crate::gc::GC_TYPE_OBJECT { + return false; + } + crate::array::subclass::array_subclass_fast_push_one_raw(arr, value).is_some() +} + /// Push into a live, forwarding-resolved plain Array. The caller owns all /// receiver-brand and Proxy handling; keeping this core separate lets the /// guarded u31 entry reuse the resolved header instead of classifying it a @@ -832,6 +852,16 @@ pub extern "C" fn js_array_push_f64_spec(arr: *mut ArrayHeader, value: f64) -> * if array_ptr_as_proxy(arr).is_some() { return js_array_push_f64(arr, value); } + // Object-backed Array subclass: the dense append needs neither the tracked + // resolver nor the exotic probe (both are classification misses for an + // OBJECT header); the string demote precedes the store as in + // `js_array_push_f64`. + if crate::array::array_receiver_gc_tag(arr).0 == crate::gc::GC_TYPE_OBJECT { + crate::string::js_string_addref_if_heap_string(value); + if crate::array::subclass::array_subclass_fast_push_one_raw(arr, value).is_some() { + return arr; + } + } let cleaned = clean_arr_ptr_mut(arr); if cleaned.is_null() { return js_array_push_f64(arr, value); diff --git a/crates/perry-runtime/src/array/subclass_tests.rs b/crates/perry-runtime/src/array/subclass_tests.rs index ba9d4b3ab5..487214ed7f 100644 --- a/crates/perry-runtime/src/array/subclass_tests.rs +++ b/crates/perry-runtime/src/array/subclass_tests.rs @@ -571,6 +571,62 @@ fn transition_cache_carrier_bits_follow_live_occupancy_across_full_trace_recompu ); } +/// The spec push entry (the typed field-push lowering's complete fallback) +/// and the generic entry both append to an object-backed Array subclass +/// through the dense fast arm, off the header tag, without the tracked +/// resolver: the receiver pointer is returned unchanged and the element is +/// readable through the dense read. +#[test] +fn spec_and_generic_push_entries_append_to_an_object_backed_subclass_densely() { + let _global = crate::gc::global_side_table_test_lock(); + crate::object::array_tail_transition::test_clear(); + let class_id = 0x0074_8696; + 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); + // Learn the tail edge once through the generic route. + assert_eq!( + js_array_push_f64(obj as *mut ArrayHeader, 1.0), + obj as *mut ArrayHeader + ); + assert_eq!(array_subclass_fast_pop(receiver), Some(1.0)); + // Reference: the fused u31 entry's dense arm. Whatever tracked-resolver + // probes the arm itself needs, the spec and generic entries must need the + // same number — none of their own before reaching it. + let probes = crate::value::addr_class::tracked_header_probe_count_for_tests; + let mut length = u32::MAX; + let before = probes(); + assert_eq!( + js_array_push_u31_with_length(obj as *mut ArrayHeader, 5, &mut length), + obj as *mut ArrayHeader + ); + let u31_probes = probes() - before; + assert_eq!(array_subclass_fast_pop(receiver), Some(5.0)); + let before = probes(); + assert_eq!( + crate::array::js_array_push_f64_spec(obj as *mut ArrayHeader, 7.0), + obj as *mut ArrayHeader + ); + let spec_probes = probes() - before; + // Back to the learned edge (length 0 -> 1) before the generic entry. + assert_eq!(array_subclass_fast_pop(receiver), Some(7.0)); + let before = probes(); + assert_eq!( + js_array_push_f64(obj as *mut ArrayHeader, 9.0), + obj as *mut ArrayHeader + ); + let generic_probes = probes() - before; + assert_eq!( + (spec_probes, generic_probes), + (u31_probes, u31_probes), + "the spec and generic entries must reach the dense arm without tracked probes of their own" + ); + assert_eq!(array_subclass_fast_length(receiver), Some(1.0)); + assert_eq!(array_subclass_fast_index_get(receiver, 0), Some(9.0)); +} + #[test] fn array_subclass_named_prefix_token_survives_only_exact_numeric_tail_transitions() { let _global = crate::gc::global_side_table_test_lock(); diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index bd070d04bf..a8c5c1f38c 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -1783,7 +1783,9 @@ pub(crate) unsafe fn cell_meta_slot(user_ptr: usize) -> Option<*mut *mut ObjectM } } -/// Does `user_ptr` name a cell that can own an `ObjectMeta`? +/// Does `user_ptr` name a cell that can own an `ObjectMeta`? (Exercised by +/// the error-cell tests; production code asks `cell_meta_slot` directly.) +#[cfg(test)] pub(crate) unsafe fn cell_has_meta_edge(user_ptr: usize) -> bool { cell_meta_slot(user_ptr).is_some() } diff --git a/crates/perry-runtime/src/object/shapes.rs b/crates/perry-runtime/src/object/shapes.rs index 3506f13a23..e0257c89a3 100644 --- a/crates/perry-runtime/src/object/shapes.rs +++ b/crates/perry-runtime/src/object/shapes.rs @@ -1671,11 +1671,6 @@ pub(crate) fn prune_dead_shape_keys(is_dead_owner: &dyn Fn(usize) -> bool) { } } -/// Metadata-only forwarding repair for the weak descriptor table and -/// pointer-keyed slot indices. Mark/copy mode does not root anything; live -/// object scans provide descriptor reachability, and post-copy rewrite follows -/// only forwarding records those live edges already created. - crate::perry_thread_local! { /// Scratch memo for [`scan_shape_table_rekey_mut`]'s per-address probe, /// reused across collections so the scan allocates nothing. diff --git a/crates/perry-runtime/src/value/addr_class.rs b/crates/perry-runtime/src/value/addr_class.rs index 41c3397ed8..85d4f5905c 100644 --- a/crates/perry-runtime/src/value/addr_class.rs +++ b/crates/perry-runtime/src/value/addr_class.rs @@ -265,6 +265,16 @@ fn classify_tracked_gc_header_with( .then_some((header_addr, TrackedGcStorage::Malloc)) } +/// Test-only count of tracked-resolver probes, so a fast path can pin that +/// it answered without one. +#[cfg(test)] +static TRACKED_HEADER_PROBES: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +#[cfg(test)] +pub(crate) fn tracked_header_probe_count_for_tests() -> u64 { + TRACKED_HEADER_PROBES.load(std::sync::atomic::Ordering::Relaxed) +} + /// Locate a `GcHeader` only after allocator-owned metadata proves that `addr` /// is a Perry GC allocation. Unlike [`try_read_gc_header`], this does not use /// an address-magnitude window as evidence of ownership: arena page membership @@ -285,6 +295,8 @@ fn classify_tracked_gc_header_with( pub(crate) unsafe fn try_read_tracked_gc_header( addr: usize, ) -> Option> { + #[cfg(test)] + TRACKED_HEADER_PROBES.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let (header_addr, storage) = classify_tracked_gc_header_with( addr, |candidate| crate::arena::classify_heap_space_in_range(candidate).map(|(_, base, _)| base),