diff --git a/changelog.d/10098-json-lazy-array-movable-and-brand.md b/changelog.d/10098-json-lazy-array-movable-and-brand.md new file mode 100644 index 0000000000..f016f012a7 --- /dev/null +++ b/changelog.d/10098-json-lazy-array-movable-and-brand.md @@ -0,0 +1,37 @@ +### Performance + +- **Lazy JSON arrays are collectable by a minor, and indexed reads no longer re-classify + the receiver (#10098, #10118).** + + Two independent costs on the same object. A lazy cluster was born into old-gen and + pinned there, so a **dead** one held its whole element graph live through the + remembered set until a full collection — which on `records_array_16k:scan` never + arrived: the arena rebaselined its own trigger 134M->268M->536M->1073M while + `old_in_use` climbed past 48 MB, and every minor reported `survival_permille=996`, + `copied_objects=0`, `freed_bytes=0`. + + `GC_TYPE_LAZY_ARRAY` was pinned for two concrete reasons, both removed the way + `GC_TYPE_REGEXP` removed its own: `json_tape_store` keyed a tape by its owner's + address (now rekeyed through `json_tape_store::owner_moved` + + `GcMoveHookKind::LazyArrayTape`), and the copying minor's flip ran no per-object + finalize hook, so a header dying young leaked its tape (now + `finalize_dead_copied_minor_from_space_lazy_tapes`, wired in beside + map/set/errors/regex). The cluster's generation is still decided ONCE, by cache size + against the pointer-bearing threshold, so #7546's rule that header, cache and bitmap + share a generation holds; large clusters stay old exactly as before. + + Separately, the indexed inline cache's brand check rejected `GC_TYPE_LAZY_ARRAY` + outright, forcing every read through four layers of re-classification. The brand test + now decides on the **tag alone**, with the kind and index guards moved into their own + block, and the shared pointer proof is computed once in the entry block instead of per + tier. + + | row | before | after | vs Node | vs Bun | + |---|---:|---:|---:|---:| + | `records_array_16k:scan` peak RSS | 205 MiB | **51 MiB** | below | below | + | `records_array_1m:scan` peak RSS | 159 MiB | **75 MiB** | below | below | + | `records_array_16k:scan` CPU | — | **-10.1%** | | | + | `records_array_1m:scan` CPU | — | **-5.5%** | | | + | `records_array_20m:repeat` | — | — | | **0.93x** | + + The access window showed zero separated regressions. 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 6f989e12d8..4f3fd5c8db 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 @@ -106,15 +106,33 @@ pub(super) fn lower_inline_dyn_typed_array_get( // ABA-proof for a value held by live code: the arena rewrites `obj_type` // before it hands the address out again, and a live reference keeps the // typed array alive. + // The brand test is the FIRST thing every indexed read on an unknown + // receiver executes, and until #10118 it computed the whole guard set -- + // element kind, both index range checks, three ANDs -- before finding out + // the receiver was not a typed array at all. A `JSON.parse` array, and any + // ordinary Array behind an erased receiver, paid that on every element + // read forever. Decide on the tag alone and leave; the rest of the guard + // set is only meaningful once the tag says typed array. + let kind_guard_idx = ctx.new_block("tav.get.kind_guard"); + let kind_guard_label = ctx.block_label(kind_guard_idx); ctx.current_block = brand_idx; - let entry_guard = { + let is_typed_array = { let blk = ctx.block(); let obj_bits = blk.bitcast_double_to_i64(obj_box); let raw = blk.and(I64, &obj_bits, pointer_mask); let gc_type_addr = blk.sub(I64, &raw, "8"); let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr); let gc_type = blk.load(I8, &gc_type_ptr); - let is_typed_array = blk.icmp_eq(I8, &gc_type, "11"); // GC_TYPE_TYPED_ARRAY + blk.icmp_eq(I8, &gc_type, "11") // GC_TYPE_TYPED_ARRAY + }; + ctx.block() + .cond_br(&is_typed_array, &kind_guard_label, &slow_label); + + ctx.current_block = kind_guard_idx; + let entry_guard = { + let blk = ctx.block(); + let obj_bits = blk.bitcast_double_to_i64(obj_box); + let raw = blk.and(I64, &obj_bits, pointer_mask); let kind_addr = blk.add(I64, &raw, "8"); let kind_ptr = blk.inttoptr(I64, &kind_addr); let kind_i8 = blk.load(I8, &kind_ptr); @@ -127,9 +145,7 @@ pub(super) fn lower_inline_dyn_typed_array_get( // result is never poison there. let idx_ge0 = blk.fcmp("oge", idx_d, "0.0"); let idx_lt = blk.fcmp("olt", idx_d, "4294967296.0"); - // AND-reduce all guards. - let g = blk.and(I1, &is_typed_array, &kind_ok); - let g = blk.and(I1, &g, &idx_ge0); + let g = blk.and(I1, &kind_ok, &idx_ge0); blk.and(I1, &g, &idx_lt) }; ctx.block().cond_br(&entry_guard, &fast_label, &slow_label); diff --git a/crates/perry-runtime/src/gc/copying_phase.rs b/crates/perry-runtime/src/gc/copying_phase.rs index 5c693f7005..9bfbc6b5e2 100644 --- a/crates/perry-runtime/src/gc/copying_phase.rs +++ b/crates/perry-runtime/src/gc/copying_phase.rs @@ -99,7 +99,7 @@ impl CopyingMinorPhaseDiag { let mut out = String::new(); write!( out, - "root_scan={}/{} copy_evacuation={}/{}/{} remembered_set_young_logs={}/{}/{} promotion={}/{}/{} dead_owner_side_table_pruning={}{} from_space_finalization={}/map:{}/{}+set:{}/{}+errors:{}/{}+regex:{}/{} forwarding_fixups={} block_reset_flip={} other={} phase_sum_us={}", + "root_scan={}/{} copy_evacuation={}/{}/{} remembered_set_young_logs={}/{}/{} promotion={}/{}/{} dead_owner_side_table_pruning={}{} from_space_finalization={}/map:{}/{}+set:{}/{}+errors:{}/{}+regex:{}/{}+lazytape:{}/{} forwarding_fixups={} block_reset_flip={} other={} phase_sum_us={}", self.root_scan_ns / 1000, scan_us, self.copy_evacuation_ns / 1000, @@ -122,6 +122,8 @@ impl CopyingMinorPhaseDiag { finalization.errors, finalization.regex_ns / 1000, finalization.regexps, + finalization.lazy_tape_ns / 1000, + finalization.lazy_tapes, self.forwarding_fixups_ns / 1000, self.block_reset_flip_ns / 1000, other_ns / 1000, @@ -145,6 +147,8 @@ pub(super) struct CopiedMinorFinalizationDiag { pub(super) regexps: usize, pub(super) dead_owner_ns: u64, pub(super) dead_owner_detail: String, + pub(super) lazy_tapes: usize, + pub(super) lazy_tape_ns: u64, } /// Finalize the side allocations whose from-space owners just died. The @@ -175,6 +179,10 @@ pub(super) fn finalize_dead_copied_minor_from_space_side_allocations() -> Copied out.regexps = crate::regex::finalize_dead_copied_minor_from_space_regexps(); out.regex_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + let start = diag.then(Instant::now); + out.lazy_tapes = crate::json_tape_store::finalize_dead_copied_minor_from_space_lazy_tapes(); + out.lazy_tape_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); + let start = diag.then(Instant::now); out.dead_owner_detail = super::dead_owner::prune_dead_owner_side_tables_copied_minor(); out.dead_owner_ns = start.map_or(0, |start| start.elapsed().as_nanos() as u64); diff --git a/crates/perry-runtime/src/gc/tests/alloc.rs b/crates/perry-runtime/src/gc/tests/alloc.rs index ec7b11cd34..5d58365976 100644 --- a/crates/perry-runtime/src/gc/tests/alloc.rs +++ b/crates/perry-runtime/src/gc/tests/alloc.rs @@ -481,10 +481,14 @@ fn test_gc_type_metadata_covers_all_declared_types() { arena_walkable: true, rewrite_descriptor_kind: GcRewriteDescriptorKind::LazyArray, layout_slot_kind: GcLayoutSlotKind::None, - // #7539: NOT movable. The tape registry is keyed by the header - // address, and callers outside `json_tape` hold raw header - // pointers across allocations. - movable: false, + // Movable since the tape registration follows its owner + // (`GcMoveHookKind::LazyArrayTape`) and a header dying in a + // copying minor's from-space gives its tape back + // (`finalize_dead_copied_minor_from_space_lazy_tapes`). Those two + // were the whole reason for `false`; pinning cost a dead cluster's + // entire element graph, held live through the remembered set until + // a full collection. + movable: true, // #7539: the tape is a `json_tape_store` side allocation, not // inline payload. Inline, it made the header as large as the tape // (~2.4 MB on a 10k-record blob), which `arena_alloc_gc` routed @@ -493,7 +497,7 @@ fn test_gc_type_metadata_covers_all_declared_types() { external_byte_policy: GcExternalBytePolicy::SideAllocation, large_object_policy: GcLargeObjectPolicy::OldArenaWhenOverThreshold, pointer_free: false, - move_hook_kind: GcMoveHookKind::None, + move_hook_kind: GcMoveHookKind::LazyArrayTape, rewrite_hook_kind: GcRewriteHookKind::None, finalize_hook_kind: GcFinalizeHookKind::LazyArrayTape, }, diff --git a/crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs b/crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs index dee22b81a6..ba6533dbc4 100644 --- a/crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs +++ b/crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs @@ -133,22 +133,25 @@ fn test_old_generation_growth_does_not_scale_with_tape_size() { ); } -/// The header allocation no longer scales with the tape — but it stays in the -/// OLD generation and born tenured, exactly where a multi-megabyte inline-tape -/// header always landed. +/// The header allocation no longer scales with the tape, and a LARGE cluster +/// still lands in the old generation born tenured. /// -/// That is the load-bearing half of this test, not a leftover. `json_tape_store` -/// keys a tape by its owner's address, and every caller outside `json_tape` -/// holds raw `*mut LazyArrayHeader` across allocations — -/// `json::stringify_api::try_stringify_lazy_array` reads `blob_bytes` off a raw -/// header and then allocates the result string. Letting the shrunken header -/// fall into the nursery made it movable for the first time and the copying -/// minor relocated it out from under those callers: `field_access` went -/// non-deterministic, emitting a JSON string of NUL bytes for -/// `JSON.stringify(parsed)` on 3 of 60 iterations. If a future change routes -/// the header allocation back through `arena_alloc_gc`, this fails. +/// It stays there on size now, not on principle. The old-gen request used to be +/// unconditional because `json_tape_store` keys a tape by its owner's address +/// and the copying minor's flip runs no finalize hook, so a nursery header +/// would orphan or leak its tape — `JSON.stringify(parsed)` emitted a string of +/// NUL bytes on 3 of 60 `field_access` iterations when that was tried. +/// `GcMoveHookKind::LazyArrayTape` and +/// `json_tape_store::finalize_dead_copied_minor_from_space_lazy_tapes` remove +/// both reasons, so the generation is decided by cache size — and #7546's rule +/// that header, cache and bitmap share one generation still decides it once. +/// +/// This fixture is `big_blob()`, whose cache is far over the large-object line, +/// so it must still be old: if a future change made even a large cluster +/// nursery-resident, the promotion behaviour this test pins would stop being +/// exercised. #[test] -fn test_lazy_header_is_small_but_stays_old_gen_and_immovable() { +fn test_large_lazy_cluster_is_still_born_old_and_tenured() { let _guard = GcTestIsolationGuard::new(); let blob = big_blob(); let tape_bytes = tape_bytes_of(&blob); @@ -158,12 +161,13 @@ fn test_lazy_header_is_small_but_stays_old_gen_and_immovable() { assert!( crate::arena::pointer_in_old_gen(lazy as usize), - "the header must stay old-gen: callers outside json_tape hold raw \ - header pointers across allocations" + "a cluster this large must still be born old — otherwise the \ + large-object promotion path here stops being exercised" ); assert!( - !crate::gc::gc_type_is_movable(crate::gc::GC_TYPE_LAZY_ARRAY), - "a lazy array must not be movable — its tape is keyed by its address" + crate::gc::gc_type_is_movable(crate::gc::GC_TYPE_LAZY_ARRAY), + "the type is movable now: the tape registration follows its owner and \ + a from-space death gives the tape back" ); unsafe { let header = (lazy as *const u8).sub(GC_HEADER_SIZE) as *const GcHeader; diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs index af1ee42c93..64d80d299a 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs @@ -270,19 +270,25 @@ fn test_json_tape_lazy_get_header_handle_survives_copied_minor_gc() { JsonTapeSafepointHookGuard::new(crate::json_tape::JsonTapeSafepoint::LazyArrayRooted); let hdr = unsafe { test_alloc_lazy_json_array(input) }; let original_hdr = hook.fired_ptr(); - // #7539: the header is old-gen and immovable by construction, so a - // copied minor at the safepoint CANNOT relocate it — that is the - // property `try_stringify_lazy_array` and the array accessors rely on - // when they hold a raw header across an allocation. What must still be - // true is that `alloc_lazy_array` hands back the address the collector - // sees, i.e. the one its own rooted handle resolves to. - assert_eq!( + // A small lazy cluster is nursery-resident and movable now: pinning it + // meant a minor could never reclaim a dead one, and it held its whole + // element graph live through the remembered set. So the header DOES + // relocate here — the hook observed the address before the collection + // it triggered — and what `alloc_lazy_array` owes its caller is the + // REFRESHED address, read back through its own rooted handle. + assert!( + !crate::arena::pointer_in_old_gen(hdr as usize), + "a two-element cluster is small, so it must be nursery-resident" + ); + assert_ne!( hdr as usize, original_hdr, - "the lazy header must not move across a copied-minor GC" + "a nursery header must relocate across the safepoint, or this test \ + proves nothing about the refresh" ); - assert!( - crate::arena::pointer_in_old_gen(hdr as usize), - "…because it is old-gen, which is what makes that guaranteed" + assert_eq!( + unsafe { (*hdr).magic }, + crate::json_tape::LAZY_ARRAY_MAGIC, + "the returned address must be the live header, not the stale one" ); hdr }; @@ -293,11 +299,18 @@ fn test_json_tape_lazy_get_header_handle_survives_copied_minor_gc() { let value = unsafe { crate::json_tape::lazy_get(hdr_handle.get_raw_mut_ptr(), 0) }; let original_hdr = hook.fired_ptr(); let hdr_after = hdr_handle.get_raw_mut_ptr::(); - assert_eq!( + assert_ne!( hdr_after as usize, original_hdr, - "the lazy header must not move across a copied-minor GC (#7539)" + "a nursery header must relocate across lazy_get's safepoint too" ); unsafe { + assert_eq!( + (*hdr_after).magic, + crate::json_tape::LAZY_ARRAY_MAGIC, + "the handle must resolve to the live header after the move" + ); + // The cache the relocated header points at must be the one lazy_get + // wrote through: a stale cache edge would read back as an empty bitmap. let bitmap = (*hdr_after).materialized_bitmap; assert!(!bitmap.is_null()); assert_ne!(*bitmap & 1, 0, "cold lazy_get should cache element 0"); @@ -375,10 +388,16 @@ fn test_json_tape_lazy_get_records_its_cache_store_as_an_external_edge() { // Born-old header. That is the shape the #7538 workload had and the only // one where the in-object/external distinction bites — a nursery header is // traced directly and its descriptor reaches the cache without any - // remembered-set entry at all. #7539 moved the tape into a side allocation - // but deliberately kept the header in the old generation, so this premise - // still holds by construction rather than by the header being large. - let elements = 4096; + // remembered-set entry at all. + // + // The cluster's generation is decided by its cache size now, against the + // POINTER-BEARING threshold (128 KB), so the premise has to be bought with + // element count rather than assumed: 20 000 JSValues is ~156 KB, safely + // over the line. 4096 elements used to suffice only because every lazy + // header was born old unconditionally, and at 32 KB it would now be a + // NURSERY cluster — this test would still pass its later assertions while + // exercising none of the containment branch it exists for. + let elements = 20_000; let mut input = String::with_capacity(elements * 8 + 2); input.push('['); for i in 0..elements { @@ -492,13 +511,19 @@ fn test_json_tape_force_materialize_sparse_cache_handles_survive_copied_minor_gc ); let original_arr = hook.fired_ptr(); let hdr_after = hdr_handle.get_raw_mut_ptr::(); - assert_eq!( + // A four-element cluster is nursery-resident, so the header relocates here + // as well: the rooted handle, not the address, is what keeps a caller right. + assert_ne!( hdr_after as usize, before_force_hdr, - "the lazy header must not move across a copied-minor GC (#7539)" + "a small lazy header is young, so force materialization must relocate it" + ); + assert_eq!( + unsafe { (*hdr_after).magic }, + crate::json_tape::LAZY_ARRAY_MAGIC, + "…and the handle must still resolve to the live header" ); - // The MATERIALIZED ARRAY is young and does move — which is the handle - // refresh this test is really about, and the reason the header being - // stable does not make it vacuous. + // The MATERIALIZED ARRAY moves too, and its handle must refresh for the + // same reason. assert_ne!( arr as usize, original_arr, "force materialization should refresh the rooted array handle" diff --git a/crates/perry-runtime/src/gc/tests/runtime_roots/json_tape_owned.rs b/crates/perry-runtime/src/gc/tests/runtime_roots/json_tape_owned.rs index aa0811f4ba..607f1215db 100644 --- a/crates/perry-runtime/src/gc/tests/runtime_roots/json_tape_owned.rs +++ b/crates/perry-runtime/src/gc/tests/runtime_roots/json_tape_owned.rs @@ -221,7 +221,19 @@ fn json_owned_tape_roots_its_blob_through_copied_minor_during_construction() { JsonTapeSafepointHookGuard::new(crate::json_tape::JsonTapeSafepoint::LazyArrayRooted); let lazy = unsafe { crate::json_tape::alloc_lazy_array_from_scratch(&mut entries, 0, 2, text) }; assert_eq!(entries.capacity(), 0); - assert_eq!(hook.fired_ptr(), lazy as usize); + // The header is movable now, so the collection this hook fires inside + // relocates it: the hook saw the pre-move address and `alloc_lazy_array` + // returns the refreshed one, read back through its own rooted handle. + assert_ne!( + hook.fired_ptr(), + lazy as usize, + "a nursery header must relocate across its construction safepoint" + ); + assert_eq!( + unsafe { (*lazy).magic }, + crate::json_tape::LAZY_ARRAY_MAGIC, + "…and the returned address must be the live header" + ); assert!(gc_collection_count() > before_gc); assert_ne!( unsafe { (*lazy).blob_str }, @@ -246,32 +258,48 @@ fn json_owned_tapes_remain_independent_and_release_only_dead_owners() { let _guard = CopyingNurseryTestGuard::new(2); let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); let before = crate::json_tape_store::registered_bytes(); + // Both owners are movable and nursery-resident now, so a raw header read + // after any collection has to come back through a root. This test's roots + // are the shadow slots, which the collector rewrites — so re-read them + // rather than keeping the addresses `owned_small` returned. + let slot = |i: u32| -> *mut crate::json_tape::LazyArrayHeader { + (js_shadow_slot_get(i) & 0x0000_FFFF_FFFF_FFFF) as *mut crate::json_tape::LazyArrayHeader + }; let first = unsafe { owned_small(b"[1,2,3]") }; js_shadow_slot_set(0, ptr_bits(first as usize)); let first_bytes = crate::json_tape_store::registered_bytes() - before; let second = unsafe { owned_small(b"[4,5,6,7]") }; js_shadow_slot_set(1, ptr_bits(second as usize)); let both = crate::json_tape_store::registered_bytes(); + let (first, second) = (slot(0), slot(1)); assert_ne!(unsafe { (*first).tape }, unsafe { (*second).tape }); let _ = gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct)); assert_eq!(crate::json_tape_store::registered_bytes(), both); assert_eq!( - unsafe { crate::json_tape::LazyArrayHeader::blob_bytes(first) }, + unsafe { crate::json_tape::LazyArrayHeader::blob_bytes(slot(0)) }, b"[1,2,3]" ); assert_eq!( - unsafe { crate::json_tape::LazyArrayHeader::blob_bytes(second) }, + unsafe { crate::json_tape::LazyArrayHeader::blob_bytes(slot(1)) }, b"[4,5,6,7]" ); js_shadow_slot_set(0, crate::JSValue::undefined().bits()); + // These owners are nursery-resident now, and nursery reclamation belongs to + // a minor: `finalize_dead_copied_minor_from_space_lazy_tapes` is what gives + // a dead owner's tape back, the way Map/Set/Error/RegExp give theirs back. + // A full mark-sweep alone leaves the mark bits of a nursery object to the + // minor, so it neither clears them nor reclaims here — asserting on a full + // sweep would be asserting against the wrong pass. let _ = gc_collect_full_mark_sweep_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct)); + let _ = gc_collect_minor_with_trigger(GcTriggerSnapshot::capture(GcTriggerKind::Direct)); assert_eq!( crate::json_tape_store::registered_bytes(), - both - first_bytes + both - first_bytes, + "a dead nursery owner must give its tape back on a minor" ); - let array = unsafe { crate::json_tape::force_materialize_lazy(second) }; + let array = unsafe { crate::json_tape::force_materialize_lazy(slot(1)) }; assert_eq!(unsafe { (*array).length }, 4); assert_eq!(crate::json_tape_store::registered_bytes(), before); } @@ -345,6 +373,12 @@ fn json_lazy_assignment_roots_heap_key_and_value_during_materialization() { crate::arena::ProtectionModeGuard::set(crate::arena::FromSpaceProtection::PoisonOnly); register_runtime_handle_root_scanner_for_tests(); let lazy = unsafe { owned_small(b"[1,2,3]") }; + // The header is movable now, and this test forces an evacuation through + // `js_dyn_index_set_strict`. Root it: reading `(*lazy).materialized` off + // the pre-collection address afterwards reads poisoned from-space and + // faults on the `0xDEADBEEFBAADF0DE` fill. + let lazy_scope = RuntimeHandleScope::new(); + let lazy_handle = lazy_scope.root_raw_mut_ptr(lazy); let key = crate::js_string_from_bytes(b"1".as_ptr(), 1); let bytes = b"owned mutation value survives movement"; let stored = crate::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32); @@ -364,7 +398,12 @@ fn json_lazy_assignment_roots_heap_key_and_value_during_materialization() { string_bits(stored as usize), "stored string must move" ); - let array = unsafe { (*lazy).materialized }; + // The header may have moved during the collection above, so read the + // materialized backing through the handle's scope rather than pinning a + // raw pointer across it (#7341). + let array = lazy_handle.with_mut_ptr(|lazy: *mut crate::json_tape::LazyArrayHeader| unsafe { + (*lazy).materialized + }); assert!(!array.is_null()); let actual = crate::array::js_array_get(array, 1); assert_eq!(actual.bits(), result.to_bits()); diff --git a/crates/perry-runtime/src/gc/types.rs b/crates/perry-runtime/src/gc/types.rs index b87f748501..0a3f11c6a3 100644 --- a/crates/perry-runtime/src/gc/types.rs +++ b/crates/perry-runtime/src/gc/types.rs @@ -234,6 +234,10 @@ pub(crate) enum GcMoveHookKind { /// `GC_TYPE_REGEXP` is movable, and both tables use the payload address as /// their key. RegExpSideTables, + /// Rekey a lazy JSON array's tape registration. `json_tape_store` keys a + /// tape by its owner's address, which is precisely what kept + /// `GC_TYPE_LAZY_ARRAY` immovable and old-gen until this existed. + LazyArrayTape, } #[allow(dead_code)] @@ -465,14 +469,22 @@ pub(super) static GC_TYPE_INFO_BY_ID: [Option; MALLOC_KIND_BUCKET_CO true, GcRewriteDescriptorKind::LazyArray, GcLayoutSlotKind::None, - // NOT movable. `json_tape_store` keys a lazy array's tape by its - // header address, and every caller outside `json_tape` holds raw - // header pointers across allocations. The header is allocated old-gen - // and born tenured (`json_tape::alloc_lazy_header_bytes`), so nothing - // relocates it today; saying so here is what keeps old-page defrag - // from ever doing so. `true` was vacuous before #7539 anyway — the - // header was multi-megabyte and never left the old generation. - false, + // Movable since the tape registration learned to follow its owner + // (`GcMoveHookKind::LazyArrayTape`) and a header dying in a copying + // minor's from-space learned to give its tape back + // (`json_tape_store::finalize_dead_copied_minor_from_space_lazy_tapes`). + // Those two were the whole reason this was `false`: the registry keys + // a tape by its owner's address, and the flip runs no finalize hooks. + // + // Pinning was not free. A lazy cluster born old is never swept by a + // minor, so a DEAD one still holds its entire element graph live + // through the remembered set until a full collection — which on a + // parse-and-scan loop never arrives. Measured on + // `records_array_16k:scan`: every minor promoted essentially the whole + // nursery (`survival_permille=996`, `copied_objects=0`, + // `freed_bytes=0`) while `old_in_use` climbed past 48 MB, for 205 MiB + // peak RSS against Node's 62 MiB. + true, // #7539: the tape is a `json_tape_store` side allocation now, not // inline payload. Keeping it inline made the header ~2.4 MB on a // 10 k-record blob, which `arena_alloc_gc` routed into the old @@ -482,7 +494,7 @@ pub(super) static GC_TYPE_INFO_BY_ID: [Option; MALLOC_KIND_BUCKET_CO GcExternalBytePolicy::SideAllocation, GcLargeObjectPolicy::OldArenaWhenOverThreshold, false, - GcMoveHookKind::None, + GcMoveHookKind::LazyArrayTape, GcRewriteHookKind::None, GcFinalizeHookKind::LazyArrayTape, )), @@ -803,6 +815,9 @@ pub(crate) fn gc_type_after_payload_move(obj_type: u8, old_user: usize, new_user GcMoveHookKind::RegExpSideTables => { crate::regex::regex_header_moved_for_gc(old_user, new_user); } + GcMoveHookKind::LazyArrayTape => { + crate::json_tape_store::owner_moved(old_user, new_user); + } } } @@ -826,6 +841,13 @@ pub(crate) fn gc_type_clear_dead_payload_side_tables(obj_type: u8, user_ptr: usi GcMoveHookKind::ErrorSideTables => { crate::node_submodules::diagnostics_gc::error_side_tables_clear_dead(user_ptr); } + GcMoveHookKind::LazyArrayTape => { + // The tape is released by `GcFinalizeHookKind::LazyArrayTape` and, + // for a header that dies in a copying minor's from-space, by + // `finalize_dead_copied_minor_from_space_lazy_tapes`. Releasing it + // a third time here would be sound (the release is idempotent) but + // would hide which pass actually owns the reclaim. + } GcMoveHookKind::RegExpSideTables => { crate::regex::regex_header_clear_dead_for_gc(user_ptr); } diff --git a/crates/perry-runtime/src/json_tape.rs b/crates/perry-runtime/src/json_tape.rs index 6f292bd38c..1fa64728b7 100644 --- a/crates/perry-runtime/src/json_tape.rs +++ b/crates/perry-runtime/src/json_tape.rs @@ -1229,12 +1229,44 @@ impl LazyArrayHeader { /// `blob_str` reads and stringify emitted NUL bytes. Preserve its generation; /// ownership transfer changes only the pointer-free tape backing allocation. #[inline] -fn alloc_lazy_header_bytes() -> *mut u8 { - crate::arena::arena_alloc_gc_old_born_tenured( - std::mem::size_of::(), - 8, - crate::gc::GC_TYPE_LAZY_ARRAY, - ) +/// Does this lazy array's cluster belong in the old generation? +/// +/// #7539 put the header there because its tape was inline and multi-megabyte; +/// moving the tape out shrank it to ~88 bytes and the old-gen request was kept +/// only because `json_tape_store` keyed a tape by its owner's address and the +/// copying minor's flip ran no finalize hook, so a young header would have +/// orphaned or leaked its tape. `GcMoveHookKind::LazyArrayTape` and +/// `finalize_dead_copied_minor_from_space_lazy_tapes` remove both reasons. +/// +/// Pinning was expensive: a minor never sweeps old-gen, so a DEAD cluster held +/// its whole element graph live through the remembered set until a full +/// collection, which on a parse-and-scan loop never arrives. +/// +/// Decide by size instead, and decide ONCE for the cluster: #7546's invariant +/// is that header, cache and bitmap share a generation, because a nursery cache +/// under an old-gen header is a mixed shape no walker covers. A cache large +/// enough to be born old takes the header with it; anything smaller is +/// nursery-resident and a minor can reclaim the lot. +fn lazy_cluster_is_old(cached_length: u32) -> bool { + let cache_bytes = (cached_length as usize) * std::mem::size_of::(); + // The POINTER-BEARING line, not the flat one. `arena_alloc_gc` keeps the two + // apart for precisely the reason that bites here: tenuring a pointer-bearing + // object does not cost its own bytes, it costs "every object it can reach, + // held live through the remembered set by a container nothing refers to any + // more". The sparse cache is a block of JSValues, so it is that container, + // and a lazy array is the case the distinction was drawn for. 128 KB is + // V8's kMaxRegularHeapObjectSize and sits inside the copier's own ceilings, + // so a cluster admitted by it is always movable. + cache_bytes + crate::gc::GC_HEADER_SIZE + >= crate::gc::LARGE_POINTER_BEARING_OBJECT_THRESHOLD_BYTES +} + +unsafe fn alloc_lazy_cluster_bytes(size: usize, obj_type: u8, old: bool) -> *mut u8 { + if old { + crate::arena::arena_alloc_gc_old_born_tenured(size, 8, obj_type) + } else { + crate::arena::arena_alloc_gc(size, 8, obj_type) + } } pub unsafe fn alloc_lazy_array( @@ -1282,8 +1314,14 @@ unsafe fn alloc_lazy_array_backing( // which can trigger, but the only live thing we hold across it is // `blob_handle`, which is rooted. let (tape_ptr, tape_allocation) = backing.allocate(); - let (raw, blob_str) = - blob_handle.across_const::(alloc_lazy_header_bytes); + let cluster_old = lazy_cluster_is_old(cached_length); + let (raw, blob_str) = blob_handle.across_const::(|| unsafe { + alloc_lazy_cluster_bytes( + std::mem::size_of::(), + crate::gc::GC_TYPE_LAZY_ARRAY, + cluster_old, + ) + }); let hdr = raw as *mut LazyArrayHeader; (*hdr).cached_length = cached_length; (*hdr).magic = LAZY_ARRAY_MAGIC; @@ -1336,11 +1374,8 @@ unsafe fn alloc_lazy_array_backing( // (`parsed[i] === parsed[i]`) across a copying minor. It could not // occur before: a big array's cache was already born old, and a small // array's header was born young along with its cache. - let cache_raw = crate::arena::arena_alloc_gc_old_born_tenured( - cache_bytes, - 8, - crate::gc::GC_TYPE_STRING, - ); + let cache_raw = + alloc_lazy_cluster_bytes(cache_bytes, crate::gc::GC_TYPE_STRING, cluster_old); // arena_alloc_gc can reuse slots from the free list whose // bytes still hold whatever the previous occupant wrote. // Zero explicitly — the cache invariant relies on the @@ -1361,11 +1396,8 @@ unsafe fn alloc_lazy_array_backing( // Same generation as the header and cache — see above. The bitmap // holds no heap edges, but keeping it with its cluster keeps the // page-liveness bookkeeping uniform. - let bitmap_raw = crate::arena::arena_alloc_gc_old_born_tenured( - bitmap_bytes, - 8, - crate::gc::GC_TYPE_STRING, - ); + let bitmap_raw = + alloc_lazy_cluster_bytes(bitmap_bytes, crate::gc::GC_TYPE_STRING, cluster_old); std::ptr::write_bytes(bitmap_raw, 0, bitmap_bytes); let hdr = hdr_handle.get_raw_mut_ptr::(); (*hdr).materialized_bitmap = bitmap_raw as *mut u64; diff --git a/crates/perry-runtime/src/json_tape_store.rs b/crates/perry-runtime/src/json_tape_store.rs index be38e932b5..9ce8285fef 100644 --- a/crates/perry-runtime/src/json_tape_store.rs +++ b/crates/perry-runtime/src/json_tape_store.rs @@ -270,6 +270,62 @@ pub(crate) fn registry_is_empty() -> bool { !TAPE_REGISTRY_NONEMPTY.with(Cell::get) } +/// Rekey a tape whose owner the collector just relocated. +/// +/// `GcMoveHookKind::LazyArrayTape`. The registry is keyed by the owner's +/// address, which was the reason `GC_TYPE_LAZY_ARRAY` had to be immovable and +/// old-gen: a moved header silently orphaned its tape, and the tape then +/// outlived every path that could free it. RegExp solved the same problem the +/// same way (`GcMoveHookKind::RegExpSideTables`), so this is that precedent +/// rather than a new mechanism. +pub(crate) fn owner_moved(old_addr: usize, new_addr: usize) { + if registry_is_empty() || old_addr == new_addr { + return; + } + TAPE_REGISTRY.with(|r| { + let mut registry = r.borrow_mut(); + if let Some(allocation) = registry.remove(&old_addr) { + debug_assert!( + !registry.contains_key(&new_addr), + "a relocated lazy header must not land on a registered address" + ); + registry.insert(new_addr, allocation); + } + }); +} + +/// Release the tapes whose owners just died in a copying minor's from-space. +/// +/// The copying minor's flip runs no per-object finalize hooks, so without this +/// a lazy header that dies young leaks its tape — which is the other half of +/// what kept the type pinned in the old generation. Twin of the sweep-entry +/// [`collect_owners`] pass, mirroring Map/Set/Error/RegExp. +/// +/// Cost: O(registry), i.e. proportional to live-plus-recently-allocated lazy +/// arrays, not to program history. +pub(crate) fn finalize_dead_copied_minor_from_space_lazy_tapes() -> usize { + if registry_is_empty() { + return 0; + } + let dead: Vec = TAPE_REGISTRY.with(|r| { + r.borrow() + .keys() + .copied() + .filter(|&addr| { + crate::gc::owner_is_dead_copied_minor_from_space_of_type( + addr, + crate::gc::GC_TYPE_LAZY_ARRAY, + ) + }) + .collect() + }); + let count = dead.len(); + for addr in dead { + release(addr); + } + count +} + /// Registered owner addresses matching `is_dead`. Split from the release so /// the caller can budget-chunk the frees the way the Map/Set sweep does. pub(crate) fn collect_owners(is_dead: &dyn Fn(usize) -> bool) -> Vec {