From 2a97c3e09bf3cc8d9165f1ff9ec44dbc06b1afd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 07:29:48 +0000 Subject: [PATCH 1/3] perf(runtime,hir): fuse the for-of IteratorNext into one runtime call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic `for…of` desugar performed, per element: a dynamic `.next()` method dispatch, a separate IteratorNext result validation call, an override probe that allocated a "next" key string and ran a by-name prototype lookup, and — for builtin collection iterators — a fresh `{value, done}` allocation. On the ECS archetype-migration profile the two collection-iterator dispatchers were 8.6% of the row, nearly all of it protocol overhead. Three changes, one entry: * `js_for_of_next(iter)` — the desugar's next-call becomes a single extern call. A builtin Map/Set iterator advances in place through the SAME dispatcher arm the manual path uses (override probe included) and reuses a cached `{value, done}` object held in the iterator's new sixth field; the cache is sound because the entry's only caller is the compiler's desugar, whose result local is a compiler temporary the loop body cannot name. Every other receiver takes the generic arm, which is byte-for-byte the two-call shape this replaces (dynamic `.next()` + validation), so generators, array iterators and user iterators are unchanged. * Manual `.next()` calls and both public dispatchers keep allocating fresh results — a caller that retains results observes spec behavior. * The override probe no longer BUILDS the prototype tower to answer "was `next` patched?": a null tower proves no override exists, because the only way user code reaches the prototype object is `Object.getPrototypeOf`, which materializes it. This also removes a per-`.next()` key-string allocation from the manual path. The sync desugar arm that previously skipped validation now routes through the fused entry as well, which validates on the generic arm — the spec IteratorNext behavior the other driver already had. Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby --- .../src/runtime_decls/strings.rs | 1 + crates/perry-hir/src/lower/stmt_loops.rs | 28 +-- .../src/collection_iter_object.rs | 213 +++++++++++++++++- .../src/object/iterator_prototypes.rs | 17 +- 4 files changed, 237 insertions(+), 22 deletions(-) diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index ec1816ad40..71566af703 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1542,6 +1542,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_with_implicit_read", DOUBLE, &[DOUBLE, DOUBLE]); // Iterator-protocol result validation (for-of lazy loop). module.declare_function("js_iterator_result_validate", DOUBLE, &[DOUBLE]); + module.declare_function("js_for_of_next", DOUBLE, &[DOUBLE]); module.declare_function("js_global_get_or_throw_unresolved", DOUBLE, &[DOUBLE]); // Ambient `require` for compiled external / compilePackages modules (#5373): // bind a bare `require` to a createRequire-backed closure instead of throwing diff --git a/crates/perry-hir/src/lower/stmt_loops.rs b/crates/perry-hir/src/lower/stmt_loops.rs index 34c1ec31cc..cb8460f179 100644 --- a/crates/perry-hir/src/lower/stmt_loops.rs +++ b/crates/perry-hir/src/lower/stmt_loops.rs @@ -348,18 +348,20 @@ fn iterator_result_validated(call: Expr) -> Expr { } } -/// `__iter.next()` (validated: a non-object result is a TypeError). +/// One fused IteratorNext (`js_for_of_next(__iter)`): builtin Map/Set +/// iterators advance in place; everything else runs the dynamic `.next()` +/// plus result validation inside the entry — the two-call shape this emitted. pub(crate) fn iterator_next_call(iter_id: LocalId) -> Expr { - iterator_result_validated(Expr::Call { - callee: Box::new(Expr::PropertyGet { - byte_offset: 0, - object: Box::new(Expr::LocalGet(iter_id)), - property: "next".to_string(), + Expr::Call { + callee: Box::new(Expr::ExternFuncRef { + name: "js_for_of_next".to_string(), + param_types: vec![Type::Any], + return_type: Type::Any, }), - args: vec![], + args: vec![Expr::LocalGet(iter_id)], type_args: vec![], byte_offset: 0, - }) + } } /// Iterator-driver loop with the ADVANCE AT THE TOP of the body: @@ -368,11 +370,9 @@ pub(crate) fn iterator_next_call(iter_id: LocalId) -> Expr { /// advance. The previous shape — `while (!__result.done) { ; /// __result = next() }` — put the advance at the body TAIL, so a `continue` /// skipped it and re-processed the SAME result forever (the footgun -/// `lazy_iter_for_stmt` documents; it can use `Stmt::For`'s update clause, -/// but the await-capable drivers here cannot carry an `await` there, so they -/// use this shape). Canonical spin: an SSE consumer's -/// `for await (...) { if (ev === "ping") continue; ... }` hung a large -/// esbuild-bundled CLI app on the first real server ping. +/// the footgun `lazy_iter_for_stmt` documents; the await-capable drivers +/// here cannot carry an `await` in a `for` update clause, so they use this +/// shape). An SSE consumer's `continue` on ping hung a bundled CLI app. /// /// The synthetic `if done break` is appended AFTER /// `insert_iterator_return_before_abrupts` runs over the user body, so the @@ -981,7 +981,7 @@ pub(super) fn lower_stmt_for_of_inner( let next_call = if needs_await { Expr::Await(Box::new(raw_next_call)) } else { - raw_next_call + iterator_next_call(iter_id) }; module.init.push(Stmt::Let { id: result_id, diff --git a/crates/perry-runtime/src/collection_iter_object.rs b/crates/perry-runtime/src/collection_iter_object.rs index 839513b8cc..cb04007f73 100644 --- a/crates/perry-runtime/src/collection_iter_object.rs +++ b/crates/perry-runtime/src/collection_iter_object.rs @@ -61,7 +61,7 @@ fn iterator_class_id(addr: usize) -> Option { unsafe fn alloc_iterator(class_id: u32, coll_nanboxed: f64, kind: i32) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let coll_h = scope.root_nanbox_f64(coll_nanboxed); - let obj_h = scope.root_raw_mut_ptr(js_object_alloc(class_id, 5)); + let obj_h = scope.root_raw_mut_ptr(js_object_alloc(class_id, 6)); let obj = || obj_h.across_mut::(|| ()).1; // Field 0: backing collection (NaN-boxed pointer so the GC scanner keeps it). js_object_set_field( @@ -81,6 +81,11 @@ unsafe fn alloc_iterator(class_id: u32, coll_nanboxed: f64, kind: i32) -> f64 { // Field 4: the KEY of the last-returned entry (a Map key / Set value), used // to re-derive the cursor after a delete-shift. Undefined until started. js_object_set_field(obj(), 4, JSValue::undefined()); + // Field 5: the recycled `{value, done}` result the FUSED for-of driver + // mutates in place (one allocation per loop, not per element). Manual + // `.next()` calls never touch it — they keep returning fresh objects, so + // a caller that retains results observes spec behavior. + js_object_set_field(obj(), 5, JSValue::undefined()); // Link `[[Prototype]]` to the shared `%MapIteratorPrototype%` / // `%SetIteratorPrototype%` singleton so `Object.getPrototypeOf(it)` and the // inherited `.next` read resolve. @@ -233,6 +238,14 @@ fn next_read_index(cursor: u32, last_key_in_place: bool, find_last: impl FnOnce( /// Dispatch `.next()` / `[Symbol.iterator]()` on a Map iterator object. pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 { + dispatch_map_iterator_method_emit(iter_obj, method_name, false) +} + +unsafe fn dispatch_map_iterator_method_emit( + iter_obj: *mut ObjectHeader, + method_name: &str, + emit_cached: bool, +) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64)); let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; @@ -248,7 +261,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n let map = || js_nanbox_get_pointer(map_h.get_nanbox_f64()) as *const MapHeader; let kind = f64::from_bits(js_object_get_field(iter_obj(), 2).bits()) as i32; if map().is_null() { - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32; let last_key = js_object_get_field(iter_obj(), 4); @@ -268,7 +281,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n // Once a collection iterator is exhausted it stays exhausted, // even if entries are appended later. js_object_set_field(iter_obj(), 0, JSValue::undefined()); - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let entry_key = crate::map::js_map_entry_key_at(map(), idx); @@ -286,7 +299,7 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n JSValue::from_bits(make_pair_array(entry_key, val).to_bits()) } }; - make_iter_result(value, false) + emit_iter_result(&scope, &iter_h, emit_cached, value, false) } "Symbol.iterator" | "@@iterator" => js_nanbox_pointer(iter_obj() as i64), "return" | "throw" => make_iter_result(JSValue::undefined(), true), @@ -296,6 +309,14 @@ pub unsafe fn dispatch_map_iterator_method(iter_obj: *mut ObjectHeader, method_n /// Dispatch `.next()` / `[Symbol.iterator]()` on a Set iterator object. pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_name: &str) -> f64 { + dispatch_set_iterator_method_emit(iter_obj, method_name, false) +} + +unsafe fn dispatch_set_iterator_method_emit( + iter_obj: *mut ObjectHeader, + method_name: &str, + emit_cached: bool, +) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); let iter_h = scope.root_nanbox_f64(js_nanbox_pointer(iter_obj as i64)); let iter_obj = || js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut ObjectHeader; @@ -311,7 +332,7 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n let set = || js_nanbox_get_pointer(set_h.get_nanbox_f64()) as *const SetHeader; let kind = f64::from_bits(js_object_get_field(iter_obj(), 2).bits()) as i32; if set().is_null() { - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32; let last_val = js_object_get_field(iter_obj(), 4); @@ -326,7 +347,7 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n if idx >= size { js_object_set_field(iter_obj(), 1, JSValue::number(size as f64)); js_object_set_field(iter_obj(), 0, JSValue::undefined()); - return make_iter_result(JSValue::undefined(), true); + return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } let elem = crate::set::js_set_value_at(set(), idx); @@ -338,10 +359,188 @@ pub unsafe fn dispatch_set_iterator_method(iter_obj: *mut ObjectHeader, method_n KIND_ENTRIES => JSValue::from_bits(make_pair_array(elem, elem).to_bits()), _ => JSValue::from_bits(elem.to_bits()), }; - make_iter_result(value, false) + emit_iter_result(&scope, &iter_h, emit_cached, value, false) } "Symbol.iterator" | "@@iterator" => js_nanbox_pointer(iter_obj() as i64), "return" | "throw" => make_iter_result(JSValue::undefined(), true), _ => f64::from_bits(TAG_UNDEFINED), } } + +/// 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. +unsafe fn emit_iter_result( + scope: &crate::gc::RuntimeHandleScope, + iter_h: &crate::gc::RuntimeHandle, + emit_cached: bool, + 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() +} + +/// 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. +#[no_mangle] +pub unsafe extern "C-unwind" fn js_for_of_next(iter: f64) -> f64 { + let jv = JSValue::from_bits(iter.to_bits()); + if jv.is_pointer() { + let raw = js_nanbox_get_pointer(iter) as usize; + if raw != 0 && !crate::value::addr_class::is_small_handle(raw) { + if let Some(header) = crate::value::addr_class::try_read_gc_header(raw) { + if header.obj_type == crate::gc::GC_TYPE_OBJECT { + let obj = raw as *mut ObjectHeader; + let class_id = (*obj).class_id; + if class_id == MAP_ITERATOR_CLASS_ID { + return dispatch_map_iterator_method_emit(obj, "next", true); + } + if class_id == SET_ITERATOR_CLASS_ID { + return dispatch_set_iterator_method_emit(obj, "next", true); + } + } + } + } + } + let result = crate::object::js_native_call_method( + iter, + b"next".as_ptr() as *const i8, + 4, + std::ptr::null(), + 0, + ); + crate::symbol::js_iterator_result_validate(result) +} + +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_FOR_OF_NEXT: unsafe extern "C-unwind" fn(f64) -> f64 = js_for_of_next; + +#[cfg(test)] +mod fused_for_of_tests { + use super::*; + + unsafe fn value_of(res: f64) -> f64 { + f64::from_bits( + js_object_get_field(js_nanbox_get_pointer(res) as *mut ObjectHeader, 0).bits(), + ) + } + unsafe fn done_of(res: f64) -> bool { + JSValue::from_bits( + js_object_get_field(js_nanbox_get_pointer(res) as *mut ObjectHeader, 1).bits(), + ) + .as_bool() + } + + /// The fused driver must walk a Set in insertion order, terminate, and + /// keep its recycled result in the iterator's field 5 — while the manual + /// dispatcher keeps allocating fresh results a caller may retain. + #[test] + fn fused_next_walks_a_set_and_recycles_its_result() { + unsafe { + let set = crate::set::js_set_alloc(4); + for v in [10.0f64, 20.0, 30.0] { + crate::set::js_set_add(set, v); + } + let iter = js_nanbox_pointer(js_set_values_iter_obj(set)); + + let r1 = js_for_of_next(iter); + assert_eq!(value_of(r1), 10.0); + assert!(!done_of(r1)); + let cached = js_object_get_field(js_nanbox_get_pointer(iter) as *mut ObjectHeader, 5); + assert!( + JSValue::from_bits(cached.bits()).is_pointer(), + "the first fused advance must install the recycled result" + ); + assert_eq!(value_of(js_for_of_next(iter)), 20.0); + assert_eq!(value_of(js_for_of_next(iter)), 30.0); + assert!(done_of(js_for_of_next(iter)), "exhausted after three"); + assert!(done_of(js_for_of_next(iter)), "stays exhausted"); + + // The manual path still returns fresh, independent results. + let m1 = dispatch_set_iterator_method( + js_nanbox_get_pointer(js_nanbox_pointer(js_set_values_iter_obj(set))) + as *mut ObjectHeader, + "next", + ); + assert_eq!(value_of(m1), 10.0); + } + } + + /// Mid-iteration delete: the cursor-repair contract (#6075) must hold on + /// the fused path because it runs the SAME advance code as the manual one. + #[test] + fn fused_next_survives_a_mid_iteration_delete() { + unsafe { + let map = crate::map::js_map_alloc(8); + for k in [1.0f64, 2.0, 3.0, 4.0] { + crate::map::js_map_set(map, k, k * 10.0); + } + let iter = js_nanbox_pointer(js_map_keys_iter_obj(map)); + assert_eq!(value_of(js_for_of_next(iter)), 1.0); + // Deleting an EARLIER entry shifts the survivors down; the fused + // next must not skip or repeat. + crate::map::js_map_delete(map, 1.0); + assert_eq!(value_of(js_for_of_next(iter)), 2.0); + assert_eq!(value_of(js_for_of_next(iter)), 3.0); + assert_eq!(value_of(js_for_of_next(iter)), 4.0); + assert!(done_of(js_for_of_next(iter))); + } + } + + /// A non-collection receiver takes the generic arm: dynamic `.next()` + /// dispatch plus validation — here, an array VALUES iterator object. + #[test] + fn fused_next_routes_other_iterators_through_the_generic_arm() { + 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); + assert!(done_of(js_for_of_next(iter))); + } + } +} diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index a30778ab26..6c77ae53f4 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -264,6 +264,12 @@ fn build_family_proto( } /// Lazily build the prototypes (idempotent). Cheap after the first call. +/// Whether any iterator-prototype tower has been materialized on this thread. +#[cfg(test)] +pub(crate) fn iterator_prototypes_materialized() -> bool { + ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) != 0 +} + pub(crate) fn ensure_iterator_prototypes() { if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == 0 { build_iterator_prototypes(); @@ -323,7 +329,16 @@ pub(crate) unsafe fn call_overridden_iterator_next( let scope = crate::gc::RuntimeHandleScope::new(); let iter = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(iter_obj as i64)); let previous = scope.root_nanbox_f64(super::js_implicit_this_get()); - ensure_iterator_prototypes(); + // An override can only be installed through the prototype OBJECT, and the + // only way user code obtains that object is `Object.getPrototypeOf(iter)` + // (or a direct prototype write), both of which materialize the tower. A + // null tower therefore PROVES no override exists — and building the tower + // here, as this probe used to, made every builtin `.next()` allocate a + // "next" key string and run a by-name prototype lookup just to learn + // nothing was patched. + if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == 0 { + return None; + } let (slot, canonical): (&crate::object::RealmAtomicI64, *const u8) = match class_id { crate::array::ARRAY_ITERATOR_CLASS_ID => ( &ARRAY_ITERATOR_PROTOTYPE_PTR, From 6f211cdfcd385fa5dbcc640c95937eae641636b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 08:08:31 +0000 Subject: [PATCH 2/3] perf(runtime): O(1) ordered Map deletes via tombstoned entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emptying an N-entry Map cost O(N^2) three ways at once: every ordered delete memmoved the trailing entries down (128 MB moved per ECS migration round), span-barriered every moved slot, and walked all three side indexes decrementing every offset above the hole. Node's per-delete cost is ~0.03 us flat; perry's grew without bound (263x node at N=16k). A delete now TOMBSTONES the entry in place: the key slot takes the reserved hole marker (`TAG_HOLE` — never a legal stored key, `normalize_zero` canonicalizes a leaked array hole to `undefined` first), the value slot is cleared through the barriered store so SATB marking still shades the overwritten child, and the live count drops while the array extent (`used`, a new MapHeader field pinned at offset 32) stays put. Raw entry indices are therefore STABLE: nothing shifts, no span barrier over the tail, and the side indexes only forget the deleted key — no offset repair at all. Insertion order (#2831) is unchanged — iteration walks raw indices and skips holes, and a delete-then-re-add still appends at the end. Tombstones are squeezed out when they outnumber the live entries, or before growing (so a delete-heavy map reclaims holes instead of doubling on dead weight), or when a raw-indexed accessor observes them — after which the typed for-of lane's new `used == size` admission holds again and the lane self-heals. The GC's Map slot descriptor bounds its Range by `used` (holes are non-pointer markers the tag-filtered scan skips), with `size <= used <= capacity` as the corruption guard; the hand-built MapHeaders in the GC test fixtures now initialize the extent they predate. Set.delete keeps its (already index-repair-free, #8993) compacting form; the same tombstone treatment is a follow-up. Claude-Session: https://claude.ai/code/session_01FUvFrRNZyc5qknBiJbYbby --- crates/perry-codegen/src/expr/arrays_finds.rs | 11 +- .../src/collection_iter_object.rs | 24 +- .../perry-runtime/src/gc/layout_slot_visit.rs | 5 +- crates/perry-runtime/src/gc/tests/barrier.rs | 8 + .../src/gc/tests/helper_stores.rs | 3 + crates/perry-runtime/src/map.rs | 367 ++++++++++++------ .../perry-runtime/src/map_tombstone_tests.rs | 163 ++++++++ .../src/object/iterator_prototypes.rs | 5 - 8 files changed, 455 insertions(+), 131 deletions(-) create mode 100644 crates/perry-runtime/src/map_tombstone_tests.rs diff --git a/crates/perry-codegen/src/expr/arrays_finds.rs b/crates/perry-codegen/src/expr/arrays_finds.rs index ab957c7d4a..ab4601a8ea 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -146,9 +146,18 @@ fn lower_map_entry_at_inline( let live = blk.icmp_eq(I8, &forwarded, "0"); let size_ptr = blk.inttoptr(I64, &m_handle); let size = blk.load(I32, &size_ptr); + // Tombstoned deletes leave `used > size`; a raw entry read is only + // dense-correct with no holes present, so a holey map falls back to + // the runtime helper — which compacts, after which this admission + // holds again (the lane self-heals). + let used_addr = blk.add(I64, &m_handle, "32"); + let used_ptr = blk.inttoptr(I64, &used_addr); + let used = blk.load(I32, &used_ptr); + let dense = blk.icmp_eq(I32, &used, &size); let in_range = blk.icmp_ult(I32, &i_i32, &size); let a = blk.and(I1, &is_map, &live); - let admitted = blk.and(I1, &a, &in_range); + let b = blk.and(I1, &a, &dense); + let admitted = blk.and(I1, &b, &in_range); blk.cond_br(&admitted, &fast_label, &slow_label); } ctx.current_block = fast_idx; diff --git a/crates/perry-runtime/src/collection_iter_object.rs b/crates/perry-runtime/src/collection_iter_object.rs index cb04007f73..5cdedf6669 100644 --- a/crates/perry-runtime/src/collection_iter_object.rs +++ b/crates/perry-runtime/src/collection_iter_object.rs @@ -265,26 +265,34 @@ unsafe fn dispatch_map_iterator_method_emit( } let cursor = f64::from_bits(js_object_get_field(iter_obj(), 1).bits()) as u32; let last_key = js_object_get_field(iter_obj(), 4); - let size = crate::map::js_map_size(map()); + let used = crate::map::map_used_entries(map()); // Is the last-returned key still at cursor-1? (SameValueZero, so a // NaN key matches itself.) If so, no delete shifted an entry at/below // the cursor. let in_place = cursor > 0 && { - let prev = crate::map::js_map_entry_key_at(map(), cursor - 1); + let prev = crate::map::map_entry_key_raw(map(), cursor - 1); crate::value::js_jsvalue_same_value_zero(prev, f64::from_bits(last_key.bits())) != 0 }; - let idx = next_read_index(cursor, in_place, || { + let mut idx = next_read_index(cursor, in_place, || { crate::map::find_key_index(map(), f64::from_bits(last_key.bits())) }); - if idx >= size { - js_object_set_field(iter_obj(), 1, JSValue::number(size as f64)); + // Tombstoned deletes leave holes in the raw entry order; the + // cursor walks raw indices, so step over them here. + while idx < used + && crate::map::map_entry_key_raw(map(), idx).to_bits() + == crate::map::MAP_HOLE_KEY_BITS + { + idx += 1; + } + if idx >= used { + js_object_set_field(iter_obj(), 1, JSValue::number(used as f64)); // Once a collection iterator is exhausted it stays exhausted, // even if entries are appended later. js_object_set_field(iter_obj(), 0, JSValue::undefined()); return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); } - let entry_key = crate::map::js_map_entry_key_at(map(), idx); + let entry_key = crate::map::map_entry_key_raw(map(), idx); // Record state for the next re-derive BEFORE any allocation below. js_object_set_field(iter_obj(), 1, JSValue::number((idx + 1) as f64)); js_object_set_field(iter_obj(), 4, JSValue::from_bits(entry_key.to_bits())); @@ -292,10 +300,10 @@ unsafe fn dispatch_map_iterator_method_emit( let value = match kind { KIND_KEYS => JSValue::from_bits(entry_key.to_bits()), KIND_VALUES => { - JSValue::from_bits(crate::map::js_map_entry_value_at(map(), idx).to_bits()) + JSValue::from_bits(crate::map::map_entry_value_raw(map(), idx).to_bits()) } _ => { - let val = crate::map::js_map_entry_value_at(map(), idx); + let val = crate::map::map_entry_value_raw(map(), idx); JSValue::from_bits(make_pair_array(entry_key, val).to_bits()) } }; diff --git a/crates/perry-runtime/src/gc/layout_slot_visit.rs b/crates/perry-runtime/src/gc/layout_slot_visit.rs index abe78b396d..b47c7444a1 100644 --- a/crates/perry-runtime/src/gc/layout_slot_visit.rs +++ b/crates/perry-runtime/src/gc/layout_slot_visit.rs @@ -215,6 +215,7 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( GcRewriteDescriptorKind::Map => { let map = user_ptr as *mut crate::map::MapHeader; let size = (*map).size; + let used = (*map).used; let capacity = (*map).capacity; // Corruption guard only: mirror Set's 16M bound (set.rs // gc_element_slot_range). Every GC walk (mark, copy, rewrite, @@ -222,7 +223,7 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( // lower cap makes larger maps invisible to the collector — // entries reachable only through a >cap map would be swept // while live and never rewritten after a move. - if size > capacity || size > 16_000_000 || (*map).entries.is_null() { + if size > used || used > capacity || used > 16_000_000 || (*map).entries.is_null() { return; } // Defensive tripwire (# fabricated-Map): if a fabricated Map @@ -248,7 +249,7 @@ pub(super) unsafe fn visit_gc_rewrite_slot_descriptors( return; } visit(GcMutableSlotDescriptor::Range { - range: HeapSlotRange::new((*map).entries as *mut u64, size as usize * 2), + range: HeapSlotRange::new((*map).entries as *mut u64, used as usize * 2), layout_kind: None, }); // #6759 phase 1: the metadata edge. This arm is the MARK path as diff --git a/crates/perry-runtime/src/gc/tests/barrier.rs b/crates/perry-runtime/src/gc/tests/barrier.rs index 7b2bfcb424..937f8a1ed2 100644 --- a/crates/perry-runtime/src/gc/tests/barrier.rs +++ b/crates/perry-runtime/src/gc/tests/barrier.rs @@ -15,6 +15,7 @@ unsafe fn alloc_old_test_map( let entries = std::alloc::alloc_zeroed(layout) as *mut u64; assert!(!entries.is_null()); (*map).size = 0; + (*map).used = 0; (*map).capacity = capacity; (*map).entries = entries as *mut f64; (map, entries, layout) @@ -26,6 +27,7 @@ unsafe fn retire_old_test_map( layout: std::alloc::Layout, ) { (*map).size = 0; + (*map).used = 0; (*map).capacity = 0; (*map).entries = std::ptr::null_mut(); std::alloc::dealloc(entries as *mut u8, layout); @@ -625,6 +627,7 @@ fn test_old_young_edge_verifier_accepts_map_external_slot() { let map_header = unsafe { header_from_user_ptr(map as *const u8) }; unsafe { (*map).size = 1; + (*map).used = 1; *entries = ptr_bits(young); (*map_header).gc_flags |= GC_FLAG_MARKED; } @@ -932,6 +935,7 @@ fn test_dirty_page_map_entry_scan_is_external_range_bounded() { let (map, entries, layout) = unsafe { alloc_old_test_map(2048) }; unsafe { (*map).size = 2048; + (*map).used = 2048; } let (dirty_idx, clean_idx) = unsafe { field_indices_on_distinct_pages(entries, 4096) }; let dirty_slot = unsafe { entries.add(dirty_idx) }; @@ -1045,6 +1049,7 @@ fn test_dirty_page_map_external_dedupes_and_clears() { let (map, entries, layout) = unsafe { alloc_old_test_map(16) }; unsafe { (*map).size = 16; + (*map).used = 16; *entries.add(1) = POINTER_TAG | young as u64; } let slot = unsafe { entries.add(1) }; @@ -1077,6 +1082,7 @@ fn test_dirty_page_map_realloc_span_marks_new_entries_pages() { let (map, entries, layout) = unsafe { alloc_old_test_map(1024) }; unsafe { (*map).size = 1024; + (*map).used = 1024; *entries.add(1023) = POINTER_TAG | young as u64; } let new_layout = std::alloc::Layout::from_size_align(2048 * 16, 8).unwrap(); @@ -1462,6 +1468,7 @@ fn test_incremental_barrier_marks_external_map_and_set_slots() { let (set, elements, set_layout) = unsafe { alloc_old_test_set(1) }; unsafe { (*map).size = 1; + (*map).used = 1; (*set).size = 1; } mark_user_ptr(map as usize); @@ -1687,6 +1694,7 @@ fn test_rewrite_remembered_dirty_range_updates_map_external_entry_span() { let (map, entries, layout) = unsafe { alloc_old_test_map(2048) }; unsafe { (*map).size = 2048; + (*map).used = 2048; } let (dirty_idx, clean_idx) = unsafe { field_indices_on_distinct_pages(entries, 4096) }; let dirty_slot = unsafe { entries.add(dirty_idx) }; diff --git a/crates/perry-runtime/src/gc/tests/helper_stores.rs b/crates/perry-runtime/src/gc/tests/helper_stores.rs index fcefac78cf..a553e8a7de 100644 --- a/crates/perry-runtime/src/gc/tests/helper_stores.rs +++ b/crates/perry-runtime/src/gc/tests/helper_stores.rs @@ -22,6 +22,7 @@ unsafe fn alloc_old_test_map( let entries = std::alloc::alloc_zeroed(layout) as *mut u64; assert!(!entries.is_null()); (*map).size = 0; + (*map).used = 0; (*map).capacity = capacity; (*map).entries = entries as *mut f64; (map, entries, layout) @@ -33,6 +34,7 @@ unsafe fn retire_old_test_map( layout: std::alloc::Layout, ) { (*map).size = 0; + (*map).used = 0; (*map).capacity = 0; (*map).entries = std::ptr::null_mut(); std::alloc::dealloc(entries as *mut u8, layout); @@ -102,6 +104,7 @@ fn map_and_set_external_helper_stores_preserve_young_children() { let (map, entries, layout) = unsafe { alloc_old_test_map(1) }; unsafe { (*map).size = 1; + (*map).used = 1; crate::gc::runtime_store_external_jsvalue_slot( map as usize, entries as usize, diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 11d1231429..704c0b9cd0 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -444,22 +444,6 @@ impl NumericIndex { } self.dense_key_count = 0; } - - fn repair_entry_indices_after_delete(&mut self, deleted_index: u32) { - for entry_index in self.hashed.values_mut() { - if *entry_index > deleted_index { - *entry_index -= 1; - } - } - if let Some(dense) = self.dense.as_mut() { - for entry_index in &mut dense.slots { - if *entry_index != DENSE_NUMERIC_EMPTY && *entry_index > deleted_index { - *entry_index -= 1; - } - } - } - } - fn allowed_dense_span(&self) -> usize { self.dense_key_count .saturating_mul(DENSE_NUMERIC_SPAN_FACTOR) @@ -1105,8 +1089,25 @@ pub struct MapHeader { /// marked as well as rewritten (#6812: an edge visited only on the rewrite /// path is invisible to marking). pub meta: *mut crate::object::ObjectMeta, + /// Extent of the entries array actually written: raw entry indices run + /// `0..used`. `size` stays the LIVE count, so `used - size` is the number + /// of tombstoned entries awaiting compaction. Appended last; codegen + /// reads it at offset 32 (pinned below). + pub used: u32, } +const _: () = { + assert!(std::mem::offset_of!(MapHeader, size) == 0); + assert!(std::mem::offset_of!(MapHeader, capacity) == 4); + assert!(std::mem::offset_of!(MapHeader, entries) == 8); + assert!(std::mem::offset_of!(MapHeader, used) == 32); +}; + +/// The tombstone a deleted entry's KEY slot takes. Never a legal stored key: +/// `normalize_zero` canonicalizes a leaked array hole to `undefined` before +/// any key reaches the entries buffer. +pub(crate) const MAP_HOLE_KEY_BITS: u64 = crate::value::TAG_HOLE; + /// Each map entry is 16 bytes (key + value, both as f64/JSValue) const ENTRY_SIZE: usize = 16; @@ -1133,6 +1134,12 @@ unsafe fn entries_ptr_mut(map: *mut MapHeader) -> *mut f64 { /// so `v == 0.0` stays false for them (NaN-tagged f64 is never equal to 0.0). #[inline(always)] fn normalize_zero(key: f64) -> f64 { + if key.to_bits() == MAP_HOLE_KEY_BITS { + // An array hole leaking through an untyped path reads as `undefined` + // at every other boundary; canonicalize here too, so the tombstone + // marker can never collide with a stored key. + return f64::from_bits(TAG_UNDEFINED); + } if key == 0.0 { 0.0 } else if key.is_nan() && crate::value::JSValue::from_bits(key.to_bits()).is_number() { @@ -1341,6 +1348,7 @@ pub extern "C" fn js_map_alloc(capacity: u32) -> *mut MapHeader { // zeroing, so this MUST be initialised explicitly — an uninitialised // meta edge is a garbage pointer the collector would follow. (*ptr).meta = std::ptr::null_mut(); + (*ptr).used = 0; // Register in map registry for runtime type detection register_map(ptr, entries, cap as usize); @@ -1406,6 +1414,95 @@ pub extern "C" fn js_map_find_key_index(map_boxed: f64, key: f64) -> f64 { #[used] static KEEP_MAP_FIND_KEY_INDEX: extern "C" fn(f64, f64) -> f64 = js_map_find_key_index; +/// Live-extent accessor for iteration (`0..used` are the raw entry indices). +#[inline(always)] +pub(crate) fn map_used_entries(map: *const MapHeader) -> u32 { + unsafe { (*map).used } +} + +/// Raw-indexed entry reads for the iterator objects: bound by `used`, no +/// compaction — the advance loop skips tombstones itself, so iterating a map +/// that is being emptied stays O(live + holes), not O(n) per element. +#[inline(always)] +pub(crate) unsafe fn map_entry_key_raw(map: *const MapHeader, idx: u32) -> f64 { + if idx >= (*map).used { + return f64::from_bits(TAG_UNDEFINED); + } + ptr::read(entries_ptr(map).add(idx as usize * 2)) +} + +#[inline(always)] +pub(crate) unsafe fn map_entry_value_raw(map: *const MapHeader, idx: u32) -> f64 { + if idx >= (*map).used { + return f64::from_bits(TAG_UNDEFINED); + } + ptr::read(entries_ptr(map).add(idx as usize * 2 + 1)) +} + +/// Squeeze the tombstones out: shift live pairs down (insertion order is +/// preserved — only holes are removed), then rebuild the three side indexes +/// from the dense buffer. One overlap-safe pass plus one dirty-span barrier, +/// exactly the cost ONE ordered delete used to pay — but amortized over the +/// deletes that created the holes. +unsafe fn compact_map_entries(map: *mut MapHeader) { + let used = (*map).used as usize; + let entries = entries_ptr_mut(map); + let mut out = 0usize; + for i in 0..used { + let key = ptr::read(entries.add(i * 2)); + if key.to_bits() == MAP_HOLE_KEY_BITS { + continue; + } + if out != i { + ptr::write(entries.add(out * 2), key); + ptr::write(entries.add(out * 2 + 1), ptr::read(entries.add(i * 2 + 1))); + } + out += 1; + } + debug_assert_eq!(out as u32, (*map).size); + (*map).used = out as u32; + if out > 0 { + // GC_STORE_AUDIT(EXTERNAL_BARRIERED): compaction is followed by a dirty-span barrier for every surviving slot. + crate::gc::runtime_write_barrier_external_slot_span( + map as usize, + entries as usize, + out * 2, + ); + } + // Raw entry indices changed; rebuild the side indexes from the dense + // buffer (the same rebuilds every GC rewrite already performs). + if let Some(index) = (*map).numeric_index.as_mut() { + index.clear(); + for i in 0..out { + let bits = ptr::read(entries.add(i * 2)).to_bits(); + if is_safe_numeric_key(bits) { + index.insert(NumericKey(bits), i as u32); + } + } + } + MAP_STRING_INDEX.with(|idx| { + let mut idx = idx.borrow_mut(); + if let Some(slot) = idx.get_mut(&(map as usize)) { + slot.clear(); + for i in 0..out { + let kb = ptr::read(entries.add(i * 2)).to_bits(); + if is_string_like(kb) { + if let Some(h) = string_content_hash(kb) { + slot.entry(h).or_insert_with(Vec::new).push(i as u32); + } + } + } + } + }); + rebuild_map_ptr_index(map); +} + +pub(crate) unsafe fn compact_if_holey(map: *mut MapHeader) { + if (*map).used != (*map).size { + compact_map_entries(map); + } +} + /// The two lookups every hot `Map` does, with nothing else in the frame. /// /// `find_key_index` grew the string-hash, pointer-index and generic-compare @@ -1421,14 +1518,14 @@ static KEEP_MAP_FIND_KEY_INDEX: extern "C" fn(f64, f64) -> f64 = js_map_find_key /// path; a key outside the span goes to the hashed index there. #[inline(always)] unsafe fn find_key_index_hot(map: *const MapHeader, key: f64) -> Option { - let size = (*map).size; + let used = (*map).used; let key_bits = key.to_bits(); if !is_plain_nonzero_number_bits(key_bits) { return None; } - if size <= SIDE_TABLE_THRESHOLD { + if used <= SIDE_TABLE_THRESHOLD { let entries = entries_ptr(map); - for i in 0..size { + for i in 0..used { if ptr::read(entries.add((i as usize) * 2)).to_bits() == key_bits { return Some(i as i32); } @@ -1443,7 +1540,7 @@ unsafe fn find_key_index_hot(map: *const MapHeader, key: f64) -> Option { return None; } let entry = *dense.slots.get_unchecked(offset); - if entry == DENSE_NUMERIC_EMPTY || entry >= size { + if entry == DENSE_NUMERIC_EMPTY || entry >= used { return Some(-1); } Some(entry as i32) @@ -1463,18 +1560,18 @@ pub(crate) unsafe fn find_key_index(map: *const MapHeader, key: f64) -> i32 { /// see the hot lane. #[inline(never)] unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { - let size = (*map).size; + let used = (*map).used; let key_bits = key.to_bits(); // Small maps: linear scan beats side-table dispatch. - if size <= SIDE_TABLE_THRESHOLD { + if used <= SIDE_TABLE_THRESHOLD { let entries = entries_ptr(map); // A plain (untagged, non-NaN), non-zero number is SameValueZero-equal // to an entry key exactly when the bits match: no tagged value can // equal a number, and only `±0` / NaN break bit identity, so those // (and every non-number) keep the general comparison below. if is_plain_nonzero_number_bits(key_bits) { - for i in 0..size { + for i in 0..used { let entry_bits = ptr::read(entries.add((i as usize) * 2)).to_bits(); if entry_bits == key_bits { return i as i32; @@ -1482,8 +1579,11 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { } return -1; } - for i in 0..size { + for i in 0..used { let entry_key = ptr::read(entries.add((i as usize) * 2)); + if entry_key.to_bits() == MAP_HOLE_KEY_BITS { + continue; + } if jsvalue_eq(entry_key, key) { return i as i32; } @@ -1496,7 +1596,7 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { if is_safe_numeric_key(key_bits) { if let Some(index) = (*map).numeric_index.as_ref() { if let Some(i) = index.get(&NumericKey(key_bits)) { - if i < size { + if i < used { return i as i32; } } @@ -1518,7 +1618,7 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { // FNV-1a collisions are rare but possible; validate // each candidate via `jsvalue_eq` (memcmp on bytes). for &cand_idx in bucket { - if cand_idx >= size { + if cand_idx >= used { continue; } let cand_key = ptr::read(entries.add((cand_idx as usize) * 2)); @@ -1547,7 +1647,7 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { let idx = idx.borrow(); if let Some(slot) = idx.get(&(map as usize)) { if let Some(&i) = slot.get(&MapPtrKey(key)) { - if i < size { + if i < used { return Some(i as i32); } } @@ -1562,8 +1662,11 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { // Linear scan for maps with no side-table entry. let entries = entries_ptr(map); - for i in 0..size { + for i in 0..used { let entry_key = ptr::read(entries.add((i as usize) * 2)); + if entry_key.to_bits() == MAP_HOLE_KEY_BITS { + continue; + } if jsvalue_eq(entry_key, key) { return i as i32; } @@ -1573,14 +1676,17 @@ unsafe fn find_key_index_cold(map: *const MapHeader, key: f64) -> i32 { } unsafe fn find_string_key_index(map: *const MapHeader, key: *const StringHeader) -> i32 { - let size = (*map).size; + let used = (*map).used; let key_value = boxed_heap_string_key(key); let key_bits = key_value.to_bits(); - if size <= SIDE_TABLE_THRESHOLD { + if used <= SIDE_TABLE_THRESHOLD { let entries = entries_ptr(map); - for i in 0..size { + for i in 0..used { let entry_key = ptr::read(entries.add((i as usize) * 2)); + if entry_key.to_bits() == MAP_HOLE_KEY_BITS { + continue; + } if jsvalue_eq(entry_key, key_value) { return i as i32; } @@ -1595,7 +1701,7 @@ unsafe fn find_string_key_index(map: *const MapHeader, key: *const StringHeader) if let Some(slot) = idx.get(&(map as usize)) { if let Some(bucket) = slot.get(&h) { for &cand_idx in bucket { - if cand_idx >= size { + if cand_idx >= used { continue; } let cand_key = ptr::read(entries.add((cand_idx as usize) * 2)); @@ -1614,8 +1720,11 @@ unsafe fn find_string_key_index(map: *const MapHeader, key: *const StringHeader) } let entries = entries_ptr(map); - for i in 0..size { + for i in 0..used { let entry_key = ptr::read(entries.add((i as usize) * 2)); + if entry_key.to_bits() == MAP_HOLE_KEY_BITS { + continue; + } if jsvalue_eq(entry_key, key_value) { return i as i32; } @@ -1626,12 +1735,19 @@ unsafe fn find_string_key_index(map: *const MapHeader, key: *const StringHeader) /// Grow the entries array if needed (header stays at same address) unsafe fn ensure_capacity(map: *mut MapHeader) -> bool { - let size = (*map).size; - let capacity = (*map).capacity; - - if size < capacity { + if (*map).used < (*map).capacity { return false; } + // Full by EXTENT. Squeeze tombstones out first — reclaiming holes is + // cheaper than doubling, and it keeps a delete-heavy map from growing on + // dead weight. + if (*map).size < (*map).used { + compact_map_entries(map); + if (*map).used < (*map).capacity { + return false; + } + } + let capacity = (*map).capacity; // Double the capacity let new_capacity = capacity * 2; @@ -1697,18 +1813,19 @@ unsafe fn map_set_string_key_value( let key = key_handle.get_raw_const_ptr::(); let value = value_handle.get_nanbox_f64(); let size = (*map).size; + let used = (*map).used; let entries = entries_ptr_mut(map); - if grew && size > 0 { + if grew && used > 0 { crate::gc::runtime_write_barrier_external_slot_span( map as usize, entries as usize, - size as usize * 2, + used as usize * 2, ); } let key_value = boxed_heap_string_key(key); - let key_slot = entries.add((size as usize) * 2); - let value_slot = entries.add((size as usize) * 2 + 1); + let key_slot = entries.add((used as usize) * 2); + let value_slot = entries.add((used as usize) * 2 + 1); // GC_STORE_AUDIT(EXTERNAL_BARRIERED): map append key/value slots use the shared external-slot helper. crate::gc::runtime_store_external_jsvalue_slot( map as usize, @@ -1722,6 +1839,7 @@ unsafe fn map_set_string_key_value( ); (*map).size = size + 1; + (*map).used = used + 1; if let Some(h) = string_content_hash(key_value.to_bits()) { MAP_STRING_INDEX.with(|idx| { @@ -1729,7 +1847,7 @@ unsafe fn map_set_string_key_value( let slot = idx .entry(map as usize) .or_insert_with(std::collections::HashMap::new); - slot.entry(h).or_insert_with(Vec::new).push(size); + slot.entry(h).or_insert_with(Vec::new).push(used); }); } @@ -1806,17 +1924,18 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let key = key_handle.get_nanbox_f64(); let value = value_handle.get_nanbox_f64(); let size = (*map).size; + let used = (*map).used; let entries = entries_ptr_mut(map); - if grew && size > 0 { + if grew && used > 0 { crate::gc::runtime_write_barrier_external_slot_span( map as usize, entries as usize, - size as usize * 2, + used as usize * 2, ); } - let key_slot = entries.add((size as usize) * 2); - let value_slot = entries.add((size as usize) * 2 + 1); + let key_slot = entries.add((used as usize) * 2); + let value_slot = entries.add((used as usize) * 2 + 1); // GC_STORE_AUDIT(EXTERNAL_BARRIERED): map append key/value slots use the shared external-slot helper. crate::gc::runtime_store_external_jsvalue_slot( map as usize, @@ -1830,6 +1949,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { ); (*map).size = size + 1; + (*map).used = used + 1; // Update the O(1) side-tables: numeric keys by bits, string keys by // content hash, pointer keys (objects/symbols/bigints) in the @@ -1837,7 +1957,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let key_bits = key.to_bits(); if is_safe_numeric_key(key_bits) { if let Some(index) = (*map).numeric_index.as_mut() { - index.insert(NumericKey(key_bits), size); + index.insert(NumericKey(key_bits), used); } } else if is_string_like(key_bits) { // String key: content-hashed index bypasses the gen-GC stale-bits @@ -1849,7 +1969,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let slot = idx .entry(map as usize) .or_insert_with(std::collections::HashMap::new); - slot.entry(h).or_insert_with(Vec::new).push(size); + slot.entry(h).or_insert_with(Vec::new).push(used); }); } } else { @@ -1858,7 +1978,7 @@ fn map_set_resolved(map: *mut MapHeader, key: f64, value: f64) { let slot = idx .entry(map as usize) .or_insert_with(crate::fast_hash::new_ptr_hash_map); - slot.insert(MapPtrKey(key), size); + slot.insert(MapPtrKey(key), used); }); } } @@ -2269,89 +2389,81 @@ unsafe fn delete_entry_at_index(map: *mut MapHeader, idx: i32) -> i32 { } let size = (*map).size; let idx = idx as usize; - if idx >= size as usize { + if idx >= (*map).used as usize { return 0; } let entries = entries_ptr_mut(map); let deleted_key = ptr::read(entries.add(idx * 2)); - // #2831: preserve insertion order. JS Map iteration must keep the - // relative order of surviving entries after a delete (and a - // delete-then-re-add appends at the end). The previous swap-and-pop - // moved the last entry into the hole, reordering iteration. Compact the - // already-owned key/value pairs with one overlap-safe move. This does not - // create a new parent -> child edge: every copied value was already in - // this Map. The span mark preserves the old -> young remembered-set - // contract for the slots' new addresses without paying two full runtime - // stores per entry. - let moved_entries = size as usize - idx - 1; - if moved_entries > 0 { - // GC_STORE_AUDIT(EXTERNAL_BARRIERED): ordered compaction is followed by a dirty-span barrier for every moved slot. - ptr::copy( - entries.add((idx + 1) * 2), - entries.add(idx * 2), - moved_entries * 2, - ); - crate::gc::runtime_write_barrier_external_slot_span( - map as usize, - entries.add(idx * 2) as usize, - moved_entries * 2, - ); - } + // O(1) ordered delete (#2831 preserved): survivors keep their RAW entry + // indices, so nothing shifts, nothing is memmoved, no span barrier over + // the tail, and no side-index offsets need repairing — the three O(n) + // costs that made emptying an N-entry map O(N²) (18.7x node on the ECS + // archetype-migration row). The entry is TOMBSTONED: its key slot takes + // the reserved hole marker (never a legal stored key — `normalize_zero` + // canonicalizes a leaked hole to `undefined`), and its value slot is + // cleared through the barriered store so SATB marking still shades the + // overwritten child. Iteration walks raw indices and skips holes; + // delete-then-re-add still appends at the end. Tombstones are squeezed + // out when they outnumber the live entries, or on growth. + crate::gc::runtime_store_external_jsvalue_slot( + map as usize, + entries.add(idx * 2) as usize, + MAP_HOLE_KEY_BITS, + ); + crate::gc::runtime_store_external_jsvalue_slot( + map as usize, + entries.add(idx * 2 + 1) as usize, + crate::value::TAG_UNDEFINED, + ); (*map).size = size - 1; + forget_map_index_entry(map, deleted_key, idx as u32); - // The old implementation rebuilt all three indexes from the entries - // buffer after every ordered delete. Repair their existing u32 offsets - // in place instead: removing one key and decrementing later offsets is a - // cache-linear pass over index values and does not re-hash surviving keys. - repair_map_indices_after_ordered_delete(map, deleted_key, idx as u32); + let used = (*map).used; + if used >= 16 && (*map).size < used / 2 { + compact_map_entries(map); + } 1 } -unsafe fn repair_map_indices_after_ordered_delete( - map: *mut MapHeader, - deleted_key: f64, - deleted_idx: u32, -) { +/// Forget ONE deleted key from whichever side index holds it. Raw entry +/// indices are stable under tombstoned deletes, so — unlike the pre-tombstone +/// repair — no surviving offset is touched. +unsafe fn forget_map_index_entry(map: *mut MapHeader, deleted_key: f64, deleted_idx: u32) { let map_addr = map as usize; let deleted_bits = deleted_key.to_bits(); - if let Some(index) = (*map).numeric_index.as_mut() { - if is_safe_numeric_key(deleted_bits) { + if is_safe_numeric_key(deleted_bits) { + if let Some(index) = (*map).numeric_index.as_mut() { index.remove(&NumericKey(deleted_bits)); } - index.repair_entry_indices_after_delete(deleted_idx); + return; } - - MAP_STRING_INDEX.with(|indexes| { - let mut indexes = indexes.borrow_mut(); - if let Some(index) = indexes.get_mut(&map_addr) { - for bucket in index.values_mut() { - bucket.retain(|entry_idx| *entry_idx != deleted_idx); - for entry_idx in bucket { - if *entry_idx > deleted_idx { - *entry_idx -= 1; + if is_string_like(deleted_bits) { + if let Some(h) = string_content_hash(deleted_bits) { + MAP_STRING_INDEX.with(|indexes| { + let mut indexes = indexes.borrow_mut(); + if let Some(index) = indexes.get_mut(&map_addr) { + if let Some(bucket) = index.get_mut(&h) { + bucket.retain(|entry_idx| *entry_idx != deleted_idx); + if bucket.is_empty() { + index.remove(&h); + } } } - } - index.retain(|_, bucket| !bucket.is_empty()); + }); } - }); - - MAP_PTR_INDEX.with(|indexes| { - let mut indexes = indexes.borrow_mut(); - if let Some(index) = indexes.get_mut(&map_addr) { - if is_ptr_index_key(deleted_bits) { + return; + } + if is_ptr_index_key(deleted_bits) { + MAP_PTR_INDEX.with(|indexes| { + let mut indexes = indexes.borrow_mut(); + if let Some(index) = indexes.get_mut(&map_addr) { index.remove(&MapPtrKey(deleted_key)); } - for entry_idx in index.values_mut() { - if *entry_idx > deleted_idx { - *entry_idx -= 1; - } - } - } - }); + }); + } } /// Rebuild ONLY the pointer-key index for `map` from its current entries @@ -2362,9 +2474,9 @@ unsafe fn rebuild_map_ptr_index(map: *mut MapHeader) { if map.is_null() { return; } - let size = (*map).size as usize; + let used = (*map).used as usize; let capacity = (*map).capacity as usize; - if size > capacity || size > 16_000_000 || (*map).entries.is_null() { + if used > capacity || used > 16_000_000 || (*map).entries.is_null() { return; } let entries = entries_ptr(map); @@ -2374,7 +2486,7 @@ unsafe fn rebuild_map_ptr_index(map: *mut MapHeader) { .entry(map as usize) .or_insert_with(crate::fast_hash::new_ptr_hash_map); slot.clear(); - for i in 0..size { + for i in 0..used { let entry_key = ptr::read(entries.add(i * 2)); if is_ptr_index_key(entry_key.to_bits()) { slot.insert(MapPtrKey(entry_key), i as u32); @@ -2406,6 +2518,7 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { // map has nothing to reset: the per-entity `adds.clear(); removes.clear()` // of a change set is this case half the time. let size = unsafe { (*map).size }; + let used = unsafe { (*map).used }; if size == 0 { return; } @@ -2415,16 +2528,17 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { // cheaper than the two thread-local resolutions plus two hash probes // that find two empty tables — the per-frame grouping maps of an ECS // are this shape, ten thousand clears a frame. - let side_tables_may_hold_this_map = size > SIDE_TABLE_CLEAR_SCAN_MAX + let side_tables_may_hold_this_map = used > SIDE_TABLE_CLEAR_SCAN_MAX || unsafe { let entries = entries_ptr(map); - (0..size as usize).any(|i| { + (0..used as usize).any(|i| { let key_bits = ptr::read(entries.add(i * 2)).to_bits(); !is_safe_numeric_key(key_bits) }) }; unsafe { (*map).size = 0; + (*map).used = 0; } unsafe { if let Some(index) = (*map).numeric_index.as_mut() { @@ -2460,6 +2574,13 @@ pub extern "C" fn js_map_entry_key_at(map: *const MapHeader, idx: u32) -> f64 { return f64::from_bits(TAG_UNDEFINED); } unsafe { + if (*map).used != (*map).size { + // Tombstones present under a raw-indexed read: the typed for-of + // lane and this fallback iterate raw indices against the live + // size, so squeeze the holes out — after which the codegen lane's + // `used == size` admission holds again and the lane self-heals. + compact_map_entries(map as *mut MapHeader); + } let size = (*map).size; if idx >= size { return f64::from_bits(TAG_UNDEFINED); @@ -2477,6 +2598,13 @@ pub extern "C" fn js_map_entry_value_at(map: *const MapHeader, idx: u32) -> f64 return f64::from_bits(TAG_UNDEFINED); } unsafe { + if (*map).used != (*map).size { + // Tombstones present under a raw-indexed read: the typed for-of + // lane and this fallback iterate raw indices against the live + // size, so squeeze the holes out — after which the codegen lane's + // `used == size` admission holds again and the lane self-heals. + compact_map_entries(map as *mut MapHeader); + } let size = (*map).size; if idx >= size { return f64::from_bits(TAG_UNDEFINED); @@ -2494,6 +2622,7 @@ pub extern "C" fn js_map_entries(map: *const MapHeader) -> *mut crate::array::Ar if map.is_null() { return crate::array::js_array_alloc(0); } + unsafe { compact_if_holey(map as *mut MapHeader) }; let scope = crate::gc::RuntimeHandleScope::new(); let map_handle = scope.root_raw_const_ptr(map); unsafe { @@ -2547,6 +2676,7 @@ pub extern "C" fn js_map_keys(map: *const MapHeader) -> *mut crate::array::Array if map.is_null() { return crate::array::js_array_alloc(0); } + unsafe { compact_if_holey(map as *mut MapHeader) }; let scope = crate::gc::RuntimeHandleScope::new(); let map_handle = scope.root_raw_const_ptr(map); unsafe { @@ -2579,6 +2709,7 @@ pub extern "C" fn js_map_values(map: *const MapHeader) -> *mut crate::array::Arr if map.is_null() { return crate::array::js_array_alloc(0); } + unsafe { compact_if_holey(map as *mut MapHeader) }; let scope = crate::gc::RuntimeHandleScope::new(); let map_handle = scope.root_raw_const_ptr(map); unsafe { @@ -2617,6 +2748,7 @@ fn copy_map_into_new(src: *const MapHeader) -> *mut MapHeader { if src.is_null() { return js_map_alloc(4); } + unsafe { compact_if_holey(src as *const MapHeader as *mut MapHeader) }; let src_handle = scope.root_raw_const_ptr(src); let size = unsafe { let s = src_handle.get_raw_const_ptr::(); @@ -2903,6 +3035,7 @@ fn js_map_foreach_impl( if map.is_null() { return; } + unsafe { compact_if_holey(map as *mut MapHeader) }; let scope = crate::gc::RuntimeHandleScope::new(); let map_handle = scope.root_raw_const_ptr(map); let callback_handle = scope.root_nanbox_f64(callback); @@ -3488,3 +3621,7 @@ mod tests { } } } + +#[cfg(test)] +#[path = "map_tombstone_tests.rs"] +mod map_tombstone_tests; diff --git a/crates/perry-runtime/src/map_tombstone_tests.rs b/crates/perry-runtime/src/map_tombstone_tests.rs new file mode 100644 index 0000000000..9c8de86d3d --- /dev/null +++ b/crates/perry-runtime/src/map_tombstone_tests.rs @@ -0,0 +1,163 @@ +//! Tombstoned ordered deletes (#2831 semantics, O(1) cost). +//! +//! A delete no longer shifts survivors: the entry is holed in place, raw +//! entry indices stay stable, and compaction runs only when tombstones +//! outnumber live entries or the array must grow. These tests pin the +//! observable contract — insertion order, delete-then-re-add, lookup +//! correctness across holes, iterator hole-skips, and the self-healing +//! compaction under raw-indexed access. + +use super::*; + +#[test] +fn ordered_delete_preserves_order_and_lookup_across_holes() { + let map = js_map_alloc(8); + for k in [10.0f64, 20.0, 30.0, 40.0, 50.0] { + js_map_set(map, k, k * 10.0); + } + + assert_eq!(js_map_delete(map, 30.0), 1, "middle"); + assert_eq!(js_map_delete(map, 10.0), 1, "front"); + assert_eq!(js_map_delete(map, 50.0), 1, "back"); + unsafe { + assert_eq!((*map).size, 2); + assert!((*map).used >= 2, "holes may remain before compaction"); + } + + // Survivors resolve, deleted keys do not — through every lookup lane. + assert_eq!(js_map_get(map, 20.0), 200.0); + assert_eq!(js_map_get(map, 40.0), 400.0); + for gone in [10.0f64, 30.0, 50.0] { + assert_eq!(js_map_has(map, gone), 0, "{gone} was deleted"); + } + + // Delete-then-re-add appends at the end (#2831): iteration order is + // 20, 40, 30 after re-adding 30. + js_map_set(map, 30.0, 999.0); + unsafe { compact_if_holey(map) }; + unsafe { + let entries = entries_ptr(map); + assert_eq!(ptr::read(entries), 20.0); + assert_eq!(ptr::read(entries.add(2)), 40.0); + assert_eq!(ptr::read(entries.add(4)), 30.0); + } + assert_eq!(js_map_get(map, 30.0), 999.0); +} + +#[test] +fn emptying_a_map_stays_consistent_and_compacts() { + let map = js_map_alloc(16); + for i in 0..64 { + js_map_set(map, i as f64, (i * 2) as f64); + } + for i in 0..64 { + assert_eq!(js_map_delete(map, i as f64), 1, "key {i} deletes once"); + assert_eq!(js_map_delete(map, i as f64), 0, "and only once"); + } + unsafe { + assert_eq!((*map).size, 0); + assert!( + (*map).used < 64, + "the tombstone threshold must have compacted at least once (used = {})", + (*map).used + ); + } + for i in 0..64 { + assert_eq!(js_map_has(map, i as f64), 0); + } + js_map_set(map, 7.0, 70.0); + assert_eq!( + js_map_get(map, 7.0), + 70.0, + "the emptied map still accepts inserts" + ); +} + +#[test] +fn raw_indexed_access_self_heals_by_compacting() { + let map = js_map_alloc(8); + for k in [1.0f64, 2.0, 3.0] { + js_map_set(map, k, k); + } + assert_eq!(js_map_delete(map, 2.0), 1); + unsafe { + assert_ne!((*map).used, (*map).size, "a hole is present"); + } + // The raw-indexed extern compacts first, so entry 1 is the THIRD key — + // exactly what the typed for-of lane's fallback needs for raw == live. + assert_eq!(js_map_entry_key_at(map, 1), 3.0); + unsafe { + assert_eq!((*map).used, (*map).size, "access healed the layout"); + } +} + +#[test] +fn iterator_skips_holes_and_survives_deleting_the_last_returned_key() { + unsafe { + let iter = crate::value::js_nanbox_pointer( + crate::collection_iter_object::js_map_keys_iter_obj(map_with(&[1.0, 2.0, 3.0, 4.0])), + ); + let key = |r: f64| { + f64::from_bits( + crate::object::js_object_get_field( + crate::value::js_nanbox_get_pointer(r) as *mut crate::object::ObjectHeader, + 0, + ) + .bits(), + ) + }; + let done = |r: f64| { + crate::value::JSValue::from_bits( + crate::object::js_object_get_field( + crate::value::js_nanbox_get_pointer(r) as *mut crate::object::ObjectHeader, + 1, + ) + .bits(), + ) + .as_bool() + }; + let next = |iter: f64| crate::collection_iter_object::js_for_of_next(iter); + + let backing = iter_backing(iter); + let r = next(iter); + assert_eq!(key(r), 1.0); + // Delete the key we just returned, and one ahead of the cursor. + js_map_delete(backing, 1.0); + js_map_delete(backing, 3.0); + let r = next(iter); + assert_eq!(key(r), 2.0, "hole at the cursor's resume point is skipped"); + let r = next(iter); + assert_eq!(key(r), 4.0, "hole ahead of the cursor is skipped"); + assert!(done(next(iter)), "then exhausted"); + } +} + +#[test] +fn clear_resets_the_extent() { + let map = js_map_alloc(4); + js_map_set(map, 1.0, 1.0); + js_map_set(map, 2.0, 2.0); + js_map_delete(map, 1.0); + js_map_clear(map); + unsafe { + assert_eq!((*map).size, 0); + assert_eq!((*map).used, 0); + } + js_map_set(map, 9.0, 90.0); + assert_eq!(js_map_get(map, 9.0), 90.0); +} + +fn map_with(keys: &[f64]) -> *mut MapHeader { + let map = js_map_alloc(8); + for &k in keys { + js_map_set(map, k, k * 100.0); + } + map +} + +unsafe fn iter_backing(iter: f64) -> *mut MapHeader { + let obj = crate::value::js_nanbox_get_pointer(iter) as *mut crate::object::ObjectHeader; + crate::value::js_nanbox_get_pointer(f64::from_bits( + crate::object::js_object_get_field(obj, 0).bits(), + )) as *mut MapHeader +} diff --git a/crates/perry-runtime/src/object/iterator_prototypes.rs b/crates/perry-runtime/src/object/iterator_prototypes.rs index 6c77ae53f4..9c955394c3 100644 --- a/crates/perry-runtime/src/object/iterator_prototypes.rs +++ b/crates/perry-runtime/src/object/iterator_prototypes.rs @@ -264,11 +264,6 @@ fn build_family_proto( } /// Lazily build the prototypes (idempotent). Cheap after the first call. -/// Whether any iterator-prototype tower has been materialized on this thread. -#[cfg(test)] -pub(crate) fn iterator_prototypes_materialized() -> bool { - ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) != 0 -} pub(crate) fn ensure_iterator_prototypes() { if ITERATOR_PROTOTYPE_PTR.load(Ordering::Acquire) == 0 { From f1ba107c58ebc2f4d8f19e5d5d1445ebe634ab80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 10:39:46 +0200 Subject: [PATCH 3/3] fix(runtime,codegen): audit the compaction stores, bind the MapHeader offset `gc_store_site_inventory.py` (a `lint` gate) rejected compaction's two raw `ptr::write`s. The justification was already correct -- the dirty-span barrier covers every surviving slot -- but its marker sat 8 lines below the writes, outside the gate's window. Moved a marker adjacent, noting the overlap argument too: `out <= i` always, so a live pair only moves DOWN within one buffer. The typed for-of lane loads `MapHeader::used` at a hardcoded offset 32, and perry-codegen does not depend on perry-runtime, so nothing bound the literal to the struct. The runtime's `offset_of!` assertion catches a REORDER, but the natural fix for it is to update its expected value -- which leaves the codegen string stale and still compiling, and the lane would then load the wrong header word and mis-admit holey maps. Named the literal and tied it to the runtime source, exactly as `hot_tls.rs` does for the same crate split. Sabotage-checked: moving the runtime offset now fails the codegen test. Also adds the missing changelog.d fragment. --- changelog.d/9020-map-tombstone-delete.md | 30 ++++++++++++++ crates/perry-codegen/src/expr/arrays_finds.rs | 39 ++++++++++++++++++- crates/perry-runtime/src/map.rs | 4 ++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9020-map-tombstone-delete.md diff --git a/changelog.d/9020-map-tombstone-delete.md b/changelog.d/9020-map-tombstone-delete.md new file mode 100644 index 0000000000..fbcf3e4f7f --- /dev/null +++ b/changelog.d/9020-map-tombstone-delete.md @@ -0,0 +1,30 @@ +Ordered `Map` deletes are O(1) instead of O(N). + +Emptying an N-entry Map was O(N²) three ways at once: every delete memmoved the +trailing entries down, span-barriered every moved slot, and walked all three +side indexes decrementing every offset above the hole. Per-delete cost doubled +with N (0.96 µs at N=2k to 8.2 µs at N=16k) where node is flat at ~0.03 µs — +263× at N=16k. On the ECS archetype-migration row, `delete_entry_at_index` and +its memmove were ~14% of the frame. + +A delete now tombstones in place: the key slot takes a reserved hole marker, the +value slot is cleared through the barriered store so SATB marking still shades +the overwritten child, and the live `size` drops while the array extent (`used`, +a new `MapHeader` field) stays put. Raw entry indices are therefore stable — no +shifting, no span barrier over the tail, and the side indexes only forget the +deleted key, so the offset-repair walkers are deleted outright. + +The hole marker can never collide with a stored key: `normalize_zero` +canonicalizes it to `undefined` on the resolved-key path, the string-key path +writes a STRING_TAG-boxed pointer that cannot equal it, and compaction only +moves keys that were already normalized on insert. + +Insertion order is unchanged: iteration walks raw indices and skips holes, and +delete-then-re-add still appends. Compaction runs when tombstones outnumber live +entries, before growing, or when a raw-indexed accessor observes holes — after +which the typed `for…of` lane's `used == size` admission holds again, so the +codegen lane self-heals rather than misreading holes. + +The GC contract bounds the Map slot descriptor's range by `used`, with +`size ≤ used ≤ capacity` as the corruption guard; holes are non-pointer markers +the tag-filtered scan skips. diff --git a/crates/perry-codegen/src/expr/arrays_finds.rs b/crates/perry-codegen/src/expr/arrays_finds.rs index ab4601a8ea..47fc0eb4e5 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -91,6 +91,18 @@ use super::index_get::numeric_index_has_integer_array_index_proof; /// other shape — a subclass instance, a plain object, an out-of-range or /// negative index, an unpublished handle — takes the runtime helper exactly /// as before, so the two paths are equivalent by construction. +/// Byte offset of `MapHeader::used`, which the tombstoned-delete lane loads to +/// check `used == size` (no holes) before admitting a raw entry read. +/// +/// `perry-codegen` does not depend on `perry-runtime`, so nothing binds this +/// literal to the struct it describes. The runtime pins the offset with an +/// `offset_of!` assertion, which catches a field REORDER — but the natural fix +/// for that assertion is to update its expected value, which leaves this string +/// stale and still compiling, and generated code would then load the wrong word +/// of the header. `map_header_used_offset_is_what_codegen_assumes` ties the two +/// together by reading the runtime source, exactly as `hot_tls.rs` does. +const MAP_HEADER_USED_OFFSET: &str = "32"; + fn lower_map_entry_at_inline( ctx: &mut FnCtx<'_>, m_box: &str, @@ -150,7 +162,7 @@ fn lower_map_entry_at_inline( // dense-correct with no holes present, so a holey map falls back to // the runtime helper — which compacts, after which this admission // holds again (the lane self-heals). - let used_addr = blk.add(I64, &m_handle, "32"); + let used_addr = blk.add(I64, &m_handle, MAP_HEADER_USED_OFFSET); let used_ptr = blk.inttoptr(I64, &used_addr); let used = blk.load(I32, &used_ptr); let dense = blk.icmp_eq(I32, &used, &size); @@ -1431,3 +1443,28 @@ pub(crate) fn lower( _ => unreachable!("expr/mod.rs dispatched a variant not handled by this submodule"), } } + +#[cfg(test)] +mod map_header_layout_tests { + /// The offset the runtime pins with `offset_of!(MapHeader, used) == N`. + fn runtime_map_used_offset() -> usize { + let src = include_str!("../../../perry-runtime/src/map.rs"); + let needle = "offset_of!(MapHeader, used) == "; + let rest = src + .split_once(needle) + .expect("MapHeader::used offset assertion not found in map.rs - was it renamed?") + .1; + let digits: String = rest.chars().take_while(char::is_ascii_digit).collect(); + digits.parse().expect("offset is a decimal literal") + } + + #[test] + fn map_header_used_offset_is_what_codegen_assumes() { + assert_eq!( + super::MAP_HEADER_USED_OFFSET.parse::().unwrap(), + runtime_map_used_offset(), + "codegen emits a stale MapHeader::used offset; the tombstone lane \ + would read the wrong header word and mis-admit holey maps", + ); + } +} diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 704c0b9cd0..b060ce40ce 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1454,6 +1454,10 @@ unsafe fn compact_map_entries(map: *mut MapHeader) { continue; } if out != i { + // GC_STORE_AUDIT(EXTERNAL_BARRIERED): the dirty-span barrier below + // covers every surviving slot this pass writes. Overlap-safe by + // construction -- `out <= i` always, so a live pair only ever moves + // DOWN within the one buffer, never onto an unread source. ptr::write(entries.add(out * 2), key); ptr::write(entries.add(out * 2 + 1), ptr::read(entries.add(i * 2 + 1))); }