diff --git a/changelog.d/9807-layout-prune-single-pass.md b/changelog.d/9807-layout-prune-single-pass.md new file mode 100644 index 0000000000..ab8c8fe72c --- /dev/null +++ b/changelog.d/9807-layout-prune-single-pass.md @@ -0,0 +1,33 @@ +**The per-object layout death-prune walks its tables once instead of three +times, and no longer allocates a `Vec` of every live key** — 50.6 MB of a +compiled claude-code turn's 304 MB in the layout tables (#9792). + +`prune_dead_per_object_layout_owners` visited every surviving key three times +per collection: `retain` to drop the dead owners, then +`layout_addr_filter_rebuild` — which first collected all of them into a +`Vec` — and then `recount_young_layout_records` to re-derive the +nursery-key count. The last two want exactly the survivor set `retain` is +already walking, so both fold into its closure. The `Vec` is gone from the +rebuild's other caller too. + +The measurement that prompted it also found the accelerator these tables sit +behind unable to do its job. `layout_addr_filter_may_hold` is a 4,096-bit +one-hash sketch documented for "one or two entries, ~0.05 % false positives"; +a new `PERRY_LAYOUT_DIAG` instrument reports **162,258 live keys and 4,096 of +4,096 bits set** on one 400-character claude-code reply. Every probe answers +"may hold", so the early returns in `transfer_per_object_descriptor` and +`transfer_per_object_slot_mask` never fire, and each rebuild was an O(live +keys) walk restoring the all-ones state it started from. Past four times the +bit count — 16,384 keys, where the false-positive rate is already 98.2 % — the +rebuild now sets all ones directly, the same conservative answer reached in +O(1); below that the filter keeps exactly the selectivity it has today. The +instrument says so out loud rather than leaving it to be inferred. Widening the sketch is not available +from the runtime: its geometry and hash are mirrored in `perry-codegen`'s +`emit_gated_forget_object_layout`, and discriminating at 162k keys would take +~190 KB of inline thread-local storage per thread. + +`transfer_per_object_descriptor` also gained the emptiness test its shared +flag cannot express: the flag and the filter are common to both per-object +tables, so a full slot-mask table drags every relocation into the typed-layout +map as well — which on cc is permanently empty (typed=0, masks=162,258). One +`len` load replaces two hashes per evacuated object. diff --git a/changelog.d/9816-for-of-array-iter-result.md b/changelog.d/9816-for-of-array-iter-result.md new file mode 100644 index 0000000000..bbf827b5f0 --- /dev/null +++ b/changelog.d/9816-for-of-array-iter-result.md @@ -0,0 +1,11 @@ +### Runtime + +- perf(runtime): a `for…of` over an array no longer allocates a `{ value, done }` + object per element. The fused `for…of` advance (`js_for_of_next`) already + recycled ONE result object per iterator for builtin Map/Set iterators; array + iterators fell through to the generic arm and minted a fresh 40-byte object + for every element. They now take the same fused arm, and the recycling routine + is one shared implementation rather than a second copy. Manual `.next()`, + spread, `Array.from`, `yield*` and `for await` are unchanged and keep + returning fresh results, so a caller that retains one still sees spec + behaviour; the recycled object is only ever the compiler's own loop temporary. diff --git a/crates/perry-runtime/src/array/iter_object.rs b/crates/perry-runtime/src/array/iter_object.rs index 94b3214552..3276f932b8 100644 --- a/crates/perry-runtime/src/array/iter_object.rs +++ b/crates/perry-runtime/src/array/iter_object.rs @@ -33,6 +33,12 @@ use crate::value::{js_nanbox_get_pointer, js_nanbox_pointer, JSValue, TAG_UNDEFI /// runtime-defined classes. pub const ARRAY_ITERATOR_CLASS_ID: u32 = 0xFFFF_0006; +/// Field holding the recycled `{value, done}` the fused `for…of` driver +/// mutates in place — one result object per ITERATOR instead of one per +/// element. Same index and same contract as the Map/Set iterator's, and the +/// same routine emits both (`iter_result::emit_iter_result_cached`). +const ITER_RESULT_CACHE_FIELD: u32 = 5; + /// Iterator kind tags — matches the i32 stored in field 2. const KIND_VALUES: i32 = 0; const KIND_KEYS: i32 = 1; @@ -66,7 +72,12 @@ unsafe fn alloc_iterator_backing(backing: f64, kind: i32) -> f64 { // The iterator allocation and the lazy prototype bootstrap can both // collect. Keep the incoming backing and the new iterator relocatable. let backing_h = scope.root_nanbox_f64(backing); - let obj_h = scope.root_raw_mut_ptr(js_object_alloc(ARRAY_ITERATOR_CLASS_ID, 3)); + // Six fields, not three: 3 and 4 are the `node:sqlite` epoch pair and 5 is + // the recycled `{value, done}` the fused `for…of` driver mutates in place + // (see `ITER_RESULT_CACHE_FIELD`). Reserving them at construction keeps the + // cache out of the per-iterator shape transition that growing into field 5 + // would otherwise cost, and matches the Map/Set iterator's layout. + let obj_h = scope.root_raw_mut_ptr(js_object_alloc(ARRAY_ITERATOR_CLASS_ID, 6)); // Field 0: backing array (NaN-boxed pointer so the GC scanner keeps it). obj_h.with_mut_ptr(|obj| { js_object_set_field( @@ -79,6 +90,14 @@ unsafe fn alloc_iterator_backing(backing: f64, kind: i32) -> f64 { obj_h.with_mut_ptr(|obj| js_object_set_field(obj, 1, JSValue::number(0.0))); // Field 2: iterator kind. obj_h.with_mut_ptr(|obj| js_object_set_field(obj, 2, JSValue::number(kind as f64))); + // Fields 3/4: the `node:sqlite` epoch pair, unused by every other kind. + obj_h.with_mut_ptr(|obj| js_object_set_field(obj, 3, JSValue::undefined())); + obj_h.with_mut_ptr(|obj| js_object_set_field(obj, 4, JSValue::undefined())); + // Field 5: the recycled fused-driver result. Manual `.next()` never reads + // or writes it, so a caller that retains a result still sees fresh objects. + obj_h.with_mut_ptr(|obj| { + js_object_set_field(obj, ITER_RESULT_CACHE_FIELD, JSValue::undefined()) + }); // Link `[[Prototype]]` to the shared `%ArrayIteratorPrototype%` singleton so // `Object.getPrototypeOf(it)` and the inherited `.next` read resolve. obj_h @@ -114,7 +133,7 @@ pub fn array_values_iter_null_done( if arr_ptr.is_null() { return f64::from_bits(TAG_UNDEFINED); } - let obj = js_object_alloc(ARRAY_ITERATOR_CLASS_ID, 5); + let obj = js_object_alloc(ARRAY_ITERATOR_CLASS_ID, 6); js_object_set_field( obj, 0, @@ -128,6 +147,7 @@ pub fn array_values_iter_null_done( JSValue::pointer(iteration_epoch as *const _ as *const u8), ); js_object_set_field(obj, 4, JSValue::number(epoch as f64)); + js_object_set_field(obj, ITER_RESULT_CACHE_FIELD, JSValue::undefined()); crate::object::attach_iterator_prototype(obj, ARRAY_ITERATOR_CLASS_ID); js_nanbox_pointer(obj as i64) } @@ -620,7 +640,22 @@ pub unsafe fn dispatch_array_iterator_method( iter_obj: *mut ObjectHeader, method_name: &str, ) -> f64 { - dispatch_array_iterator_method_inner(iter_obj, method_name, true) + dispatch_array_iterator_method_inner(iter_obj, method_name, true, false) +} + +/// The FUSED `for…of` advance (`js_for_of_next`): same algorithm, but the +/// `{value, done}` is the iterator's own recycled one rather than a fresh +/// allocation per element. Only the compiler's `for…of` desugar reaches this, +/// and its result local is a temporary the loop body cannot name — see +/// [`crate::iter_result::emit_iter_result_cached`]. The override probe still +/// runs first, so a patched own `next` wins exactly as on the manual path. +pub(crate) unsafe fn dispatch_array_iterator_method_emit( + iter_obj: *mut ObjectHeader, + method_name: &str, + emit_cached: bool, + honor_override: bool, +) -> f64 { + dispatch_array_iterator_method_inner(iter_obj, method_name, honor_override, emit_cached) } /// Builtin advance only — the canonical prototype thunk's entry (#9019): @@ -632,13 +667,14 @@ pub(crate) unsafe fn dispatch_array_iterator_method_builtin( iter_obj: *mut ObjectHeader, method_name: &str, ) -> f64 { - dispatch_array_iterator_method_inner(iter_obj, method_name, false) + dispatch_array_iterator_method_inner(iter_obj, method_name, false, false) } unsafe fn dispatch_array_iterator_method_inner( iter_obj: *mut ObjectHeader, method_name: &str, honor_override: bool, + emit_cached: bool, ) -> f64 { // #7475: the raw `iter_obj` parameter is not a GC root, and this function // allocates in several places — `js_object_set_field` (shape transition / @@ -662,6 +698,15 @@ unsafe fn dispatch_array_iterator_method_inner( JSValue::undefined() } }; + // `node:sqlite`'s iterator yields `{ done, value }`; every other kind + // yields `{ value, done }`. The key order is observable through + // `Object.keys`/`JSON.stringify`, so it picks the shared keys array (and + // therefore the shape) the result is built with. + let result_order = if kind == KIND_VALUES_NULL_DONE { + crate::iter_result::IterResultOrder::DoneValue + } else { + crate::iter_result::IterResultOrder::ValueDone + }; match method_name { "next" => { if honor_override { @@ -692,10 +737,15 @@ unsafe fn dispatch_array_iterator_method_inner( // Array iterators clear their backing array at exhaustion. SQLite's // statement iterator restarts a completed execution on the next call. if JSValue::from_bits(backing_f64.to_bits()).is_undefined() { - if kind == KIND_VALUES_NULL_DONE { - return make_sqlite_iter_result(done_value(), true); - } - return make_iter_result(done_value(), true); + return crate::iter_result::emit_iter_result_cached( + &scope, + &iter_h, + ITER_RESULT_CACHE_FIELD, + emit_cached, + result_order, + done_value(), + true, + ); } let backing_ptr = js_nanbox_get_pointer(backing_f64); // Field 1: current index. @@ -714,11 +764,20 @@ unsafe fn dispatch_array_iterator_method_inner( if idx >= len { if kind == KIND_VALUES_NULL_DONE { + // SQLite's statement iterator restarts on the next call. js_object_set_field(iter_obj(), 1, JSValue::number(0.0)); - return make_sqlite_iter_result(done_value(), true); + } else { + js_object_set_field(iter_obj(), 0, JSValue::undefined()); } - js_object_set_field(iter_obj(), 0, JSValue::undefined()); - return make_iter_result(done_value(), true); + return crate::iter_result::emit_iter_result_cached( + &scope, + &iter_h, + ITER_RESULT_CACHE_FIELD, + emit_cached, + result_order, + done_value(), + true, + ); } // Advance the stored cursor before computing the value so a @@ -759,11 +818,15 @@ unsafe fn dispatch_array_iterator_method_inner( _ => JSValue::undefined(), }; let value_h = scope.root_nanbox_u64(value.bits()); - if kind == KIND_VALUES_NULL_DONE { - make_sqlite_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), false) - } else { - make_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), false) - } + crate::iter_result::emit_iter_result_cached( + &scope, + &iter_h, + ITER_RESULT_CACHE_FIELD, + emit_cached, + result_order, + JSValue::from_bits(value_h.get_nanbox_u64()), + false, + ) } // Iterators are themselves iterable — `[Symbol.iterator]()` on one // returns the same iterator (matches Node, and lets `js_get_iterator` diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 99359c9d92..1c454050ef 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -173,12 +173,14 @@ pub use self::iter_methods::{ js_array_map_discard, js_array_reduce, js_array_some, js_array_some_captureless, js_array_to_locale_string, js_validate_array_callback, js_validate_array_map_callback, }; -pub(crate) use self::iter_object::dispatch_array_iterator_method_builtin; pub use self::iter_object::{ arguments_values_iter, array_entries_iter, array_keys_iter, array_values_iter, array_values_iter_null_done, dispatch_array_iterator_method, js_array_entries_iter_obj, js_array_keys_iter_obj, js_array_values_iter_obj, ARRAY_ITERATOR_CLASS_ID, }; +pub(crate) use self::iter_object::{ + dispatch_array_iterator_method_builtin, dispatch_array_iterator_method_emit, +}; pub(crate) use self::iterator::iter_bt_dump; pub(crate) use self::iterator::{array_from_spread_value, is_builtin_iterator_class_id}; pub use self::iterator::{ diff --git a/crates/perry-runtime/src/collection_iter_object.rs b/crates/perry-runtime/src/collection_iter_object.rs index 4a91eef8ec..62864230e7 100644 --- a/crates/perry-runtime/src/collection_iter_object.rs +++ b/crates/perry-runtime/src/collection_iter_object.rs @@ -196,13 +196,11 @@ static KEEP_SET_KEYS_ITER: extern "C" fn(*const SetHeader) -> i64 = js_set_keys_ #[used] static KEEP_SET_ENTRIES_ITER: extern "C" fn(*const SetHeader) -> i64 = js_set_entries_iter_obj; -/// Build the `{ value, done }` iterator-result object. Mirrors -/// `array/iter_object.rs::make_iter_result`. -// #7564: this was a local five-allocation copy with every intermediate in a -// bare Rust local — see `crate::iter_result` for what that cost and why it was -// a stale-from-space hazard. `use` rather than a wrapper so the call sites -// below read unchanged. -use crate::iter_result::make_iter_result; +// #7564: the `{ value, done }` constructor was a local five-allocation copy +// with every intermediate in a bare Rust local — see `crate::iter_result` for +// what that cost and why it was a stale-from-space hazard. Since the fused +// driver's recycling moved there too, this module reaches results only through +// `iter_result::emit_iter_result_cached` and imports no constructor of its own. /// `[key, value]` pair array for Map entries / Set entries (`[v, v]`). unsafe fn make_pair_array(a: f64, b: f64) -> f64 { @@ -376,19 +374,14 @@ unsafe fn dispatch_set_iterator_method_emit( } } -/// Emit a `{value, done}` iterator result. -/// -/// `emit_cached == false` (every manual `.next()` and both public -/// dispatchers) allocates a fresh object per call, exactly as before — -/// results a caller retains behave per spec. -/// -/// `emit_cached == true` is reserved for [`js_for_of_next`], whose only -/// caller is the compiler's `for…of` desugar. There the result local is a -/// compiler temporary the loop body cannot name, read for `done`/`value` -/// before the next advance — so mutating one cached object per ITERATOR is -/// unobservable, and it deletes the per-element allocation that dominated -/// generic iteration. The cache lives in the iterator object's field 5, so -/// the GC traces and rewrites it like any other field. +/// Field of a Map/Set iterator object that holds the recycled `{value, done}` +/// the fused `for…of` driver mutates in place. `alloc_iterator` reserves it. +const ITER_RESULT_CACHE_FIELD: u32 = 5; + +/// Emit a `{value, done}` iterator result — see +/// [`crate::iter_result::emit_iter_result_cached`] for the caching contract. +/// The array iterator uses the same routine through the same helper, so the +/// two cannot drift. unsafe fn emit_iter_result( scope: &crate::gc::RuntimeHandleScope, iter_h: &crate::gc::RuntimeHandle, @@ -396,42 +389,34 @@ unsafe fn emit_iter_result( value: JSValue, done: bool, ) -> f64 { - let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; - if !emit_cached { - return make_iter_result(value, done); - } - let cached = js_object_get_field(iter_obj(), 5); - if JSValue::from_bits(cached.bits()).is_pointer() { - let res = js_nanbox_get_pointer(f64::from_bits(cached.bits())) as *mut ObjectHeader; - // Barriered field stores: the iterator (and its cached result) may be - // tenured while `value` is young. - js_object_set_field(res, 0, value); - js_object_set_field(res, 1, JSValue::bool(done)); - return js_nanbox_pointer(res as i64); - } - // First fused advance on this iterator: build the result once and cache - // it. `make_iter_result` allocates, so root `value` across it. - let value_h = scope.root_nanbox_u64(value.bits()); - let res = make_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), done); - let res_h = scope.root_nanbox_f64(res); - js_object_set_field( - iter_obj(), - 5, - JSValue::from_bits(res_h.get_nanbox_f64().to_bits()), - ); - res_h.get_nanbox_f64() + crate::iter_result::emit_iter_result_cached( + scope, + iter_h, + ITER_RESULT_CACHE_FIELD, + emit_cached, + crate::iter_result::IterResultOrder::ValueDone, + value, + done, + ) } /// One fused `IteratorNext` for the `for…of` desugar: advance + result in a /// single runtime call. /// -/// A builtin Map/Set iterator advances in place and reuses its cached result -/// object (see [`emit_iter_result`]); the override probe inside the -/// dispatcher still runs first, so a patched `next` wins exactly as it does -/// on the manual path. Every other receiver — array iterators, generators, -/// user iterators — takes the arm at the bottom, which is byte-for-byte the -/// two-call desugar this entry replaces: the dynamic `.next()` dispatch -/// followed by spec IteratorNext result validation. +/// A builtin Map/Set/Array iterator advances in place and reuses its cached +/// result object (see [`crate::iter_result::emit_iter_result_cached`]); the +/// override probe inside each dispatcher still runs first, so a patched `next` +/// wins exactly as it does on the manual path. Every other receiver — +/// generators, user iterators, string and typed-array iterators — takes the arm +/// at the bottom, which is byte-for-byte the two-call desugar this entry +/// replaces: the dynamic `.next()` dispatch followed by spec IteratorNext +/// result validation. +/// +/// The array arm is the one that matters on a real program: the allocation-site +/// census of the compiled claude-code TUI attributed **100 %** of its iterator +/// -result bytes to `array::iter_object` — one 40-byte object per element of +/// every generic `for…of`, which is what a minified bundle emits whenever the +/// iterated value is not a statically proven array. #[no_mangle] pub unsafe extern "C-unwind" fn js_for_of_next(iter: f64) -> f64 { let jv = JSValue::from_bits(iter.to_bits()); @@ -457,6 +442,13 @@ pub unsafe extern "C-unwind" fn js_for_of_next(iter: f64) -> f64 { dispatch_set_iterator_method_emit(obj, "next", true, true), ); } + if class_id == crate::array::ARRAY_ITERATOR_CLASS_ID { + return crate::symbol::js_iterator_result_validate( + crate::array::dispatch_array_iterator_method_emit( + obj, "next", true, true, + ), + ); + } } } } @@ -547,18 +539,97 @@ mod fused_for_of_tests { } } - /// A non-collection receiver takes the generic arm: dynamic `.next()` - /// dispatch plus validation — here, an array VALUES iterator object. + /// The ARRAY arm — the one the allocation census says carries 100 % of a + /// real program's iterator-result bytes. Same contract as the Set arm: + /// correct walk, terminates, and ONE recycled result installed in the + /// iterator's cache field, while the manual dispatcher keeps allocating + /// fresh ones. #[test] - fn fused_next_routes_other_iterators_through_the_generic_arm() { + fn fused_next_walks_an_array_and_recycles_its_result() { unsafe { let arr = crate::array::js_array_alloc(2); crate::array::js_array_push_f64(arr, 7.0); crate::array::js_array_push_f64(arr, 8.0); let iter = crate::array::array_values_iter(js_nanbox_pointer(arr as i64)); - assert_eq!(value_of(js_for_of_next(iter)), 7.0); - assert_eq!(value_of(js_for_of_next(iter)), 8.0); + + // Read the cache field immediately after each advance rather than + // comparing two nan-boxed pointers taken across a possible + // collection: a copying minor between the two calls would move the + // result and make a raw bit comparison fail for the wrong reason. + let cache_now = || { + let iter_obj = js_nanbox_get_pointer(iter) as *mut ObjectHeader; + js_object_get_field(iter_obj, 5).bits() + }; + let r1 = js_for_of_next(iter); + assert_eq!(value_of(r1), 7.0); + assert!( + JSValue::from_bits(cache_now()).is_pointer(), + "the first fused advance must install the recycled result" + ); + assert_eq!( + r1.to_bits(), + cache_now(), + "the first result IS the cached object" + ); + let r2 = js_for_of_next(iter); + assert_eq!(value_of(r2), 8.0); + assert_eq!( + r2.to_bits(), + cache_now(), + "the fused driver must hand back the cached object, or nothing is saved" + ); assert!(done_of(js_for_of_next(iter))); + assert!(done_of(js_for_of_next(iter)), "stays exhausted"); + + // The manual path still returns fresh, independent results — a + // caller that retains one must not see it mutate underneath. + let manual_iter = crate::array::array_values_iter(js_nanbox_pointer(arr as i64)); + let m1 = crate::array::dispatch_array_iterator_method( + js_nanbox_get_pointer(manual_iter) as *mut ObjectHeader, + "next", + ); + let m2 = crate::array::dispatch_array_iterator_method( + js_nanbox_get_pointer(manual_iter) as *mut ObjectHeader, + "next", + ); + assert_eq!(value_of(m1), 7.0); + assert_eq!(value_of(m2), 8.0); + assert_ne!( + m1.to_bits(), + m2.to_bits(), + "manual .next() must keep allocating fresh results" + ); + assert_eq!( + value_of(m1), + 7.0, + "the first manual result must still read 7 after the second call" + ); + } + } + + /// A receiver outside the three fused families still takes the generic + /// arm: dynamic `.next()` dispatch plus spec IteratorNext validation. + /// Without this the fused branch could be "always taken" and the test + /// above would still pass. + #[test] + fn fused_next_routes_other_iterators_through_the_generic_arm() { + unsafe { + let s = crate::string::js_string_from_bytes(b"ab".as_ptr(), 2); + let iter = crate::string::string_values_iter(s); + let iter_obj = js_nanbox_get_pointer(iter) as *mut ObjectHeader; + let r1 = js_for_of_next(iter); + let r2 = js_for_of_next(iter); + assert_ne!( + r1.to_bits(), + r2.to_bits(), + "the generic arm must not recycle — it has no cache field" + ); + assert!(done_of(js_for_of_next(iter))); + assert_eq!( + (*iter_obj).class_id, + crate::string::STRING_ITERATOR_CLASS_ID, + "receiver really is outside the fused families" + ); } } } diff --git a/crates/perry-runtime/src/gc/layout_tables.rs b/crates/perry-runtime/src/gc/layout_tables.rs index e5d71a1c03..420c2b1df0 100644 --- a/crates/perry-runtime/src/gc/layout_tables.rs +++ b/crates/perry-runtime/src/gc/layout_tables.rs @@ -168,21 +168,11 @@ fn note_new_layout_record(user_ptr: usize) { } } -/// Re-derive this thread's young-record count from the live keys and publish -/// the delta. Runs after every death prune (all cycle kinds) and whenever the -/// tables empty, so promotion (a key moving to an old page) and death both -/// bring the count back down. -fn recount_young_layout_records() { - let live = { - let masks = hot_layout_slot_masks().borrow(); - let typed = hot_typed_layouts().borrow(); - masks - .keys() - .chain(typed.keys()) - .filter(|key| layout_key_may_be_nursery(**key)) - .count() - }; - let live = u32::try_from(live).unwrap_or(u32::MAX); +/// Publish this thread's young-record count and the delta to the process +/// total. The count itself is derived by the death prune's single pass over +/// the live keys (all cycle kinds), so promotion (a key moving to an old page) +/// and death both bring it back down without a walk of their own. +fn publish_young_layout_records(live: u32) { let prev = hot_per_object_layout_hint().young_records.replace(live); use std::sync::atomic::Ordering::SeqCst; if live > prev { @@ -204,28 +194,97 @@ pub(in crate::gc) fn prune_dead_per_object_layout_owners(is_dead_owner: &dyn Fn( if !per_object_layouts_maybe_nonempty() { return; } + // ONE pass over each table, not three. The old shape visited every live + // key three times per collection — `retain`, then + // `layout_addr_filter_rebuild` (which first collected them all into a + // `Vec`), then `recount_young_layout_records` — and the survivor + // set the last two want is exactly what `retain` is already walking. On cc + // that was 162k keys x 47 prunes per 400-character reply, with the `Vec` + // alone allocating 50.6 MB of the turn's 304 MB (#9792). + let hint = hot_per_object_layout_hint(); + // A filter this occupancy has outgrown is not worth rebuilding: see + // [`layout_addr_filter_saturating_occupancy`]. Decide from the pre-prune + // size, which bounds the survivor count from above, so the decision is one + // branch captured by the closure rather than a test per key. + let occupancy = hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len(); + let rebuild_filter = occupancy <= layout_addr_filter_saturating_occupancy(); + if rebuild_filter { + // Cleared FIRST so the bits set below describe survivors only — a key + // that dies in this pass must leave no bit behind, which is the whole + // point of rebuilding. + layout_addr_filter_clear(); + } else { + layout_addr_filter_saturate(); + } + let mut young: u32 = 0; + let mut keep = |key: usize| { + if is_dead_owner(key) { + return false; + } + if rebuild_filter { + let (word, bit) = layout_addr_filter_slot(key); + // SAFETY: the filter is a plain `UnsafeCell` in this thread's own + // hot slot and nothing else holds a reference to it here. This is + // the same single-threaded access every probe makes; the tables + // borrowed around it are different thread-locals. + unsafe { + (*hint.filter.get())[word] |= bit; + } + } + if layout_key_may_be_nursery(key) { + young = young.saturating_add(1); + } + true + }; let masks_emptied = { let mut masks = hot_layout_slot_masks().borrow_mut(); let had = !masks.is_empty(); - masks.retain(|key, _| !is_dead_owner(*key)); + masks.retain(|key, _| keep(*key)); had && masks.is_empty() }; let typed_emptied = { let mut typed = hot_typed_layouts().borrow_mut(); let had = !typed.is_empty(); - typed.retain(|key, _| !is_dead_owner(*key)); + typed.retain(|key, _| keep(*key)); had && typed.is_empty() }; + publish_young_layout_records(young); + // Runs last: when it finds both tables empty it disarms the flag, zeroes + // the young count published above and clears the filter, which is the + // correct end state whichever branch the pass took. refresh_per_object_layouts_flag(masks_emptied || typed_emptied); - if per_object_layouts_maybe_nonempty() { - // Stale filter bits are what the pruned keys leave behind; rebuilding - // from the survivors keeps the runtime probes as selective as the - // tables really are. - layout_addr_filter_rebuild(); - recount_young_layout_records(); + if crate::hot_diag::layout_on() { + layout_diag_note_prune(rebuild_filter); } } +/// `PERRY_LAYOUT_DIAG`'s per-prune sample. Out of line and behind +/// [`crate::hot_diag::layout_on`] so an unarmed build pays one relaxed load. +#[cold] +fn layout_diag_note_prune(rebuilt_filter: bool) { + let (typed_len, masks_len) = ( + hot_typed_layouts().borrow().len(), + hot_layout_slot_masks().borrow().len(), + ); + let hint = hot_per_object_layout_hint(); + // SAFETY: as in the pass above — this thread's own filter, no other + // reference live. + let set = unsafe { + (*hint.filter.get()) + .iter() + .map(|w| w.count_ones() as usize) + .sum::() + }; + crate::hot_diag::layout_note_prune( + typed_len, + masks_len, + set, + LAYOUT_ADDR_FILTER_BITS, + rebuilt_filter, + layout_addr_filter_saturating_occupancy(), + ); +} + #[cfg(test)] pub(in crate::gc) fn test_per_object_layout_present(user_ptr: usize) -> bool { hot_layout_slot_masks().borrow().contains_key(&user_ptr) @@ -366,18 +425,74 @@ pub(in crate::gc) fn layout_addr_filter_note(user_ptr: usize) { /// their own (two keys may share one), so this is what keeps a workload that /// genuinely churns per-object records from saturating the filter forever. fn layout_addr_filter_rebuild() { + let occupancy = hot_layout_slot_masks().borrow().len() + hot_typed_layouts().borrow().len(); + if occupancy > layout_addr_filter_saturating_occupancy() { + // Walking the keys would set almost every bit, so this is where the + // rebuild lands anyway — reached in O(1) instead of O(live keys). + layout_addr_filter_saturate(); + return; + } layout_addr_filter_clear(); - let keys: Vec = { - let masks = hot_layout_slot_masks().borrow(); - let typed = hot_typed_layouts().borrow(); - masks.keys().copied().chain(typed.keys().copied()).collect() - }; let hint = hot_per_object_layout_hint(); - for k in keys { + // Straight from the tables: the `Vec` of every live key that used + // to buffer this walk allocated 50.6 MB per 400-character cc reply, and + // bought nothing — no borrow held here conflicts with the filter, which + // lives in a different thread-local. + let set_bit = |k: usize| { let (word, bit) = layout_addr_filter_slot(k); + // SAFETY: this thread's own filter, no other reference live; the + // tables borrowed around it are different thread-locals. unsafe { (*hint.filter.get())[word] |= bit; } + }; + for k in hot_layout_slot_masks().borrow().keys() { + set_bit(*k); + } + for k in hot_typed_layouts().borrow().keys() { + set_bit(*k); + } + hint.sets.set(0); +} + +/// Live keys past which the 4,096-bit sketch stops being an accelerator. +/// +/// The filter is a one-hash bitmap, so `n` live keys leave it answering +/// "may hold" for about `1 - e^(-n/4096)` of all addresses: 12 % at 512 keys, +/// 63 % at 4,096, and indistinguishable from "always yes" past ~16k. cc holds +/// **162,258** (`PERRY_LAYOUT_DIAG`, one 400-character reply), i.e. every bit +/// set, every probe positive, and every rebuild an O(live keys) walk that +/// restores exactly the all-ones state it started from. +/// +/// Sizing the filter up is not available here: the geometry and hash are +/// mirrored in generated code (`perry-codegen`'s +/// `emit_gated_forget_object_layout`), so widening it is a codegen change, and +/// a sketch that discriminated at 162k keys would need ~1.5 Mbit — 190 KB of +/// inline thread-local storage on every thread, to serve a workload that has +/// already lost the fast path. What is available is to stop *paying* for a +/// gate that cannot pay back: past this occupancy the filter is set to all +/// ones, which is the conservative answer it would have reached anyway, and +/// the walk is skipped. Nothing downstream changes behaviour — `may_hold` is +/// a hint whose `true` every caller already handles. +/// +/// Four times the bit count is deliberately far past the point where the +/// filter merely *degrades*: at 16,384 keys its false-positive rate is 98.2 %, +/// so a rebuilt filter still proves absence for under one address in fifty +/// while costing a walk of every live key. Below that the filter is left +/// exactly as it was — a workload holding a few thousand records keeps the +/// selectivity it has today, and this branch never fires for it. +#[inline] +fn layout_addr_filter_saturating_occupancy() -> usize { + LAYOUT_ADDR_FILTER_BITS * 4 +} + +/// Set every bit: "may hold" for any address. Conservative by construction — +/// the filter's `false` is the only load-bearing answer. +fn layout_addr_filter_saturate() { + let hint = hot_per_object_layout_hint(); + // SAFETY: this thread's own filter, no other reference live. + unsafe { + (*hint.filter.get()).fill(u64::MAX); } hint.sets.set(0); } @@ -809,6 +924,15 @@ pub(in crate::gc) fn transfer_per_object_descriptor(old_user: usize, new_user: u return false; } let mut typed = hot_typed_layouts().borrow_mut(); + // The flag and the filter above are shared with `LAYOUT_SLOT_MASKS`, so a + // full mask table drags every relocation in here even when this map is + // empty — which is cc's steady state (`PERRY_LAYOUT_DIAG`: typed=0, + // masks=162,258). An empty map has nothing to remove at either address, so + // the two hashes below are pure loss; the `len` test that proves it is one + // load. #9792. + if typed.is_empty() { + return false; + } typed.remove(&new_user); match typed.remove(&old_user) { Some(layout) => { diff --git a/crates/perry-runtime/src/hot_diag.rs b/crates/perry-runtime/src/hot_diag.rs index faa49fba65..b4bd5238ee 100644 --- a/crates/perry-runtime/src/hot_diag.rs +++ b/crates/perry-runtime/src/hot_diag.rs @@ -324,6 +324,154 @@ fn ic_sink() -> &'static Option { }) } +// --------------------------------------------------------------------------- +// Per-object layout tables: occupancy vs the address filter that gates them +// --------------------------------------------------------------------------- + +static LAYOUT_SINK: OnceLock> = OnceLock::new(); +static LAYOUT_ON: AtomicBool = AtomicBool::new(false); + +fn layout_sink() -> &'static Option { + LAYOUT_SINK.get_or_init(|| { + let sink = sink_from_env("PERRY_LAYOUT_DIAG"); + LAYOUT_ON.store(sink.is_some(), Ordering::Relaxed); + sink + }) +} + +/// Is the per-object-layout occupancy instrument armed? +#[inline] +pub fn layout_on() -> bool { + if LAYOUT_SINK.get().is_none() { + layout_sink(); + } + LAYOUT_ON.load(Ordering::Relaxed) +} + +/// One collection's view of the per-object layout tables and the 4096-bit +/// address filter that is supposed to keep evacuation off them. +/// +/// The question this exists to settle: `layout_addr_filter_may_hold` is +/// documented for "one or two entries … ~0.05 % false-positive rate", and +/// `transfer_per_object_descriptor` / `transfer_per_object_slot_mask` return +/// early only when it says no. If the tables hold far more keys than the +/// filter has bits, it answers "maybe" for every address, both early returns +/// stop firing, and every evacuated object pays the full two-map hash path — +/// while nothing in the system says so out loud. +/// +/// `keys` is also what a filter rebuild used to cost: one `Vec` of +/// every live key per prune, plus a second full walk to recount the young +/// records. +#[derive(Default)] +pub struct LayoutDiag { + prunes: u64, + typed_len: usize, + masks_len: usize, + typed_max: usize, + masks_max: usize, + /// Set bits in the address filter, and its capacity in bits. + filter_bits_set: usize, + filter_bits_total: usize, + filter_bits_set_max: usize, + /// Live keys past which a rebuild stops producing a selective filter. + useful_keys: usize, + /// Prunes that rebuilt the filter from the survivors, and prunes that + /// found it already outgrown and skipped the walk. + rebuilt: u64, + outgrown: u64, + /// Keys visited by prunes that DID rebuild — the walk that is still paid. + rebuilt_keys: u64, +} + +crate::perry_thread_local! { + static LAYOUT_DIAG: RefCell = RefCell::new(LayoutDiag::default()); +} + +/// Record one death-prune's occupancy. `rebuilt_filter` says whether this +/// prune rebuilt the address filter from its survivors, or found the tables +/// too full for a 4,096-bit sketch to discriminate and saturated it instead. +pub fn layout_note_prune( + typed_len: usize, + masks_len: usize, + filter_bits_set: usize, + filter_bits_total: usize, + rebuilt_filter: bool, + useful_keys: usize, +) { + LAYOUT_DIAG.with(|d| { + let mut d = d.borrow_mut(); + d.prunes += 1; + d.typed_len = typed_len; + d.masks_len = masks_len; + d.typed_max = d.typed_max.max(typed_len); + d.masks_max = d.masks_max.max(masks_len); + d.filter_bits_set = filter_bits_set; + d.filter_bits_total = filter_bits_total; + d.filter_bits_set_max = d.filter_bits_set_max.max(filter_bits_set); + d.useful_keys = useful_keys; + if rebuilt_filter { + d.rebuilt += 1; + d.rebuilt_keys += (typed_len + masks_len) as u64; + } else { + d.outgrown += 1; + } + let text = d.render(); + if let Some(sink) = layout_sink() { + write_sink(sink, &text); + } + }); +} + +impl LayoutDiag { + fn render(&self) -> String { + use std::fmt::Write as _; + let mut out = String::with_capacity(512); + let pct = |n: usize, d: usize| { + if d == 0 { + 0.0 + } else { + 100.0 * n as f64 / d as f64 + } + }; + let keys = self.typed_len + self.masks_len; + let _ = writeln!( + out, + "[layout-diag] prunes={} keys_now={} (typed={} masks={}) keys_max={} \ + (typed={} masks={})", + self.prunes, + keys, + self.typed_len, + self.masks_len, + self.typed_max + self.masks_max, + self.typed_max, + self.masks_max + ); + let _ = writeln!( + out, + " filter: {}/{} bits set ({:.1} %), max {}/{} ({:.1} %); selective up to \ + {} keys, so this table is {}", + self.filter_bits_set, + self.filter_bits_total, + pct(self.filter_bits_set, self.filter_bits_total), + self.filter_bits_set_max, + self.filter_bits_total, + pct(self.filter_bits_set_max, self.filter_bits_total), + self.useful_keys, + if keys > self.useful_keys { + "OUTGROWN it -- every probe answers `may hold`" + } else { + "within it" + } + ); + let _ = writeln!( + out, + " filter rebuilds={} over {} keys walked; outgrown-and-skipped={}", + self.rebuilt, self.rebuilt_keys, self.outgrown + ); + out + } +} + /// Is the IC-miss instrument armed? One relaxed load once initialised. #[inline] pub fn ic_on() -> bool { diff --git a/crates/perry-runtime/src/iter_result.rs b/crates/perry-runtime/src/iter_result.rs index 832e567d81..f0125407b3 100644 --- a/crates/perry-runtime/src/iter_result.rs +++ b/crates/perry-runtime/src/iter_result.rs @@ -189,6 +189,71 @@ pub(crate) unsafe fn make_sqlite_iter_result(value: JSValue, done: bool) -> f64 build_iter_result_ordered(JSValue::bool(done), value, IterResultOrder::DoneValue) } +/// Emit a `{ value, done }` for the FUSED `for…of` driver, recycling ONE +/// result object per ITERATOR instead of allocating one per element. +/// +/// `emit_cached == false` — every manual `.next()` and both public +/// dispatchers — allocates a fresh object, exactly as before, so a result the +/// caller retains behaves per spec. +/// +/// `emit_cached == true` is reserved for +/// [`crate::collection_iter_object::js_for_of_next`], whose only caller is the +/// compiler's `for…of` desugar. There the result local is a compiler temporary +/// the loop body cannot name, and the driver reads `done` and `value` out of it +/// before the next advance — so mutating one cached object per ITERATOR is +/// unobservable. The cache lives in the iterator object's `cache_field`, so the +/// GC traces and rewrites it like any other field. +/// +/// The Map/Set iterators have worked this way since the fused driver landed; +/// this is the same routine, lifted here so the array iterator uses the ONE +/// implementation rather than a second copy — which is the drift `#7564` +/// removed from the five result constructors this module replaced. +pub(crate) unsafe fn emit_iter_result_cached( + scope: &crate::gc::RuntimeHandleScope, + iter_h: &crate::gc::RuntimeHandle<'_>, + cache_field: u32, + emit_cached: bool, + order: IterResultOrder, + value: JSValue, + done: bool, +) -> f64 { + let (first, second) = match order { + IterResultOrder::ValueDone => (value, JSValue::bool(done)), + IterResultOrder::DoneValue => (JSValue::bool(done), value), + }; + if !emit_cached { + return build_iter_result_ordered(first, second, order); + } + let iter_obj = || crate::js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; + let cached = crate::object::js_object_get_field(iter_obj(), cache_field); + if JSValue::from_bits(cached.bits()).is_pointer() { + let res = crate::js_nanbox_get_pointer(f64::from_bits(cached.bits())) as *mut ObjectHeader; + // Barriered field stores: the iterator (and its cached result) may be + // tenured while `value` is young. + crate::object::js_object_set_field(res, 0, first); + crate::object::js_object_set_field(res, 1, second); + return crate::js_nanbox_pointer(res as i64); + } + // First fused advance on this iterator: build the result once and cache it. + // `build_iter_result_ordered` allocates, so root both field values across + // it, and re-read the iterator and the result through storage the collector + // rewrites afterwards. + let first_h = scope.root_nanbox_u64(first.bits()); + let second_h = scope.root_nanbox_u64(second.bits()); + let res = build_iter_result_ordered( + JSValue::from_bits(first_h.get_nanbox_u64()), + JSValue::from_bits(second_h.get_nanbox_u64()), + order, + ); + let res_h = scope.root_nanbox_f64(res); + crate::object::js_object_set_field( + iter_obj(), + cache_field, + JSValue::from_bits(res_h.get_nanbox_f64().to_bits()), + ); + res_h.get_nanbox_f64() +} + /// GC root scanner for the shared keys arrays. /// /// MARKING: nothing else in the heap references these arrays — the result diff --git a/crates/perry-runtime/src/object/reserved_floor.rs b/crates/perry-runtime/src/object/reserved_floor.rs index ffd744d278..ea8cfebdba 100644 --- a/crates/perry-runtime/src/object/reserved_floor.rs +++ b/crates/perry-runtime/src/object/reserved_floor.rs @@ -29,8 +29,8 @@ use crate::array::ArrayHeader; /// `0` for every class id without a reserved raw-field layout. Keep each /// entry in lock-step with the family's allocator/dispatcher: /// -/// * array: `array/iter_object.rs` (fields 0..4: backing, cursor, kind, -/// snapshot len, epoch) +/// * array: `array/iter_object.rs` (fields 0..5: backing, cursor, kind, +/// `node:sqlite` epoch pointer, epoch value, cached fused result) /// * map/set: `collection_iter_object.rs` (fields 0..5: backing, cursor, /// kind, size-at-last-next, last key, cached fused result) /// * string: `string/iter_object.rs` (fields 0..1) @@ -39,7 +39,7 @@ use crate::array::ArrayHeader; /// * iterator helpers: `iterator_helpers.rs` (fields 0..3) pub(crate) fn reserved_slot_floor_for_class_id(class_id: u32) -> u32 { match class_id { - crate::array::ARRAY_ITERATOR_CLASS_ID => 5, + crate::array::ARRAY_ITERATOR_CLASS_ID => 6, crate::collection_iter_object::MAP_ITERATOR_CLASS_ID | crate::collection_iter_object::SET_ITERATOR_CLASS_ID => 6, crate::string::STRING_ITERATOR_CLASS_ID => 2, @@ -242,7 +242,7 @@ mod tests { fn floors_cover_every_reserved_family_and_nothing_else() { assert_eq!( reserved_slot_floor_for_class_id(crate::array::ARRAY_ITERATOR_CLASS_ID), - 5 + 6 ); assert_eq!( reserved_slot_floor_for_class_id(crate::collection_iter_object::MAP_ITERATOR_CLASS_ID), diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index b52c8629f5..f64aef019e 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -323,6 +323,12 @@ "verdict": "not_a_gc_pointer", "why": "Inline-cache miss diagnostics (`PERRY_IC_DIAG`). `IcDiag` is two `Instant`s, three counters, and `sites: HashMap` whose KEY is a PIC cache-slot address \u2014 malloc'd arena storage from `field_get_set/ic_slot.rs`, never GC heap \u2014 and whose value is a `String` plus counters. Nothing here is a managed pointer." }, + { + "file": "crates/perry-runtime/src/hot_diag.rs", + "name": "LAYOUT_DIAG", + "verdict": "not_a_gc_pointer", + "why": "#9807 layout-prune diagnostics (`PERRY_LAYOUT_DIAG`), off unless armed. Every field is a count, a length, or a running maximum (`prunes`, `typed_len`, `masks_len`, `typed_max`, `masks_max`, `filter_bits_*`, `useful_keys`, `rebuilt`). No field stores an address." + }, { "file": "crates/perry-runtime/src/hot_diag.rs", "name": "REGEX_DIAG",