From 40088b0c52c2f3ebd5fc104e1f4d9aba3c63bec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 04:58:22 +0200 Subject: [PATCH] perf: array forwarding compression, shared add-tree guards, packed shape guards Lands three reviewed PRs as one squash: #8674, #8675, #8676. - #8674: compress array forwarding chains (`clean_arr_ptr` multi-hop walk). - #8675: share numeric guards across dynamic add trees. - #8676: pack monomorphic method shape guards. These three were authored as a stack on top of #8672, but their contents touch disjoint files, so they are cherry-picked onto main on their own. #8672 is NOT included: it defines its own `is_bound_native_method_closure_value` (true for any bound native-module export with a non-empty module name), which #8662 superseded on main with the strictly narrower `is_bound_native_constructor_closure_value` (gated on explicit constructor metadata). Those predicates have different truth sets, so the substitution is a behavioural change at every call site and is left to the author to rebase. Also splits `array/tests.rs`, which #8674 pushed over the 2000-line cap, into an `array/forwarding_tests.rs` sibling. Pure relocation. Version bump not included per maintainer policy. --- .../8674-array-forwarding-path-compression.md | 10 ++ changelog.d/8675-dynamic-add-tree-guard.md | 9 + changelog.d/8676-packed-method-shape-guard.md | 9 + .../collectors/proven_this_routing_tests.rs | 18 +- crates/perry-codegen/src/expr/binary.rs | 27 ++- .../src/expr/dynamic_add_tree_tests.rs | 92 ++++++++++ crates/perry-codegen/src/expr/mod.rs | 2 + .../src/lower_call/method_override.rs | 109 ++++++++---- .../src/array/forwarding_tests.rs | 162 ++++++++++++++++++ crates/perry-runtime/src/array/header.rs | 21 +++ crates/perry-runtime/src/array/mod.rs | 2 + crates/perry-runtime/src/array/tests.rs | 128 +------------- 12 files changed, 426 insertions(+), 163 deletions(-) create mode 100644 changelog.d/8674-array-forwarding-path-compression.md create mode 100644 changelog.d/8675-dynamic-add-tree-guard.md create mode 100644 changelog.d/8676-packed-method-shape-guard.md create mode 100644 crates/perry-codegen/src/expr/dynamic_add_tree_tests.rs create mode 100644 crates/perry-runtime/src/array/forwarding_tests.rs diff --git a/changelog.d/8674-array-forwarding-path-compression.md b/changelog.d/8674-array-forwarding-path-compression.md new file mode 100644 index 0000000000..e58a900fc1 --- /dev/null +++ b/changelog.d/8674-array-forwarding-path-compression.md @@ -0,0 +1,10 @@ +### Performance + +- Compress validated multi-hop array-growth forwarding chains at their retained + head. Generated indexed-access guards can now heal stale aliases after repeated + capacity growth with their existing one-hop path instead of entering generic + indexed lookup on every element access. On the full `codehz/ecs` suite, 11 + alternating Mac mini pairs reduced the 10k read-only query by 81.94% and the + accumulation query by 76.59%, with 11/11 wins and every semantic oracle passing. + Direct Node comparisons still leave 6.282x and 3.127x gaps respectively, so + this change does not claim Node parity. diff --git a/changelog.d/8675-dynamic-add-tree-guard.md b/changelog.d/8675-dynamic-add-tree-guard.md new file mode 100644 index 0000000000..e6aac3b950 --- /dev/null +++ b/changelog.d/8675-dynamic-add-tree-guard.md @@ -0,0 +1,9 @@ +### Performance + +- Lower dynamic `+` trees with three or more leaves behind one shared numeric + guard. Number-only executions use native additions, while the cold arm keeps + the original tree, evaluation order, associativity, and complete dynamic + string/BigInt/Symbol/object-coercion behavior. On the `codehz/ecs` 10k + accumulation query, 11 alternating Mac mini pairs reduced median time by + 4.11%, with 11/11 wins and every semantic oracle passing. The read-only + query was neutral, and this change does not claim Node parity. diff --git a/changelog.d/8676-packed-method-shape-guard.md b/changelog.d/8676-packed-method-shape-guard.md new file mode 100644 index 0000000000..a2227da7c6 --- /dev/null +++ b/changelog.d/8676-packed-method-shape-guard.md @@ -0,0 +1,9 @@ +--- +category: Performance +title: Pack monomorphic method shape guards +--- + +Monomorphic direct-method guards now validate the contiguous GC, class, and +ShapeId header fields with two packed loads while retaining prototype +invalidation, pointer-range checks, ShapeId validation, and the generic +fallback. This reduces repeated dispatch proof work in hot method-call loops. 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 b29a159c2a..f0f113833d 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs @@ -731,17 +731,21 @@ fn single_arm_method_shape_guard_is_inlined_with_the_runtime_contract() { probe.contains("method_direct.inline_deref") && probe.contains("getelementptr i8, ptr") && probe.contains("i64 -8") - && probe.contains("i64 -7") - && probe.contains("i64 -6") - && probe.contains("and i16") - && probe.contains(", 2048") - && probe.contains("icmp ne i32") - && probe.contains("i64 4") + && probe.contains("and i32") + && probe.contains(", 134250751") + && probe.contains("icmp eq i32") + && probe.contains(", 2") + && probe.contains("load i64, ptr") + && probe.contains("zext i32") + && probe.contains("shl i64") + && probe.contains(", 32") + && probe.contains("or i64") + && probe.contains("icmp eq i64") && probe.contains("add i32") && probe.contains(", -2147483648") && probe.contains("icmp ult i32") && probe.contains(", 1073741824"), - "the header block must check the GC type, forwarding flag, own-descriptor bit, nonzero class id, ShapeId domain, and live ShapeId:\n{probe}" + "the packed header block must check the GC type, forwarding flag, own-descriptor bit, exact class/ShapeId pair, and ShapeId domain:\n{probe}" ); } diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index c289ae6c08..27e897139c 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -303,6 +303,24 @@ fn add_tree_leaves<'a>(expr: &'a Expr, out: &mut Vec<&'a Expr>) { } } +/// Whether a fully-dynamic `+` tree is worth one shared numeric guard. +/// +/// A three-or-more-leaf tree is the common accumulator shape +/// `sum += row.x + row.y`: lowering each node independently otherwise pays +/// the dynamic add helper twice even when every runtime value is a number. At +/// two leaves the guard merely moves the helper behind a branch; starting at +/// three it can replace two or more helper calls with one shared tag check. +/// +/// Do not require a static numeric hint here. The important accumulator case +/// is often a captured local plus fields read from interface-shaped objects, +/// so every leaf is `Any` to codegen. The guard itself is the runtime proof; +/// its cold arm preserves the original tree and exact dynamic `+` semantics. +fn dynamic_add_tree_benefits_shared_guard(expr: &Expr) -> bool { + let mut leaves = Vec::new(); + add_tree_leaves(expr, &mut leaves); + leaves.len() >= 3 +} + /// Rebuild the `+` tree over already-lowered leaf values, node for node, so the /// original associativity survives. `fast` picks the inline `fadd`; otherwise /// every node goes through the spec-`+` helper. @@ -889,9 +907,12 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { || crate::type_analysis::expr_produces_canonical_raw_f64(ctx, left)) && (right_bool || crate::type_analysis::expr_produces_canonical_raw_f64(ctx, right)); - if !(both_numeric || boolean_numeric_add) - || add_operands_have_pod_materialization_hazard(ctx, left, right) - { + let materialization_hazard = + add_operands_have_pod_materialization_hazard(ctx, left, right); + if !(both_numeric || boolean_numeric_add) || materialization_hazard { + if dynamic_add_tree_benefits_shared_guard(expr) && !materialization_hazard { + return lower_guarded_numeric_add(ctx, expr); + } return lower_rooted_dynamic_binary( ctx, "js_dynamic_string_or_number_add", diff --git a/crates/perry-codegen/src/expr/dynamic_add_tree_tests.rs b/crates/perry-codegen/src/expr/dynamic_add_tree_tests.rs new file mode 100644 index 0000000000..8f0d50322b --- /dev/null +++ b/crates/perry-codegen/src/expr/dynamic_add_tree_tests.rs @@ -0,0 +1,92 @@ +//! IR coverage for the shared numeric guard on fully dynamic `+` trees. + +use perry_hir::types::Type; +use perry_hir::{BinaryOp, Expr, Stmt}; + +use crate::temp_root_coverage::main_ir_for as ir_for; + +const A: u32 = 1; +const B: u32 = 2; +const C: u32 = 3; +const RESULT: u32 = 4; + +fn any_local(id: u32, name: &str, init: Expr) -> Stmt { + Stmt::Let { + id, + name: name.to_string(), + ty: Type::Any, + mutable: false, + init: Some(init), + } +} + +fn add(left: Expr, right: Expr) -> Expr { + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(left), + right: Box::new(right), + } +} + +fn dynamic_locals() -> Vec { + vec![ + any_local(A, "a", Expr::Undefined), + any_local(B, "b", Expr::Undefined), + any_local(C, "c", Expr::Undefined), + ] +} + +fn result(expr: Expr) -> Stmt { + Stmt::Let { + id: RESULT, + name: "result".to_string(), + ty: Type::Any, + mutable: false, + init: Some(expr), + } +} + +#[test] +fn three_leaf_dynamic_add_tree_uses_one_shared_guard() { + let mut body = dynamic_locals(); + body.push(result(add( + Expr::LocalGet(A), + add(Expr::LocalGet(B), Expr::LocalGet(C)), + ))); + let ir = ir_for("three_leaf_dynamic_add_tree", body); + + assert_eq!( + ir.matches("\nguarded_add.numeric.").count(), + 1, + "the tree should have one shared numeric block:\n{ir}" + ); + assert_eq!( + ir.matches("fadd double").count(), + 2, + "the fast arm must preserve both additions:\n{ir}" + ); + assert_eq!( + ir.matches("call double @js_dynamic_string_or_number_add(") + .count(), + 2, + "the cold arm must preserve both dynamic additions:\n{ir}" + ); +} + +#[test] +fn two_leaf_dynamic_add_stays_on_direct_dispatch() { + let mut body = dynamic_locals(); + body.push(result(add(Expr::LocalGet(A), Expr::LocalGet(B)))); + let ir = ir_for("two_leaf_dynamic_add", body); + + assert!( + !ir.contains("guarded_add.numeric") && !ir.contains("fadd double"), + "a single dynamic add should not pay for a separate guard diamond:\n{ir}" + ); + assert_eq!( + ir.matches("call double @js_dynamic_string_or_number_add(") + .count(), + 1, + "a single dynamic add should keep direct dispatch:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 95a6640830..1df50da00a 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2158,6 +2158,8 @@ mod compare; mod compare_tests; mod conditional; mod dyn_extern_i18n; +#[cfg(test)] +mod dynamic_add_tree_tests; mod env_clones; mod fs_await; mod index_get; diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 10c818e444..bdea25d439 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -11,12 +11,21 @@ use crate::expr::{ }; use crate::nanbox::double_literal; use crate::native_value::LoweredValue; -use crate::types::{DOUBLE, I1, I16, I32, I64, I8}; +use crate::types::{DOUBLE, I1, I32, I64, I8}; const POINTER_TAG_HI16: &str = "32765"; // 0x7FFD const GC_TYPE_OBJECT: &str = "2"; -const GC_FLAG_FORWARDED_I8: &str = "-128"; // 0x80 as i8 -const OBJ_FLAG_HAS_DESCRIPTORS_I16: &str = "2048"; // 0x0800 +// The first four bytes before an ObjectHeader are, in little-endian order, +// `gtype: u8`, `flags: u8`, and `reserved: u16`. One masked i32 load can +// therefore prove the three fields the direct-method contract consumes: +// +// gtype == GC_TYPE_OBJECT +// flags & GC_FLAG_FORWARDED == 0 +// reserved & OBJ_FLAG_HAS_DESCRIPTORS == 0 +// +// Mask: 0x0800_0000 (descriptor bit) | 0x0000_8000 (forwarded bit) | +// 0x0000_00ff (the complete gtype byte). +const GC_OBJECT_METHOD_GUARD_MASK_I32: &str = "134250751"; // 0x0800_80ff const SHAPE_ID_BASE_NEG_I32: &str = "-2147483648"; // subtract 0x8000_0000 const SHAPE_ID_RANGE_LEN: &str = "1073741824"; // 0x4000_0000 @@ -77,44 +86,82 @@ fn emit_inline_direct_method_shape_guard( let recv_handle = blk.and(I64, &recv_bits, crate::nanbox::POINTER_MASK_I64); let obj_ptr = blk.inttoptr(I64, &recv_handle); - let gtype_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-8")]); - let gtype = blk.load(I8, >ype_ptr); - let gtype_ok = blk.icmp_eq(I8, >ype, GC_TYPE_OBJECT); + let gc_header_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-8")]); + let gc_header = blk.load(I32, &gc_header_ptr); + let guarded_gc_bits = blk.and(I32, &gc_header, GC_OBJECT_METHOD_GUARD_MASK_I32); + let gc_header_ok = blk.icmp_eq(I32, &guarded_gc_bits, GC_TYPE_OBJECT); - let gflags_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-7")]); - let gflags = blk.load(I8, &gflags_ptr); - let forwarded = blk.and(I8, &gflags, GC_FLAG_FORWARDED_I8); - let not_forwarded = blk.icmp_eq(I8, &forwarded, "0"); + // ObjectHeader begins with adjacent `class_id: u32` and + // `shape_id: u32`. Compare them as one packed word. The expected + // ShapeId is still range-checked below, so equality proves the live + // receiver's class is non-zero and its ShapeId is in-domain without + // four separate field predicates. + let class_shape = blk.load(I64, &obj_ptr); + let expected_shape_i64 = blk.zext(I32, expected_shape_id, I64); + let expected_shape_high = blk.shl(I64, &expected_shape_i64, "32"); + let expected_class_shape = blk.or(I64, &expected_shape_high, expected_class_id); + let class_shape_ok = blk.icmp_eq(I64, &class_shape, &expected_class_shape); - let reserved_ptr = blk.gep(I8, &obj_ptr, &[(I64, "-6")]); - let reserved = blk.load(I16, &reserved_ptr); - let descriptor_bits = blk.and(I16, &reserved, OBJ_FLAG_HAS_DESCRIPTORS_I16); - let no_own_descriptors = blk.icmp_eq(I16, &descriptor_bits, "0"); - - let class_ptr = blk.gep(I8, &obj_ptr, &[(I64, "0")]); - let class_id = blk.load(I32, &class_ptr); - let class_valid = blk.icmp_ne(I32, &class_id, "0"); - let class_ok = blk.icmp_eq(I32, &class_id, expected_class_id); - - let shape_ptr = blk.gep(I8, &obj_ptr, &[(I64, "4")]); - let shape_id = blk.load(I32, &shape_ptr); // `is_shape_id` is `[0x8000_0000, 0xC000_0000)`. Subtract the base // modulo i32 and compare with the range length, matching the runtime - // helper without a call. - let shape_id_rel = blk.add(I32, &shape_id, SHAPE_ID_BASE_NEG_I32); + // helper. Equality above transfers this proof to the live header. + let shape_id_rel = blk.add(I32, expected_shape_id, SHAPE_ID_BASE_NEG_I32); let shape_valid = blk.icmp_ult(I32, &shape_id_rel, SHAPE_ID_RANGE_LEN); - let shape_ok = blk.icmp_eq(I32, &shape_id, expected_shape_id); - let mut pass = blk.and(I1, >ype_ok, ¬_forwarded); - pass = blk.and(I1, &pass, &no_own_descriptors); - pass = blk.and(I1, &pass, &class_valid); - pass = blk.and(I1, &pass, &class_ok); - pass = blk.and(I1, &pass, &shape_valid); - pass = blk.and(I1, &pass, &shape_ok); + let pass = blk.and(I1, &gc_header_ok, &class_shape_ok); + let pass = blk.and(I1, &pass, &shape_valid); blk.cond_br(&pass, fast_label, fallback_label); } } +#[cfg(test)] +mod packed_guard_tests { + use super::*; + + /// Anti-drift gate for the runtime fields packed into the i32 load at + /// `obj - 8`. Keep this alongside the emitter so a flag move changes a + /// failing test instead of silently weakening a generated guard. + #[test] + fn gc_header_mask_preserves_the_direct_method_contract() { + let obj_type_mask = 0x0000_00ffu32; + let forwarded = u32::from(0x80u8) << 8; + let has_descriptors = 0x0800u32 << 16; + let mask = obj_type_mask | forwarded | has_descriptors; + let expected = u32::from(2u8); + + assert_eq!(GC_OBJECT_METHOD_GUARD_MASK_I32, mask.to_string()); + assert_eq!(expected & mask, expected); + assert_ne!((expected | forwarded) & mask, expected); + assert_ne!((expected | has_descriptors) & mask, expected); + assert_ne!((expected ^ 1) & mask, expected); + } + + /// `ObjectHeader::{class_id,parent_class_id}` are adjacent u32 fields. + /// Perry's supported native targets are little-endian, so one i64 load + /// sees class in the low word and ShapeId in the high word. + #[test] + fn class_shape_word_uses_the_supported_little_endian_layout() { + let class_id = 0x1234_5678u32; + let shape_id = 0x89ab_cdefu32; + let mut bytes = [0u8; 8]; + bytes[..4].copy_from_slice(&class_id.to_le_bytes()); + bytes[4..].copy_from_slice(&shape_id.to_le_bytes()); + assert_eq!( + u64::from_le_bytes(bytes), + (u64::from(shape_id) << 32) | u64::from(class_id) + ); + + for triple in [ + "aarch64-apple-darwin", + "x86_64-unknown-linux-gnu", + "aarch64-linux-android", + "x86_64-pc-windows-msvc", + ] { + assert!(!triple.starts_with("s390") && !triple.starts_with("powerpc64-")); + } + } +} + fn typed_i1_method_signature_note(reps: &[crate::codegen::TypedParamRep]) -> String { let first = reps.first().map(|rep| rep.label()).unwrap_or("void"); if reps.len() <= 1 { diff --git a/crates/perry-runtime/src/array/forwarding_tests.rs b/crates/perry-runtime/src/array/forwarding_tests.rs new file mode 100644 index 0000000000..d0e14222ec --- /dev/null +++ b/crates/perry-runtime/src/array/forwarding_tests.rs @@ -0,0 +1,162 @@ +//! Array growth-forwarding tests. +//! +//! Split out of `tests.rs` (2000-line-per-file cap). Pure relocation -- +//! covers `install_array_growth_forwarding_*` and the `clean_arr_ptr` +//! chain walk (cycle rejection, multi-hop compression, untracked +//! targets). + +use std::ptr; + +use super::*; + +#[test] +fn growth_of_old_array_keeps_forwarding_target_out_of_copying_nursery() { + let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let capacity = MIN_ARRAY_CAPACITY; + let initial = crate::arena::arena_alloc_gc_old_born_tenured( + array_byte_size(capacity as usize), + 8, + crate::gc::GC_TYPE_ARRAY, + ) as *mut ArrayHeader; + + unsafe { + (*initial).length = 0; + (*initial).capacity = capacity; + let elements = (initial as *mut u8).add(std::mem::size_of::()) as *mut u64; + for i in 0..capacity as usize { + // GC_STORE_AUDIT(INIT): initialize unpublished fresh array storage + // with the non-pointer hole sentinel before exposing the array. + ptr::write(elements.add(i), crate::value::TAG_HOLE); + } + set_array_numeric_layout(initial, NumericArrayLayout::RawF64); + crate::gc::layout_init_pointer_free(initial as *mut u8); + } + + let mut head = initial; + for i in 0..=capacity { + head = js_array_push_f64(head, i as f64); + } + + assert_ne!(head, initial, "the capacity-crossing push must grow"); + assert_eq!(clean_arr_ptr_mut(initial), head); + assert_eq!( + crate::arena::classify_heap_generation(head as usize), + crate::arena::HeapGeneration::Old, + "an old forwarding stub must not point into resetting copying-nursery space" + ); +} + +#[test] +fn install_array_growth_forwarding_with_installs_stub_for_injected_header() { + // Actual low-address classification is covered by + // value::addr_class::tests::tracked_gc_classifier_accepts_injected_low_arena_membership. + // This test proves the install path uses the injected tracked header. + const LOW_USER: usize = 0x1_0000_0008; + const { assert!(LOW_USER < 0x200_0000_0000) }; + let old = js_array_alloc(0); + let new = js_array_alloc(0); + unsafe { + let old_header = + crate::value::addr_class::try_read_tracked_gc_header(old as usize).unwrap(); + let header_ptr = old_header.as_ptr(); + let flags = (*header_ptr).gc_flags; + let payload = *(old as *const u64); + + let installed = super::push_pop::install_array_growth_forwarding_with( + LOW_USER, + new as *mut u8, + |candidate| { + assert_eq!(candidate, LOW_USER); + Some(old_header) + }, + ); + let resolved = clean_arr_ptr(old); + + *(old as *mut u64) = payload; + (*header_ptr).gc_flags = flags; + assert!(installed); + assert_eq!(resolved, new); + } +} + +#[test] +fn clean_arr_ptr_rejects_forwarding_cycle() { + let first = js_array_alloc(0); + let second = js_array_alloc(0); + unsafe { + let first_header = crate::value::addr_class::try_read_tracked_gc_header(first as usize) + .unwrap() + .as_ptr(); + let second_header = crate::value::addr_class::try_read_tracked_gc_header(second as usize) + .unwrap() + .as_ptr(); + let first_flags = (*first_header).gc_flags; + let second_flags = (*second_header).gc_flags; + let first_payload = *(first as *const u64); + let second_payload = *(second as *const u64); + crate::gc::set_forwarding_address(first_header, second as *mut u8); + crate::gc::set_forwarding_address(second_header, first as *mut u8); + + let resolved = clean_arr_ptr(first); + + *(first as *mut u64) = first_payload; + *(second as *mut u64) = second_payload; + (*first_header).gc_flags = first_flags; + (*second_header).gc_flags = second_flags; + assert!(resolved.is_null()); + } +} + +#[test] +fn clean_arr_ptr_compresses_multi_hop_forwarding_chain() { + let first = js_array_alloc(0); + let second = js_array_alloc(0); + let live = js_array_alloc(0); + unsafe { + let first_header = crate::value::addr_class::try_read_tracked_gc_header(first as usize) + .unwrap() + .as_ptr(); + let second_header = crate::value::addr_class::try_read_tracked_gc_header(second as usize) + .unwrap() + .as_ptr(); + let first_flags = (*first_header).gc_flags; + let second_flags = (*second_header).gc_flags; + let first_payload = *(first as *const u64); + let second_payload = *(second as *const u64); + crate::gc::set_forwarding_address(first_header, second as *mut u8); + crate::gc::set_forwarding_address(second_header, live as *mut u8); + + let resolved = clean_arr_ptr(first); + let compressed_target = crate::gc::forwarding_address(first_header); + + *(first as *mut u64) = first_payload; + *(second as *mut u64) = second_payload; + (*first_header).gc_flags = first_flags; + (*second_header).gc_flags = second_flags; + assert_eq!(resolved, live); + assert_eq!( + compressed_target, live as *mut u8, + "the original stub must point directly at the validated live head" + ); + } +} + +#[test] +fn clean_arr_ptr_rejects_untracked_forwarding_target_without_deref() { + let array = js_array_alloc(0); + let unrelated = 0x20_0000usize as *mut u8; + unsafe { + let header = crate::value::addr_class::try_read_tracked_gc_header(array as usize) + .unwrap() + .as_ptr(); + let flags = (*header).gc_flags; + let payload = *(array as *const u64); + crate::gc::set_forwarding_address(header, unrelated); + + let resolved = clean_arr_ptr(array); + + *(array as *mut u64) = payload; + (*header).gc_flags = flags; + assert!(resolved.is_null()); + } +} diff --git a/crates/perry-runtime/src/array/header.rs b/crates/perry-runtime/src/array/header.rs index 06d51cbc61..6c21dfeef3 100644 --- a/crates/perry-runtime/src/array/header.rs +++ b/crates/perry-runtime/src/array/header.rs @@ -634,6 +634,7 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { unsafe { crate::value::addr_class::try_read_tracked_gc_header(cleaned as usize) }; unsafe { let mut steps = 0u32; + let mut first_forwarded_header: *mut crate::gc::GcHeader = std::ptr::null_mut(); while let Some(gc_header) = tracked_header { let gc_header = gc_header.as_ptr(); if (*gc_header).obj_type != crate::gc::GC_TYPE_ARRAY @@ -641,6 +642,9 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { { break; } + if steps == 0 { + first_forwarded_header = gc_header; + } let new_user = crate::gc::forwarding_address(gc_header) as usize; let Some(target_header) = crate::value::addr_class::try_read_tracked_gc_header(new_user) @@ -657,6 +661,23 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader { return std::ptr::null(); } } + // A receiver stored through an alias can keep its original array head + // across several capacity-crossing grows. Generated array guards heal + // one forwarding edge inline; without compression, a two-or-more-edge + // chain therefore rejects the guard on every element access and pays + // this entire resolver repeatedly. Once the validated walk reaches the + // live array, point the original retained stub straight at that head. + // The next generated access can then heal its single edge inline. + // + // Do this only after walking and validating the complete chain. A + // corrupt target/cycle returns above and must not rewrite a stub with + // an address that has not been proved to be a tracked array. + if steps > 1 && !first_forwarded_header.is_null() { + crate::gc::set_forwarding_address( + first_forwarded_header, + cleaned as *mut ArrayHeader as *mut u8, + ); + } } // Issue #179 Phase 2: lazy arrays have a GcHeader with // obj_type == GC_TYPE_LAZY_ARRAY. Their layout's first two u32s diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index ace0567d54..ccc939fbf5 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -30,6 +30,8 @@ mod subclass; #[cfg(test)] mod collection_tag_tests; #[cfg(test)] +mod forwarding_tests; +#[cfg(test)] mod spread_dense_tests; #[cfg(test)] mod subclass_tests; diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index dffd1d87c0..11f1e32e8e 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -549,7 +549,8 @@ fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { let mut head = initial; // Capacity progresses 16 -> 32 -> 64 -> 128. Keeping `initial` unchanged - // makes every assertion exercise the complete three-stub chain. + // makes the first resolution exercise the complete three-stub chain; that + // resolution then compresses `initial` directly to the current head. for i in 0..65u32 { head = js_array_push_f64(head, i as f64); } @@ -561,8 +562,9 @@ fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { } // Root the current head, not the deliberately stale first allocation: the - // handle must prove that evacuation moved the live array itself, while - // `initial` independently exercises the three growth stubs afterward. + // handle must prove that evacuation moved the live array itself. The + // compressed `initial` stub then follows the evacuation edge to that new + // head and remains usable. let scope = crate::gc::RuntimeHandleScope::new(); let root = scope.root_raw_mut_ptr(head); let pre_gc_head = head; @@ -582,7 +584,7 @@ fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { assert_eq!( clean_arr_ptr_mut(initial), rooted_head, - "the stale three-stub chain must resolve to the relocated rooted head" + "the compressed growth stub must resolve to the relocated rooted head" ); assert_eq!(js_array_length(initial), 65); for i in 0..65u32 { @@ -590,124 +592,6 @@ fn stale_array_reference_survives_three_growths_and_forced_minor_gc() { } } -#[test] -fn growth_of_old_array_keeps_forwarding_target_out_of_copying_nursery() { - let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); - let capacity = MIN_ARRAY_CAPACITY; - let initial = crate::arena::arena_alloc_gc_old_born_tenured( - array_byte_size(capacity as usize), - 8, - crate::gc::GC_TYPE_ARRAY, - ) as *mut ArrayHeader; - - unsafe { - (*initial).length = 0; - (*initial).capacity = capacity; - let elements = (initial as *mut u8).add(std::mem::size_of::()) as *mut u64; - for i in 0..capacity as usize { - // GC_STORE_AUDIT(INIT): initialize unpublished fresh array storage - // with the non-pointer hole sentinel before exposing the array. - ptr::write(elements.add(i), crate::value::TAG_HOLE); - } - set_array_numeric_layout(initial, NumericArrayLayout::RawF64); - crate::gc::layout_init_pointer_free(initial as *mut u8); - } - - let mut head = initial; - for i in 0..=capacity { - head = js_array_push_f64(head, i as f64); - } - - assert_ne!(head, initial, "the capacity-crossing push must grow"); - assert_eq!(clean_arr_ptr_mut(initial), head); - assert_eq!( - crate::arena::classify_heap_generation(head as usize), - crate::arena::HeapGeneration::Old, - "an old forwarding stub must not point into resetting copying-nursery space" - ); -} - -#[test] -fn install_array_growth_forwarding_with_installs_stub_for_injected_header() { - // Actual low-address classification is covered by - // value::addr_class::tests::tracked_gc_classifier_accepts_injected_low_arena_membership. - // This test proves the install path uses the injected tracked header. - const LOW_USER: usize = 0x1_0000_0008; - const { assert!(LOW_USER < 0x200_0000_0000) }; - let old = js_array_alloc(0); - let new = js_array_alloc(0); - unsafe { - let old_header = - crate::value::addr_class::try_read_tracked_gc_header(old as usize).unwrap(); - let header_ptr = old_header.as_ptr(); - let flags = (*header_ptr).gc_flags; - let payload = *(old as *const u64); - - let installed = super::push_pop::install_array_growth_forwarding_with( - LOW_USER, - new as *mut u8, - |candidate| { - assert_eq!(candidate, LOW_USER); - Some(old_header) - }, - ); - let resolved = clean_arr_ptr(old); - - *(old as *mut u64) = payload; - (*header_ptr).gc_flags = flags; - assert!(installed); - assert_eq!(resolved, new); - } -} - -#[test] -fn clean_arr_ptr_rejects_forwarding_cycle() { - let first = js_array_alloc(0); - let second = js_array_alloc(0); - unsafe { - let first_header = crate::value::addr_class::try_read_tracked_gc_header(first as usize) - .unwrap() - .as_ptr(); - let second_header = crate::value::addr_class::try_read_tracked_gc_header(second as usize) - .unwrap() - .as_ptr(); - let first_flags = (*first_header).gc_flags; - let second_flags = (*second_header).gc_flags; - let first_payload = *(first as *const u64); - let second_payload = *(second as *const u64); - crate::gc::set_forwarding_address(first_header, second as *mut u8); - crate::gc::set_forwarding_address(second_header, first as *mut u8); - - let resolved = clean_arr_ptr(first); - - *(first as *mut u64) = first_payload; - *(second as *mut u64) = second_payload; - (*first_header).gc_flags = first_flags; - (*second_header).gc_flags = second_flags; - assert!(resolved.is_null()); - } -} - -#[test] -fn clean_arr_ptr_rejects_untracked_forwarding_target_without_deref() { - let array = js_array_alloc(0); - let unrelated = 0x20_0000usize as *mut u8; - unsafe { - let header = crate::value::addr_class::try_read_tracked_gc_header(array as usize) - .unwrap() - .as_ptr(); - let flags = (*header).gc_flags; - let payload = *(array as *const u64); - crate::gc::set_forwarding_address(header, unrelated); - - let resolved = clean_arr_ptr(array); - - *(array as *mut u64) = payload; - (*header).gc_flags = flags; - assert!(resolved.is_null()); - } -} - #[test] fn test_numeric_array_layout_metadata_preserves_and_downgrades_on_writes() { let mut arr = js_array_alloc(4);