diff --git a/changelog.d/9513-map-set-iteration-compaction.md b/changelog.d/9513-map-set-iteration-compaction.md new file mode 100644 index 0000000000..5020ce5f83 --- /dev/null +++ b/changelog.d/9513-map-set-iteration-compaction.md @@ -0,0 +1,62 @@ +**Map/Set: mutation during `for…of` is O(1) per step again, and never skips an entry.** + +`#9020` made ordered `delete` O(1) by tombstoning entries in place, and moved +the squeeze that used to happen per delete into the raw-index *readers* +(`js_map_entry_key_at` / `js_set_value_at`) as a "self-heal": the first raw +read that saw a hole compacted the whole collection. The `for…of` fast path +reads raw slots on every step, so a loop that deletes while iterating paid one +full compaction per delete — 50,000 entries with 12,500 deletes inside the walk +took 13–32 s against Node's 0.07 s (190–460×), quadratic in the collection +size; the identical deletes with no iterator open cost 0.5 s. + +Worse, every raw-index cursor (the fast path, the iterator objects) recovered +from a squeeze by re-finding the last returned key and, when that key was +itself deleted, reading `cursor-1` — which assumed exactly ONE hole had been +squeezed. Deleting several already-visited entries plus the current one in a +single loop body skipped entries, and enough holes ended the loop early +(40 entries, 21 deleted at the 21st: Perry visited 21, Node 40). + +Fixed the way V8 transitions its ordered-hash-table iterators: every squeeze +(`compact_*`, and `clear` while a walk may be open) records which raw indices +it removed, in a per-collection log, and bumps a `compaction_epoch` in the +header. A cursor carries the epoch it last synchronised with; one runtime call +per step (`js_map_cursor_next` / `js_set_cursor_next`) rebases it — down by +exactly the removed count below it, in order, through every record since — +then steps over tombstones and returns the next live raw index. This is exact +by the walk's own invariant (the yielded entries are precisely the live ones +below the cursor), needs no key lookup, no "iteration active" registration +that a `break`, `return` or abandoned generator could leak, and the walkers' +reads no longer compact at all. The codegen inline entry read is bounded by +the raw extent instead of requiring a dense buffer. + +Two reader contracts, kept apart. `js_map_entry_key_at` / `js_map_entry_value_at` +/ `js_set_value_at` remain the LIVE-index accessors #9504 made the array-like +`map[i]` / `set[i]` read, `console.table` and collection equality go through: +they squeeze tombstones first so raw index == live index, never hand out a +hole, and now record that squeeze — so a `for…of` cursor open on the same +collection rebases exactly instead of skipping. The walkers use new RAW twins +(`js_map_entry_key_raw_at`, `js_map_entry_value_raw_at`, `js_set_value_raw_at`), +bounded by the raw extent, which never compact; only the cursor ever reads +them, and it only yields live raw indices. The iterator objects use the same +rebase (their field 3 now holds the epoch). History is retained under a budget +of removed raw indices — `max(4096, capacity)` per collection — never a record +count, because a delete+re-add pair on a collection at full capacity squeezes +one hole per pair on the grow path and a single loop body can force dozens of +those. A `clear()` record supersedes everything before it and truncates it. +Exceeding the budget therefore takes one loop body deleting more than the +collection's whole capacity between two of its own reads; a cursor trimmed out +of its history is stepped over holes without the lost rebase, and the bound is +pinned by a forty-squeezes-in-one-body test for Map and Set. + +Also correct now: `clear()` inside a walk restarts the cursor at 0, so entries +added afterwards are visited, as the spec's in-place emptying of `[[MapData]]` +requires. + +Tests: runtime unit tests for both reader contracts (raw twins never compact; +live-index accessors squeeze, record, and a cursor past the hole rebases), the +exact multi-hole rebase, successive squeezes + `clear`, forty squeezes in one +body at full capacity, `clear` truncating the history, and address reuse (Map +and Set); #9504's `a_tombstoned_collection_never_hands_a_hole_to_an_indexed_read` +stays green; `test_gap_map_set_multi_delete_during_iteration.ts` covers the fast path, +the iterator objects, re-add, `clear`, and a 50k-entry churn, all +Node-differential. diff --git a/crates/perry-codegen/src/expr/arrays_finds.rs b/crates/perry-codegen/src/expr/arrays_finds.rs index 5cf143711e..c4ea2bfbbd 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -91,8 +91,8 @@ 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. +/// Byte offset of `MapHeader::used`, the raw extent the inline entry read is +/// bounded by (the for-of cursor only ever yields a live raw index below it). /// /// `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 @@ -156,20 +156,19 @@ fn lower_map_entry_at_inline( let gc_flags = blk.load(I8, &flags_ptr); let forwarded = blk.and(I8, &gc_flags, GC_FLAG_FORWARDED); 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). + // The index is a LIVE raw index by construction: it comes from + // `js_map_cursor_next`, and the for-of desugars in perry-hir are the + // only producers of these reads. So the read is bounded by the raw + // extent `used`, not the live `size`, and holes need no admission + // check — the cursor already stepped over them. (This lane used to + // require `used == size` and fall back to a runtime helper that + // compacted the whole map on every holey read.) 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); - let in_range = blk.icmp_ult(I32, &i_i32, &size); + let in_range = blk.icmp_ult(I32, &i_i32, &used); let a = blk.and(I1, &is_map, &live); - let b = blk.and(I1, &a, &dense); - let admitted = blk.and(I1, &b, &in_range); + let admitted = blk.and(I1, &a, &in_range); blk.cond_br(&admitted, &fast_label, &slow_label); } ctx.current_block = fast_idx; @@ -595,8 +594,14 @@ pub(crate) fn lower( // calling `js_map_entries` (which materializes N+1 small Arrays). Expr::MapEntryKeyAt { map, idx } | Expr::MapEntryValueAt { map, idx } => { let (runtime_fn, value_slot) = match expr { - Expr::MapEntryKeyAt { .. } => ("js_map_entry_key_at", false), - Expr::MapEntryValueAt { .. } => ("js_map_entry_value_at", true), + // The RAW twins: these nodes are produced only by the for-of + // desugars, whose cursor yields live raw indices, so the + // fallback must read raw slots bounded by `used` and never + // compact. (`js_map_entry_key_at` without `_raw` is the + // live-index accessor the array-like `map[i]` read uses, + // which compacts — #9504.) + Expr::MapEntryKeyAt { .. } => ("js_map_entry_key_raw_at", false), + Expr::MapEntryValueAt { .. } => ("js_map_entry_value_raw_at", true), _ => unreachable!(), }; rooting::with_operands_rooted(ctx, &[map, idx], |ctx, vals| { @@ -619,7 +624,7 @@ pub(crate) fn lower( let i_i32 = blk.fptosi(DOUBLE, &i_dbl, I32); Ok(blk.call( DOUBLE, - "js_set_value_at", + "js_set_value_raw_at", &[(I64, &s_handle), (I32, &i_i32)], )) }) diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 48d75e0462..b101f9cbf9 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1235,10 +1235,22 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // would do. (Map ptr, entry idx) → key / value. module.declare_function("js_map_entry_key_at", DOUBLE, &[I64, I32]); module.declare_function("js_map_entry_value_at", DOUBLE, &[I64, I32]); + // RAW twins for the for-of walkers: bounded by the raw extent, never + // compact (the un-suffixed accessors above are live-index and compact). + module.declare_function("js_map_entry_key_raw_at", DOUBLE, &[I64, I32]); + module.declare_function("js_map_entry_value_raw_at", DOUBLE, &[I64, I32]); + module.declare_function("js_set_value_raw_at", DOUBLE, &[I64, I32]); // #6075: current index of a key/value (or -1) for the delete-safe for-of // fast path. Takes the NaN-boxed collection + key; strips internally. module.declare_function("js_map_find_key_index", DOUBLE, &[DOUBLE, DOUBLE]); module.declare_function("js_set_find_value_index", DOUBLE, &[DOUBLE, DOUBLE]); + // The delete-safe for-of cursor: (NaN-boxed collection, cursor, epoch) → + // next live raw index or -1, and the collection's compaction epoch the + // loop stores after each step. See perry-hir `map_set_delete_safe_for_of`. + module.declare_function("js_map_cursor_next", DOUBLE, &[DOUBLE, DOUBLE, DOUBLE]); + module.declare_function("js_map_compaction_epoch", DOUBLE, &[DOUBLE]); + module.declare_function("js_set_cursor_next", DOUBLE, &[DOUBLE, DOUBLE, DOUBLE]); + module.declare_function("js_set_compaction_epoch", DOUBLE, &[DOUBLE]); // Map/Set forEach: (collection_ptr, callback_nanboxed_f64, thisArg_f64) -> void (#2830) module.declare_function("js_map_foreach", VOID, &[I64, DOUBLE, DOUBLE]); module.declare_function("js_set_foreach", VOID, &[I64, DOUBLE, DOUBLE]); diff --git a/crates/perry-hir/src/lower/for_head.rs b/crates/perry-hir/src/lower/for_head.rs index eb835c66cc..b005be1cdd 100644 --- a/crates/perry-hir/src/lower/for_head.rs +++ b/crates/perry-hir/src/lower/for_head.rs @@ -518,67 +518,50 @@ pub(crate) fn rewrite_collection_view_for_of( }) } -/// Build the delete-safe control for the Map/Set `for-of` fast path (#6075). +/// Build the cursor control for the Map/Set `for-of` fast path. /// -/// The fast path iterates the backing entries array by index (`arr_id` holds the -/// collection, `idx_id` the cursor). But `delete` compacts that array (entries -/// after the hole shift down), so deleting an entry at index ≤ the cursor moves -/// an unvisited entry below the cursor and skips it. This re-derives the read -/// index each iteration from the last-returned key, so a shift can't skip. +/// The fast path walks the backing entries buffer by RAW index (`arr_id` +/// holds the collection, `idx_id` the cursor). Two things can happen to the +/// raw layout while the body runs: +/// +/// * a `delete` tombstones an entry in place (#9020) — raw indices are +/// stable, the cursor only has to step over the hole; +/// * a squeeze (`delete` past the tombstone threshold, `set` at capacity, +/// `clear`) moves survivors to lower raw indices — the cursor must move +/// down by exactly the number of removed slots below it. +/// +/// Both are answered by ONE runtime call per step, `js_map_cursor_next` / +/// `js_set_cursor_next`: it rebases the cursor through the collection's +/// compaction log (every squeeze since the epoch the loop last recorded) and +/// returns the next live raw index, or `-1` when the extent is exhausted. +/// The loop keeps the epoch it last synchronised with in a state temp. Cost +/// per step is O(holes stepped over); nothing here is O(n), and nothing here +/// compacts — the reader-side "self-heal" compaction that preceded this made +/// a delete-during-`for…of` loop O(n) per delete and, when more than one +/// hole was squeezed at once, its `cursor-1` recovery skipped entries. /// /// Returns `(init_lets, condition, body_prefix)`: /// - `init_lets` — declare the state temps; push BEFORE the `for`. -/// - `condition` — replaces `idx < size`. Overwrites `idx_id` with the corrected -/// read index (plain cursor while the previously-read key is still at -/// `cursor-1`; else locate the last key — `+1` after it, or into its vacated -/// slot if it was itself deleted) and yields whether that index is in range. -/// - `body_prefix` — prepend to the loop body (runs after the in-range check): -/// records the visited key for the next iteration's in-place check. -/// -/// `find` is O(1) for numeric/string keys and only called when a shift is -/// detected, so normal / append-only iteration keeps the plain cursor path. +/// - `condition` — replaces `idx < size`: rebases + advances `idx_id` and +/// yields whether an entry is available. +/// - `body_prefix` — empty; kept so both desugars keep their shape. pub(crate) fn map_set_delete_safe_for_of( ctx: &mut LoweringContext, arr_id: LocalId, idx_id: LocalId, is_set: bool, ) -> (Vec, Expr, Vec) { - let lk_id = ctx.fresh_local(); // last-returned key - let sz_id = ctx.fresh_local(); // current size (spilled once per iteration) - let fk_id = ctx.fresh_local(); // find() result - - // `.size` on a Map/Set-typed receiver is codegen-recognized and lowered to - // js_map_size / js_set_size (the raw `MapSize`/`SetSize` nodes are not - // codegen expressions), matching the original loop bound. - let size_of = |a: Expr| -> Expr { - Expr::PropertyGet { - byte_offset: 0, - object: Box::new(a), - property: "size".to_string(), - } - }; - let key_at = |a: Expr, i: Expr| -> Expr { - if is_set { - Expr::SetValueAt { - set: Box::new(a), - idx: Box::new(i), - } - } else { - Expr::MapEntryKeyAt { - map: Box::new(a), - idx: Box::new(i), - } - } - }; - let find_fn = if is_set { - "js_set_find_value_index" + let ep_id = ctx.fresh_local(); // compaction epoch last synchronised with + let nx_id = ctx.fresh_local(); // js_*_cursor_next result + let (next_fn, epoch_fn) = if is_set { + ("js_set_cursor_next", "js_set_compaction_epoch") } else { - "js_map_find_key_index" + ("js_map_cursor_next", "js_map_compaction_epoch") }; - let find_call = |args: Vec| -> Expr { + let extern_call = |name: &str, args: Vec| -> Expr { Expr::Call { callee: Box::new(Expr::ExternFuncRef { - name: find_fn.to_string(), + name: name.to_string(), param_types: Vec::new(), return_type: Type::Number, }), @@ -587,110 +570,40 @@ pub(crate) fn map_set_delete_safe_for_of( byte_offset: 0, } }; - let cmp = |op: CompareOp, l: Expr, r: Expr| -> Expr { - Expr::Compare { - op, - left: Box::new(l), - right: Box::new(r), - } - }; - - // cursor-1 (index of the entry read on the previous iteration). - let prev_idx = |i: Expr| Expr::Binary { - op: BinaryOp::Sub, - left: Box::new(i), - right: Box::new(Expr::Number(1.0)), - }; - - // read_idx = cursor == 0 // not started - // ? cursor - // : key_at(coll, cursor-1) === last_key // last entry still in place - // ? cursor // no shift at/below cursor - // : (j = find(coll, last_key)) >= 0 ? j + 1 : cursor - 1 - // - // Comparing the entry now at cursor-1 to the last-returned key detects ANY - // delete that compacted an entry at/below the cursor — including a delete - // balanced by an add in the same turn (which leaves `size` unchanged), which - // a size-only gate would miss. Map/Set keys are unique under SameValueZero, - // so `===` here is exact except for a NaN key (which just forces the `find` - // path — still correct). - let rederive = Expr::Conditional { - condition: Box::new(cmp( - CompareOp::Eq, - Expr::LocalGet(idx_id), - Expr::Number(0.0), - )), - then_expr: Box::new(Expr::LocalGet(idx_id)), - else_expr: Box::new(Expr::Conditional { - condition: Box::new(cmp( - CompareOp::Eq, - key_at(Expr::LocalGet(arr_id), prev_idx(Expr::LocalGet(idx_id))), - Expr::LocalGet(lk_id), - )), - then_expr: Box::new(Expr::LocalGet(idx_id)), - else_expr: Box::new(Expr::Sequence(vec![ - Expr::LocalSet( - fk_id, - Box::new(find_call(vec![ - Expr::LocalGet(arr_id), - Expr::LocalGet(lk_id), - ])), - ), - Expr::Conditional { - // A delete only shifts entries down: a merely-shifted last key - // is now below the cursor (`0 <= j < cursor`) → resume after - // it. Deleted (`j < 0`) or deleted-then-re-added at the end - // (`j >= cursor`) → read the entry now in its old slot - // (`cursor - 1`). - condition: Box::new(cmp( - CompareOp::Ge, - Expr::LocalGet(fk_id), - Expr::Number(0.0), - )), - then_expr: Box::new(Expr::Conditional { - condition: Box::new(cmp( - CompareOp::Lt, - Expr::LocalGet(fk_id), - Expr::LocalGet(idx_id), - )), - then_expr: Box::new(Expr::Binary { - op: BinaryOp::Add, - left: Box::new(Expr::LocalGet(fk_id)), - right: Box::new(Expr::Number(1.0)), - }), - else_expr: Box::new(prev_idx(Expr::LocalGet(idx_id))), - }), - else_expr: Box::new(prev_idx(Expr::LocalGet(idx_id))), - }, - ])), - }), - }; - + // nx = cursor_next(coll, idx, ep); ep = epoch(coll); idx = nx; nx >= 0 let condition = Expr::Sequence(vec![ - Expr::LocalSet(sz_id, Box::new(size_of(Expr::LocalGet(arr_id)))), - Expr::LocalSet(idx_id, Box::new(rederive)), - cmp(CompareOp::Lt, Expr::LocalGet(idx_id), Expr::LocalGet(sz_id)), + Expr::LocalSet( + nx_id, + Box::new(extern_call( + next_fn, + vec![ + Expr::LocalGet(arr_id), + Expr::LocalGet(idx_id), + Expr::LocalGet(ep_id), + ], + )), + ), + Expr::LocalSet( + ep_id, + Box::new(extern_call(epoch_fn, vec![Expr::LocalGet(arr_id)])), + ), + Expr::LocalSet(idx_id, Box::new(Expr::LocalGet(nx_id))), + Expr::Compare { + op: CompareOp::Ge, + left: Box::new(Expr::LocalGet(nx_id)), + right: Box::new(Expr::Number(0.0)), + }, ]); - - // Record the key just read so the next iteration can check it is still in - // place at cursor-1. - let body_prefix = vec![Stmt::Expr(Expr::LocalSet( - lk_id, - Box::new(key_at(Expr::LocalGet(arr_id), Expr::LocalGet(idx_id))), - ))]; - - let mk_let = |id: LocalId, tag: &str, ty: Type, init: Expr| Stmt::Let { + let mk_let = |id: LocalId, tag: &str, init: Expr| Stmt::Let { id, name: format!("__miter_{}_{}", tag, id), - ty, + ty: Type::Number, mutable: true, init: Some(init), }; let init_lets = vec![ - mk_let(lk_id, "lk", Type::Any, Expr::Undefined), - mk_let(sz_id, "sz", Type::Number, Expr::Number(0.0)), - mk_let(fk_id, "fk", Type::Number, Expr::Number(0.0)), + mk_let(ep_id, "ep", Expr::Number(0.0)), + mk_let(nx_id, "nx", Expr::Number(0.0)), ]; - - (init_lets, condition, body_prefix) + (init_lets, condition, Vec::new()) } diff --git a/crates/perry-runtime/src/collection_iter_object.rs b/crates/perry-runtime/src/collection_iter_object.rs index df9f52c848..4a91eef8ec 100644 --- a/crates/perry-runtime/src/collection_iter_object.rs +++ b/crates/perry-runtime/src/collection_iter_object.rs @@ -85,13 +85,14 @@ unsafe fn alloc_iterator(class_id: u32, coll_nanboxed: f64, kind: i32) -> f64 { obj_h.with_mut_ptr::(|obj| { js_object_set_field(obj, 2, JSValue::number(kind as f64)) }); - // Field 3: collection size observed at the last `next()`. `-1` sentinel means - // "not started" (no entry returned yet). Used to detect a mid-iteration - // delete (which compacts the entries array, shifting live entries below the - // cursor) so the cursor can be re-derived from the last key (#6075). - obj_h.with_mut_ptr::(|obj| js_object_set_field(obj, 3, JSValue::number(-1.0))); - // 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. + // Field 3: the backing collection's compaction epoch this iterator last + // synchronised with (starts at 0 — a cursor of 0 rebases to 0 through any + // history). `next()` rebases the cursor through every squeeze recorded + // since, so a compaction below the cursor can never skip an entry (#6075, + // #6165 — and the multi-hole squeeze the key-based re-derive got wrong). + obj_h.with_mut_ptr::(|obj| js_object_set_field(obj, 3, JSValue::number(0.0))); + // Field 4: unused since the epoch-based rebase (was the last-returned + // key); kept so the object layout and the cached-result field 5 stay put. obj_h.with_mut_ptr::(|obj| 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 @@ -218,37 +219,6 @@ unsafe fn make_pair_array(a: f64, b: f64) -> f64 { pair.with_mut_ptr::(|pair| js_nanbox_pointer(pair as i64)) } -/// Compute the entries-array index to read next, self-correcting for a -/// mid-iteration delete. `cursor` = index just past the last-returned entry; -/// `last_key_in_place` = the previously-read key is still at `cursor-1`; -/// `find_last` locates the last-returned key's current index (or `< 0` if it was -/// deleted). -/// -/// Deleting an entry compacts the backing array (entries after the hole shift -/// down one slot, #2831), so a delete at index ≤ cursor would move an unvisited -/// entry below the cursor and skip it. If the last-returned key is still sitting -/// at `cursor-1`, no such shift happened and the plain cursor is correct — so -/// normal / append-only iteration keeps the fast path and object-keyed maps pay -/// no lookup. Otherwise re-derive from the last key: locate it (`+1` after it), -/// or, if it was itself deleted, read the entry that shifted into its slot -/// (`cursor-1`). Comparing the key (rather than the size) also catches a delete -/// balanced by an add in the same turn. (#6075 / #6165) -fn next_read_index(cursor: u32, last_key_in_place: bool, find_last: impl FnOnce() -> i32) -> u32 { - if cursor == 0 || last_key_in_place { - return cursor; - } - let j = find_last(); - // A delete only shifts entries DOWN, so a last key that merely shifted is now - // below the cursor (`j < cursor`) → resume after it. Otherwise it was deleted - // (`j < 0`) or deleted-then-re-added at the end (`j >= cursor`) — either way - // the entry that shifted into its old slot sits at `cursor-1`. - if j >= 0 && (j as u32) < cursor { - (j as u32) + 1 - } else { - cursor.saturating_sub(1) - } -} - /// 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, true) @@ -293,38 +263,31 @@ unsafe fn dispatch_map_iterator_method_emit( 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); + // Field 3: the backing Map's compaction epoch this iterator last + // synchronised with. `map_cursor_next_raw` rebases the cursor + // through every squeeze since (exactly — by removed-slot count, + // not by re-finding a key that may itself be gone), then steps + // over tombstones. + let epoch = f64::from_bits(js_object_get_field(iter_obj(), 3).bits()); + let epoch = if epoch > 0.0 { epoch as u32 } else { 0 }; 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::map_entry_key_raw(map(), cursor - 1); - crate::value::js_jsvalue_same_value_zero(prev, f64::from_bits(last_key.bits())) != 0 - }; - let mut idx = next_read_index(cursor, in_place, || { - crate::map::find_key_index(map(), f64::from_bits(last_key.bits())) - }); - // 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 { + let next = crate::map::map_cursor_next_raw(map(), cursor, epoch); + js_object_set_field( + iter_obj(), + 3, + JSValue::number(crate::map::map_compaction_epoch(map()) as f64), + ); + let Some(idx) = next else { 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::map_entry_key_raw(map(), idx); - // Record state for the next re-derive BEFORE any allocation below. + // Record the cursor 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())); let value = match kind { KIND_KEYS => JSValue::from_bits(entry_key.to_bits()), @@ -382,31 +345,24 @@ unsafe fn dispatch_set_iterator_method_emit( 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); + // Field 3: the backing Set's compaction epoch (see the Map arm). + let epoch = f64::from_bits(js_object_get_field(iter_obj(), 3).bits()); + let epoch = if epoch > 0.0 { epoch as u32 } else { 0 }; let used = crate::set::set_used_entries(set()); - let in_place = cursor > 0 && { - let prev = crate::set::set_value_raw(set(), cursor - 1); - crate::value::js_jsvalue_same_value_zero(prev, f64::from_bits(last_val.bits())) != 0 - }; - let mut idx = next_read_index(cursor, in_place, || { - crate::set::find_value_index(set(), f64::from_bits(last_val.bits())) - }); - // Tombstoned deletes leave holes in the raw order; step over them. - while idx < used - && crate::set::set_value_raw(set(), idx).to_bits() - == crate::set::SET_HOLE_VALUE_BITS - { - idx += 1; - } - if idx >= used { + let next = crate::set::set_cursor_next_raw(set(), cursor, epoch); + js_object_set_field( + iter_obj(), + 3, + JSValue::number(crate::set::set_compaction_epoch(set()) as f64), + ); + let Some(idx) = next else { js_object_set_field(iter_obj(), 1, JSValue::number(used as f64)); js_object_set_field(iter_obj(), 0, JSValue::undefined()); return emit_iter_result(&scope, &iter_h, emit_cached, JSValue::undefined(), true); - } + }; let elem = crate::set::set_value_raw(set(), idx); js_object_set_field(iter_obj(), 1, JSValue::number((idx + 1) as f64)); - js_object_set_field(iter_obj(), 4, JSValue::from_bits(elem.to_bits())); let value = match kind { // For Sets, keys === values; entries yields [v, v] pairs. diff --git a/crates/perry-runtime/src/gc/dead_owner.rs b/crates/perry-runtime/src/gc/dead_owner.rs index 09da9907a9..ab32b2e0f1 100644 --- a/crates/perry-runtime/src/gc/dead_owner.rs +++ b/crates/perry-runtime/src/gc/dead_owner.rs @@ -314,11 +314,23 @@ pub(super) const DEAD_KEY_PRUNES: &[DeadKeyPrune] = &[ owner: DeadKeyOwner::Any, prune: crate::map::prune_dead_map_iterator_array_owners, }, + // Re-keyed by `map_header_moved_for_gc`; a dead Map's squeeze history + // serves no cursor. + DeadKeyPrune { + table: "MAP_COMPACTION_LOG", + owner: DeadKeyOwner::Any, + prune: crate::map::prune_dead_map_compaction_log_owners, + }, DeadKeyPrune { table: "SET_ITERATOR_ARRAYS", owner: DeadKeyOwner::Any, prune: crate::set::prune_dead_set_iterator_array_owners, }, + DeadKeyPrune { + table: "SET_COMPACTION_LOG", + owner: DeadKeyOwner::Any, + prune: crate::set::prune_dead_set_compaction_log_owners, + }, DeadKeyPrune { table: "state().descriptors.property_descriptors + .accessor_descriptors", owner: DeadKeyOwner::Any, diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 6d2f5beffd..9b6ba1a560 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -51,6 +51,158 @@ pub(crate) fn map_foreach_stack_restore(depth: usize) { MAP_FOREACH_STACK.with(|stack| stack.borrow_mut().truncate(depth)); } +/// One squeeze of the entries buffer, as a raw-index cursor needs to see it: +/// which raw indices below the cursor disappeared, so it can move down by +/// exactly that many. +enum RemovedSlots { + /// `clear()` discarded the whole extent `0..n`. + Prefix(u32), + /// Compaction squeezed out these tombstoned raw indices (ascending). + Indices(Vec), +} + +impl RemovedSlots { + fn count_below(&self, cursor: u32) -> u32 { + match self { + RemovedSlots::Prefix(n) => cursor.min(*n), + RemovedSlots::Indices(v) => v.partition_point(|&i| i < cursor) as u32, + } + } +} + +struct CompactionRecord { + /// The header's `compaction_epoch` AFTER this squeeze. + epoch: u32, + removed: RemovedSlots, +} + +/// Retention budget for a map's squeeze history, in removed raw indices. +/// +/// A cursor rebases exactly only through records it has not been trimmed out +/// of, so the budget is what bounds the exactness window: exceeding it needs +/// ONE loop body to delete more than `max(MAP_COMPACTION_LOG_MIN_BUDGET, +/// capacity)` entries (deletes plus re-adds count separately) between two of +/// its own reads. Record COUNT is not the bound — a delete+re-add pair on a +/// collection at full capacity squeezes one hole per pair, so counting +/// records would make the window a few dozen pairs. +/// +/// Bounded in memory by construction: at most `budget` indices (4 bytes each) +/// plus a record header per squeeze; a `clear()` record supersedes everything +/// before it and truncates them. +const MAP_COMPACTION_LOG_MIN_BUDGET: usize = 4096; + +struct MapCompactionLog { + records: std::collections::VecDeque, + /// Sum of `records[..].removed.retained_len()`. + retained: usize, +} + +impl RemovedSlots { + /// Budget cost of this record. + fn retained_len(&self) -> usize { + match self { + RemovedSlots::Prefix(_) => 1, + RemovedSlots::Indices(v) => v.len(), + } + } +} + +crate::perry_thread_local! { + /// Per-Map history of squeezes, keyed by header address. Re-keyed on GC + /// move by `map_header_moved_for_gc`; dropped by `js_map_alloc` for a + /// reused address and by the dead-owner prune. + static MAP_COMPACTION_LOG: RefCell< + crate::fast_hash::PtrHashMap, + > = RefCell::new(crate::fast_hash::new_ptr_hash_map()); +} + +/// Append a squeeze record and advance the header epoch. Every operation +/// that moves entries to lower raw indices or discards the extent calls +/// this — it is what lets a live cursor find its entry again. +unsafe fn note_map_compaction(map: *mut MapHeader, removed: RemovedSlots) { + let epoch = (*map).compaction_epoch.wrapping_add(1); + (*map).compaction_epoch = epoch; + let budget = std::cmp::max(MAP_COMPACTION_LOG_MIN_BUDGET, (*map).capacity as usize); + MAP_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + let entry = log.entry(map as usize).or_insert_with(|| MapCompactionLog { + records: std::collections::VecDeque::new(), + retained: 0, + }); + if matches!(removed, RemovedSlots::Prefix(_)) { + // The extent was discarded: every cursor rebases to 0 through + // this record whatever came before, so older history is dead. + entry.records.clear(); + entry.retained = 0; + } + entry.retained += removed.retained_len(); + entry.records.push_back(CompactionRecord { epoch, removed }); + while entry.retained > budget && entry.records.len() > 1 { + if let Some(oldest) = entry.records.pop_front() { + entry.retained -= oldest.removed.retained_len(); + } + } + }); +} + +/// Rebase a raw-index cursor that last synchronised at `loop_epoch` onto the +/// current raw layout: each squeeze since then removed some raw indices +/// below it, and the cursor moves down by exactly that count, in order. +/// +/// Exact by the walk's own invariant: the entries a raw-index walk has +/// yielded are precisely the LIVE entries below its cursor (deletes +/// tombstone in place, adds append at the end, compaction preserves order), +/// so after a squeeze the cursor belongs at "old cursor minus removed slots +/// below it" — no key lookup, no guess about how many holes were squeezed. +unsafe fn rebase_map_cursor(map: *const MapHeader, cursor: u32, loop_epoch: u32) -> u32 { + if (*map).compaction_epoch == loop_epoch { + return cursor; + } + MAP_COMPACTION_LOG.with(|log| { + let log = log.borrow(); + let Some(entry) = log.get(&(map as usize)) else { + return cursor; + }; + let mut c = cursor; + for rec in entry + .records + .iter() + .filter(|r| r.epoch.wrapping_sub(loop_epoch) as i32 > 0) + { + c -= rec.removed.count_below(c); + } + c + }) +} + +/// The next live raw index at or after `cursor` (itself rebased through any +/// squeezes since `loop_epoch`), or `None` when the extent is exhausted. +/// +/// The raw-index walkers — the `for…of` fast path and the iterator objects — +/// call this once per step. It is O(holes stepped over) and NEVER compacts: +/// the reader-side "self-heal" compaction it replaces ran once per hole +/// observed, which made a delete-during-`for…of` loop O(n) per delete, and +/// the single-hole cursor arithmetic that went with it skipped entries when +/// several holes were squeezed at once. +pub(crate) unsafe fn map_cursor_next_raw( + map: *const MapHeader, + cursor: u32, + loop_epoch: u32, +) -> Option { + let mut idx = rebase_map_cursor(map, cursor, loop_epoch); + let used = (*map).used; + let entries = entries_ptr(map); + while idx < used && ptr::read(entries.add(idx as usize * 2)).to_bits() == MAP_HOLE_KEY_BITS { + idx += 1; + } + (idx < used).then_some(idx) +} + +#[inline] +pub(crate) fn map_compaction_epoch(map: *const MapHeader) -> u32 { + unsafe { (*map).compaction_epoch } +} + fn mark_map_iterator_array(arr: *mut crate::array::ArrayHeader) { if !arr.is_null() { MAP_ITERATOR_ARRAYS.with(|r| { @@ -90,11 +242,31 @@ pub(crate) fn prune_dead_map_iterator_array_owners(is_dead_owner: &dyn Fn(usize) }); } +/// Dead-owner prune for `MAP_COMPACTION_LOG`: a dead Map's squeeze history +/// has no cursor left to serve. +pub(crate) fn prune_dead_map_compaction_log_owners(is_dead_owner: &dyn Fn(usize) -> bool) { + MAP_COMPACTION_LOG.with(|log| { + log.borrow_mut().retain(|owner, _| !is_dead_owner(*owner)); + }); +} + #[cfg(test)] pub(crate) fn test_clear_map_iterator_arrays() { MAP_ITERATOR_ARRAYS.with(|r| r.borrow_mut().clear()); } +/// Test-only: the allocation-time reset a fresh Map performs on a reused +/// address (`js_map_alloc` drops any stale squeeze log and zeroes the epoch), +/// applied to an existing header so a test can prove a cursor from a previous +/// tenant's history is not rebased. +#[cfg(test)] +pub(crate) fn test_reset_compaction_log_for(map: *mut MapHeader) { + MAP_COMPACTION_LOG.with(|log| { + log.borrow_mut().remove(&(map as usize)); + }); + unsafe { (*map).compaction_epoch = 0 }; +} + #[cfg(test)] crate::perry_thread_local! { static TEST_FORCE_HELPER_GC: std::cell::Cell = const { std::cell::Cell::new(0) }; @@ -822,6 +994,13 @@ pub(crate) fn map_header_moved_for_gc(old_addr: usize, new_addr: usize) { idx.insert(new_addr, slot); } }); + MAP_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + log.remove(&new_addr); + if let Some(records) = log.remove(&old_addr) { + log.insert(new_addr, records); + } + }); MAP_FOREACH_STACK.with(|stack| { for addr in stack.borrow_mut().iter_mut() { if *addr == old_addr { @@ -1003,6 +1182,7 @@ pub(crate) fn release_current_thread_map_side_allocations() { } MAP_STRING_INDEX.with(|idx| idx.borrow_mut().clear()); MAP_PTR_INDEX.with(|idx| idx.borrow_mut().clear()); + MAP_COMPACTION_LOG.with(|log| log.borrow_mut().clear()); } #[cfg(test)] @@ -1131,9 +1311,17 @@ pub struct MapHeader { 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). + /// of tombstoned entries awaiting compaction. Codegen reads it at offset + /// 32 (pinned below). pub used: u32, + /// Bumped by every operation that moves an entry to a LOWER raw index + /// (`compact_map_entries`) or discards the extent (`clear`). A raw-index + /// cursor — the `for…of` fast path, the iterator objects — records the + /// epoch it last synchronised with; when the header's epoch has moved on, + /// the cursor rebases itself through `MAP_COMPACTION_LOG` (see + /// `map_cursor_next_raw`) instead of guessing. Appended last so every + /// preceding offset is unchanged; fits in the padding after `used`. + pub compaction_epoch: u32, } const _: () = { @@ -1141,6 +1329,8 @@ const _: () = { assert!(std::mem::offset_of!(MapHeader, capacity) == 4); assert!(std::mem::offset_of!(MapHeader, entries) == 8); assert!(std::mem::offset_of!(MapHeader, used) == 32); + assert!(std::mem::offset_of!(MapHeader, compaction_epoch) == 36); + assert!(std::mem::size_of::() == 40); }; /// The tombstone a deleted entry's KEY slot takes. Never a legal stored key: @@ -1389,6 +1579,13 @@ pub extern "C" fn js_map_alloc(capacity: u32) -> *mut MapHeader { // meta edge is a garbage pointer the collector would follow. (*ptr).meta = std::ptr::null_mut(); (*ptr).used = 0; + (*ptr).compaction_epoch = 0; + // A previous tenant of this address may have left a compaction log + // behind if the dead-owner prune has not run yet; a fresh Map must + // start with no history, or a cursor could rebase through it. + MAP_COMPACTION_LOG.with(|log| { + log.borrow_mut().remove(&(ptr as usize)); + }); // Register in map registry for runtime type detection register_map(ptr, entries, cap as usize); @@ -1454,6 +1651,42 @@ 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; +/// C-ABI, for the `for…of` fast path (`perry-hir`'s +/// `map_set_delete_safe_for_of`): the next live raw index at or after +/// `cursor`, after rebasing it through every squeeze since `epoch`, or +/// `-1.0` when the extent is exhausted. Takes the NaN-boxed collection; the +/// cursor and epoch are the loop's Number state temps. +#[no_mangle] +pub extern "C" fn js_map_cursor_next(map_boxed: f64, cursor: f64, epoch: f64) -> f64 { + let map = clean_map_ptr(crate::value::js_nanbox_get_pointer(map_boxed) as *const MapHeader); + if map.is_null() { + return -1.0; + } + let cursor = if cursor > 0.0 { cursor as u32 } else { 0 }; + let epoch = if epoch > 0.0 { epoch as u32 } else { 0 }; + match unsafe { map_cursor_next_raw(map, cursor, epoch) } { + Some(idx) => idx as f64, + None => -1.0, + } +} +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_MAP_CURSOR_NEXT: extern "C" fn(f64, f64, f64) -> f64 = js_map_cursor_next; + +/// C-ABI companion: the header's compaction epoch, which the loop stores +/// after each step so the next `js_map_cursor_next` knows what it has seen. +#[no_mangle] +pub extern "C" fn js_map_compaction_epoch(map_boxed: f64) -> f64 { + let map = clean_map_ptr(crate::value::js_nanbox_get_pointer(map_boxed) as *const MapHeader); + if map.is_null() { + return 0.0; + } + map_compaction_epoch(map) as f64 +} +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_MAP_COMPACTION_EPOCH: extern "C" fn(f64) -> f64 = js_map_compaction_epoch; + /// Live-extent accessor for iteration (`0..used` are the raw entry indices). #[inline(always)] pub(crate) fn map_used_entries(map: *const MapHeader) -> u32 { @@ -1488,9 +1721,12 @@ unsafe fn compact_map_entries(map: *mut MapHeader) { let used = (*map).used as usize; let entries = entries_ptr_mut(map); let mut out = 0usize; + // The raw indices squeezed out, for the cursors that were walking them. + let mut removed: Vec = Vec::with_capacity(used.saturating_sub((*map).size as usize)); for i in 0..used { let key = ptr::read(entries.add(i * 2)); if key.to_bits() == MAP_HOLE_KEY_BITS { + removed.push(i as u32); continue; } if out != i { @@ -1539,6 +1775,9 @@ unsafe fn compact_map_entries(map: *mut MapHeader) { } }); rebuild_map_ptr_index(map); + if !removed.is_empty() { + note_map_compaction(map, RemovedSlots::Indices(removed)); + } } pub(crate) unsafe fn compact_if_holey(map: *mut MapHeader) { @@ -2564,8 +2803,11 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { let size = unsafe { (*map).size }; let used = unsafe { (*map).used }; if size == 0 { - if !map_foreach_is_active(map) { - unsafe { (*map).used = 0 }; + if !map_foreach_is_active(map) && used > 0 { + unsafe { + (*map).used = 0; + note_map_compaction(map, RemovedSlots::Prefix(used)); + } } return; } @@ -2606,7 +2848,12 @@ pub extern "C" fn js_map_clear(map: *mut MapHeader) { } } } else { + // Discarding the extent moves nothing, but a live raw-index + // cursor must restart at 0 to see what is appended next (spec: + // the [[MapData]] list is emptied in place, later adds are + // visited), so record it as a squeeze of the whole extent. (*map).used = 0; + note_map_compaction(map, RemovedSlots::Prefix(used)); } } unsafe { @@ -2643,21 +2890,46 @@ 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); + // LIVE-index accessor (#9462 / #9504): `idx` counts live entries, the + // contract `js_array_length` (== `size`) pairs with for the + // array-like `map[i]` read, `console.table` and collection equality. + // Tombstones are squeezed first so raw index == live index — a one-off + // O(n) on a holey collection for those paths only, and RECORDED in the + // compaction log, so a `for…of` cursor open on this map rebases + // exactly. The walkers themselves never come here: they read through + // `js_map_entry_key_raw_at`, which never compacts (a compaction per + // observed hole is what made delete-during-`for…of` O(n) per delete). + compact_if_holey(map as *mut MapHeader); + if idx >= (*map).size { + return f64::from_bits(TAG_UNDEFINED); } - let size = (*map).size; - if idx >= size { + let entries = entries_ptr(map); + ptr::read(entries.add(idx as usize * 2)) + } +} + +/// RAW twin of `js_map_entry_key_at` for the raw-index walkers (the `for…of` +/// fast path's `MapEntryKeyAt` lowering): bounded by the raw extent `used`, +/// never compacts. The index comes from `js_map_cursor_next`, which only +/// ever yields a LIVE raw index, so no hole reaches user code. +#[no_mangle] +pub extern "C" fn js_map_entry_key_raw_at(map: *const MapHeader, idx: u32) -> f64 { + let map = clean_map_ptr(map); + if map.is_null() { + return f64::from_bits(TAG_UNDEFINED); + } + unsafe { + if idx >= (*map).used { return f64::from_bits(TAG_UNDEFINED); } let entries = entries_ptr(map); ptr::read(entries.add(idx as usize * 2)) } } +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_MAP_ENTRY_KEY_RAW_AT: extern "C" fn(*const MapHeader, u32) -> f64 = + js_map_entry_key_raw_at; /// Companion to `js_map_entry_key_at` — read the value at entry index `idx`. #[no_mangle] @@ -2667,21 +2939,36 @@ 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); + // Live-index accessor — see `js_map_entry_key_at`. + compact_if_holey(map as *mut MapHeader); + if idx >= (*map).size { + return f64::from_bits(TAG_UNDEFINED); } - let size = (*map).size; - if idx >= size { + let entries = entries_ptr(map); + ptr::read(entries.add(idx as usize * 2 + 1)) + } +} + +/// RAW twin of `js_map_entry_value_at` for the raw-index walkers: bounded by +/// `used`, never compacts. +#[no_mangle] +pub extern "C" fn js_map_entry_value_raw_at(map: *const MapHeader, idx: u32) -> f64 { + let map = clean_map_ptr(map); + if map.is_null() { + return f64::from_bits(TAG_UNDEFINED); + } + unsafe { + if idx >= (*map).used { return f64::from_bits(TAG_UNDEFINED); } let entries = entries_ptr(map); ptr::read(entries.add(idx as usize * 2 + 1)) } } +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_MAP_ENTRY_VALUE_RAW_AT: extern "C" fn(*const MapHeader, u32) -> f64 = + js_map_entry_value_raw_at; /// Get the entries of a map as an array of [key, value] pairs /// Returns an array where each element is a 2-element array [key, value] @@ -3628,11 +3915,22 @@ mod tests { .filter(|(i, _)| *i != 1) .map(|(_, key)| key.to_bits()), ); - let actual_keys = (0..js_map_size(map)) - .map(|i| js_map_entry_key_at(map, i).to_bits()) - .collect::>(); + // Walk the LIVE entries the way a for-of does: raw reads never + // compact any more, so raw index != live index while holes exist, + // and the cursor is what steps over them. + let live_keys = |map: *mut MapHeader| { + let mut keys = Vec::new(); + let mut idx = 0u32; + while let Some(i) = unsafe { map_cursor_next_raw(map, idx, map_compaction_epoch(map)) } + { + keys.push(js_map_entry_key_raw_at(map, i).to_bits()); + idx = i + 1; + } + keys + }; assert_eq!( - actual_keys, expected_keys, + live_keys(map), + expected_keys, "delete must preserve survivor order" ); @@ -3640,15 +3938,18 @@ mod tests { js_map_set_string_number(map, string_key_ptr(4), 444.0); js_map_set(map, pointer_keys[1], 1_111.0); assert_eq!(js_map_size(map), 28); - assert_eq!(js_map_entry_key_at(map, 25).to_bits(), 2.0f64.to_bits()); + let after_readd = live_keys(map); + assert_eq!(after_readd.len(), 28); + assert_eq!(after_readd[25], 2.0f64.to_bits()); assert_eq!( - js_map_entry_key_at(map, 26).to_bits(), + after_readd[26], string_keys[4].get_nanbox_f64().to_bits(), "delete-then-re-add must append at the end" ); assert_eq!( - js_map_entry_key_at(map, 27).to_bits(), - pointer_keys[1].to_bits() + after_readd[27], + pointer_keys[1].to_bits(), + "delete-then-re-add must append at the end" ); } diff --git a/crates/perry-runtime/src/map_tombstone_tests.rs b/crates/perry-runtime/src/map_tombstone_tests.rs index 549972572f..3f95b3d546 100644 --- a/crates/perry-runtime/src/map_tombstone_tests.rs +++ b/crates/perry-runtime/src/map_tombstone_tests.rs @@ -4,8 +4,9 @@ //! 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. +//! correctness across holes, iterator hole-skips, the no-compaction +//! contract of the raw-indexed readers, and the exact epoch-based cursor +//! rebase across squeezes (single-hole, multi-hole, successive, and `clear`). use super::*; @@ -171,7 +172,7 @@ fn emptying_a_map_stays_consistent_and_compacts() { } #[test] -fn raw_indexed_access_self_heals_by_compacting() { +fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { let map = js_map_alloc(8); for k in [1.0f64, 2.0, 3.0] { js_map_set(map, k, k); @@ -180,14 +181,165 @@ fn raw_indexed_access_self_heals_by_compacting() { 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); + // The RAW twins the for-of walkers use are plain bounded reads: raw + // index 2 is still the THIRD key, the hole at 1 stays, and the layout is + // untouched — the walkers' reads used to compact the whole map once per + // observed hole. + assert_eq!(js_map_entry_key_raw_at(map, 2), 3.0); + assert_eq!(js_map_entry_value_raw_at(map, 2), 3.0); + assert_eq!( + js_map_entry_key_raw_at(map, 1).to_bits(), + MAP_HOLE_KEY_BITS, + "the raw twin exposes the hole — only the cursor ever reads it" + ); unsafe { - assert_eq!((*map).used, (*map).size, "access healed the layout"); + assert_ne!( + (*map).used, + (*map).size, + "the raw read left the layout alone" + ); + assert_eq!( + crate::map::map_compaction_epoch(map), + 0, + "no squeeze happened" + ); + // The hole is visible only to the cursor walker, which steps over it. + assert_eq!(crate::map::map_cursor_next_raw(map, 0, 0), Some(0)); + assert_eq!( + crate::map::map_cursor_next_raw(map, 1, 0), + Some(2), + "hole at 1 skipped" + ); + assert_eq!( + crate::map::map_cursor_next_raw(map, 3, 0), + None, + "extent exhausted" + ); + } + // The LIVE-index accessors (#9462 / #9504 — the array-like `map[i]` read, + // console.table, collection equality) squeeze first so that live index 1 + // IS the third key and never a hole… + assert_eq!( + js_map_entry_key_at(map, 1), + 3.0, + "live index 1 is the third key" + ); + assert_eq!(js_map_entry_value_at(map, 1), 3.0); + assert_eq!( + js_map_entry_key_at(map, 2).to_bits(), + crate::value::TAG_UNDEFINED, + "past the live size is undefined, not a hole" + ); + unsafe { + assert_eq!( + (*map).used, + (*map).size, + "the live accessor squeezed the hole" + ); + assert_eq!( + crate::map::map_compaction_epoch(map), + 1, + "…and recorded it, so a cursor that was past the hole (raw 2, synced \ + at epoch 0) rebases onto the third key's new raw index 1" + ); + assert_eq!(crate::map::map_cursor_next_raw(map, 2, 0), Some(1)); + assert_eq!(js_map_entry_key_raw_at(map, 1), 3.0); } } +#[test] +fn cursor_rebases_exactly_across_a_multi_hole_compaction() { + // 40 keys. A walk has yielded k0..k20 (cursor = 21) when the body deletes + // exactly those 21 — holes now outnumber the 19 live entries, so the + // delete path squeezes 21 holes below the cursor in ONE compaction. The + // old key-based recovery read `cursor-1` and skipped 19 entries; the + // rebase moves the cursor down by the removed count below it: 21 → 0. + let map = js_map_alloc(64); + for i in 0..40 { + js_map_set(map, i as f64, i as f64); + } + let epoch0 = crate::map::map_compaction_epoch(map); + for i in 0..=20 { + assert_eq!(js_map_delete(map, i as f64), 1); + } + unsafe { + assert_eq!((*map).used, (*map).size, "the delete path compacted"); + } + assert_ne!(crate::map::map_compaction_epoch(map), epoch0); + let next = unsafe { crate::map::map_cursor_next_raw(map, 21, epoch0) }; + assert_eq!( + next, + Some(0), + "cursor 21 minus the 21 slots removed below it" + ); + assert_eq!( + js_map_entry_key_at(map, 0), + 21.0, + "which is the true next key" + ); + // A cursor already synchronised with the new epoch is not rebased again. + let epoch1 = crate::map::map_compaction_epoch(map); + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 3, epoch1) }, + Some(3) + ); +} + +#[test] +fn cursor_rebases_through_successive_squeezes_and_clear() { + let map = js_map_alloc(64); + for i in 0..40 { + js_map_set(map, i as f64, i as f64); + } + let epoch0 = crate::map::map_compaction_epoch(map); + // Squeeze 1 at the 21st delete (k0..k20 gone), squeeze 2 at the 32nd + // (k21..k31 gone from the compacted layout: 8 live < 19 / 2). The cursor, + // still at raw 21 with epoch0, must rebase through BOTH records in order. + for i in 0..=31 { + assert_eq!(js_map_delete(map, i as f64), 1); + } + unsafe { + assert_eq!((*map).used, (*map).size); + assert_eq!((*map).size, 8); + } + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 21, epoch0) }, + Some(0) + ); + assert_eq!(js_map_entry_key_at(map, 0), 32.0); + // clear() during a walk discards the extent; a cursor then resumes at 0 + // and visits whatever is appended afterwards (the spec empties the + // [[MapData]] list in place, so later adds are visited). + let epoch1 = crate::map::map_compaction_epoch(map); + js_map_clear(map); + js_map_set(map, 100.0, 1.0); + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 5, epoch1) }, + Some(0) + ); + assert_eq!(js_map_entry_key_at(map, 0), 100.0); +} + +#[test] +fn a_fresh_map_at_a_reused_address_starts_without_history() { + // A previous tenant's squeeze log must not rebase a new Map's cursor. + let map = js_map_alloc(64); + for i in 0..40 { + js_map_set(map, i as f64, i as f64); + } + for i in 0..=20 { + js_map_delete(map, i as f64); + } + assert_ne!(crate::map::map_compaction_epoch(map), 0); + // Simulate address reuse: re-run the allocation-time reset on this + // header and check a stale-epoch cursor is left alone. + crate::map::test_reset_compaction_log_for(map); + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 5, 0) }, + Some(5) + ); +} + #[test] fn iterator_skips_holes_and_survives_deleting_the_last_returned_key() { unsafe { @@ -258,3 +410,78 @@ unsafe fn iter_backing(iter: f64) -> *mut MapHeader { crate::object::js_object_get_field(obj, 0).bits(), )) as *mut MapHeader } + +#[test] +fn cursor_stays_exact_across_forty_squeezes_in_one_body() { + // A map at FULL capacity squeezes once per delete+re-add pair on the grow + // path (`ensure_capacity`: used == capacity with a hole → compact), so one + // loop body can force dozens of squeezes between two reads of a cursor. + // History is budgeted by removed-index count, not record count, so all of + // them are retained and the rebase stays exact. + let map = js_map_alloc(64); + for i in 0..64 { + js_map_set(map, i as f64, i as f64); + } + unsafe { + assert_eq!((*map).used, (*map).capacity, "premise: at capacity"); + } + let epoch0 = crate::map::map_compaction_epoch(map); + // The walk has yielded k0..k9 (cursor = 10). The body then deletes and + // re-adds k0..k39: ten holes BELOW the cursor, thirty above, forty + // squeezes in total. + for i in 0..40 { + assert_eq!(js_map_delete(map, i as f64), 1); + js_map_set(map, i as f64, (i * 100) as f64); + } + let squeezes = crate::map::map_compaction_epoch(map).wrapping_sub(epoch0); + assert!( + squeezes >= 33, + "premise: {squeezes} squeezes happened, need > 32" + ); + unsafe { + assert_eq!((*map).used, (*map).size); + } + // Every entry yielded so far (k0..k9) was deleted, so no live entry + // remains below the old cursor: it rebases to 0, where the first + // not-yet-visited live entry now sits — k40, because k10..k39 were + // re-appended behind k40..k63 and behind the re-added k0..k9. + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 10, epoch0) }, + Some(0) + ); + assert_eq!(js_map_entry_key_raw_at(map, 0), 40.0); + // Live order is k40..k63, k0..k9, k10..k39 — the raw walk from the + // rebased cursor sees exactly that. + let mut order = Vec::new(); + let mut idx = 0u32; + let epoch_now = crate::map::map_compaction_epoch(map); + while let Some(i) = unsafe { crate::map::map_cursor_next_raw(map, idx, epoch_now) } { + order.push(js_map_entry_key_raw_at(map, i)); + idx = i + 1; + } + let expected: Vec = (40..64).chain(0..40).map(|k| k as f64).collect(); + assert_eq!(order, expected); +} + +#[test] +fn clear_truncates_the_squeeze_history() { + let map = js_map_alloc(64); + for i in 0..64 { + js_map_set(map, i as f64, i as f64); + } + let epoch0 = crate::map::map_compaction_epoch(map); + for i in 0..40 { + js_map_delete(map, i as f64); + js_map_set(map, i as f64, 0.0); + } + js_map_clear(map); + js_map_set(map, 7.0, 7.0); + // Whatever the cursor was, a clear rebases it to 0 — and the log behind + // the clear record is gone, so this holds however many squeezes preceded + // it. + assert_eq!( + unsafe { crate::map::map_cursor_next_raw(map, 10, epoch0) }, + Some(0) + ); + assert_eq!(js_map_entry_key_raw_at(map, 0), 7.0); +} diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 3445ef7e94..6595d0e20c 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -336,6 +336,7 @@ pub(crate) fn test_clear_set_roots() { drop(allocation); } SET_INDEX.with(|idx| idx.borrow_mut().clear()); + SET_COMPACTION_LOG.with(|log| log.borrow_mut().clear()); } pub fn scan_set_roots(_mark: &mut dyn FnMut(f64)) { @@ -411,6 +412,13 @@ pub(crate) fn set_header_moved_for_gc(old_addr: usize, new_addr: usize) { idx.insert(new_addr, slot); } }); + SET_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + log.remove(&new_addr); + if let Some(records) = log.remove(&old_addr) { + log.insert(new_addr, records); + } + }); SET_FOREACH_STACK.with(|stack| { for addr in stack.borrow_mut().iter_mut() { if *addr == old_addr { @@ -558,6 +566,7 @@ pub(crate) fn release_current_thread_set_side_allocations() { drop(allocation); } SET_INDEX.with(|idx| idx.borrow_mut().clear()); + SET_COMPACTION_LOG.with(|log| log.borrow_mut().clear()); } /// Set header - GC-movable address, elements allocated separately @@ -577,8 +586,14 @@ pub struct SetHeader { pub meta: *mut crate::object::ObjectMeta, /// Extent of the elements array actually written: raw element indices run /// `0..used`. `size` stays the LIVE count; `used - size` counts the - /// tombstoned slots awaiting compaction. Appended last (offset pinned). + /// tombstoned slots awaiting compaction (offset pinned). pub used: u32, + /// Bumped whenever an element moves to a LOWER raw index + /// (`compact_set_elements`) or the extent is discarded (`clear`). Raw-index + /// cursors record the epoch they last saw and rebase through + /// `SET_COMPACTION_LOG` when it has moved (see `set_cursor_next_raw`). + /// Appended last; fits in the padding after `used`. + pub compaction_epoch: u32, } const _: () = { @@ -586,6 +601,8 @@ const _: () = { assert!(std::mem::offset_of!(SetHeader, capacity) == 4); assert!(std::mem::offset_of!(SetHeader, elements) == 8); assert!(std::mem::offset_of!(SetHeader, used) == 24); + assert!(std::mem::offset_of!(SetHeader, compaction_epoch) == 28); + assert!(std::mem::size_of::() == 32); }; /// The tombstone a deleted element's slot takes — same reserved marker as the @@ -875,13 +892,199 @@ pub(crate) unsafe fn set_value_raw(set: *const SetHeader, idx: u32) -> f64 { /// Squeeze the tombstones out (insertion order preserved), then rebuild the /// lookup index from the dense buffer. +/// One squeeze of the elements buffer, as a raw-index cursor needs to see it +/// (the Map twin is `map::RemovedSlots`): which raw indices below the cursor +/// disappeared, so it can move down by exactly that many. +enum SetRemovedSlots { + /// `clear()` discarded the whole extent `0..n`. + Prefix(u32), + /// Compaction squeezed out these tombstoned raw indices (ascending). + Indices(Vec), +} + +impl SetRemovedSlots { + fn count_below(&self, cursor: u32) -> u32 { + match self { + SetRemovedSlots::Prefix(n) => cursor.min(*n), + SetRemovedSlots::Indices(v) => v.partition_point(|&i| i < cursor) as u32, + } + } +} + +struct SetCompactionRecord { + /// The header's `compaction_epoch` AFTER this squeeze. + epoch: u32, + removed: SetRemovedSlots, +} + +/// Retention budget for a set's squeeze history, in removed raw indices. +/// +/// A cursor rebases exactly only through records it has not been trimmed out +/// of, so the budget is what bounds the exactness window: exceeding it needs +/// ONE loop body to delete more than `max(SET_COMPACTION_LOG_MIN_BUDGET, +/// capacity)` entries (deletes plus re-adds count separately) between two of +/// its own reads. Record COUNT is not the bound — a delete+re-add pair on a +/// collection at full capacity squeezes one hole per pair, so counting +/// records would make the window a few dozen pairs. +/// +/// Bounded in memory by construction: at most `budget` indices (4 bytes each) +/// plus a record header per squeeze; a `clear()` record supersedes everything +/// before it and truncates them. +const SET_COMPACTION_LOG_MIN_BUDGET: usize = 4096; + +struct SetCompactionLog { + records: std::collections::VecDeque, + /// Sum of `records[..].removed.retained_len()`. + retained: usize, +} + +impl SetRemovedSlots { + /// Budget cost of this record. + fn retained_len(&self) -> usize { + match self { + SetRemovedSlots::Prefix(_) => 1, + SetRemovedSlots::Indices(v) => v.len(), + } + } +} + +crate::perry_thread_local! { + /// Per-Set history of squeezes, keyed by header address. Re-keyed on GC + /// move by `set_header_moved_for_gc`; dropped by `js_set_alloc` for a + /// reused address and by the dead-owner prune. + static SET_COMPACTION_LOG: RefCell< + crate::fast_hash::PtrHashMap, + > = RefCell::new(crate::fast_hash::new_ptr_hash_map()); +} + +/// Append a squeeze record and advance the header epoch (every operation +/// that moves elements to lower raw indices or discards the extent). +unsafe fn note_set_compaction(set: *mut SetHeader, removed: SetRemovedSlots) { + let epoch = (*set).compaction_epoch.wrapping_add(1); + (*set).compaction_epoch = epoch; + let budget = std::cmp::max(SET_COMPACTION_LOG_MIN_BUDGET, (*set).capacity as usize); + SET_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + let entry = log.entry(set as usize).or_insert_with(|| SetCompactionLog { + records: std::collections::VecDeque::new(), + retained: 0, + }); + if matches!(removed, SetRemovedSlots::Prefix(_)) { + // The extent was discarded: every cursor rebases to 0 through + // this record whatever came before, so older history is dead. + entry.records.clear(); + entry.retained = 0; + } + entry.retained += removed.retained_len(); + entry + .records + .push_back(SetCompactionRecord { epoch, removed }); + while entry.retained > budget && entry.records.len() > 1 { + if let Some(oldest) = entry.records.pop_front() { + entry.retained -= oldest.removed.retained_len(); + } + } + }); +} + +/// Rebase a raw-index cursor that last synchronised at `loop_epoch` onto the +/// current raw layout — exact for the same reason as `map::rebase_map_cursor`: +/// the elements a walk has yielded are precisely the live ones below its +/// cursor, so each squeeze moves the cursor down by the removed count below it. +unsafe fn rebase_set_cursor(set: *const SetHeader, cursor: u32, loop_epoch: u32) -> u32 { + if (*set).compaction_epoch == loop_epoch { + return cursor; + } + SET_COMPACTION_LOG.with(|log| { + let log = log.borrow(); + let Some(entry) = log.get(&(set as usize)) else { + return cursor; + }; + let mut c = cursor; + for rec in entry + .records + .iter() + .filter(|r| r.epoch.wrapping_sub(loop_epoch) as i32 > 0) + { + c -= rec.removed.count_below(c); + } + c + }) +} + +/// The next live raw index at or after `cursor` (rebased through any +/// squeezes since `loop_epoch`), or `None` when the extent is exhausted. +/// O(holes stepped over); never compacts. See `map::map_cursor_next_raw`. +pub(crate) unsafe fn set_cursor_next_raw( + set: *const SetHeader, + cursor: u32, + loop_epoch: u32, +) -> Option { + let mut idx = rebase_set_cursor(set, cursor, loop_epoch); + let used = (*set).used; + let elements = elements_ptr(set); + while idx < used && ptr::read(elements.add(idx as usize)).to_bits() == SET_HOLE_VALUE_BITS { + idx += 1; + } + (idx < used).then_some(idx) +} + +#[inline] +pub(crate) fn set_compaction_epoch(set: *const SetHeader) -> u32 { + unsafe { (*set).compaction_epoch } +} + +/// Dead-owner prune for `SET_COMPACTION_LOG`. +pub(crate) fn prune_dead_set_compaction_log_owners(is_dead_owner: &dyn Fn(usize) -> bool) { + SET_COMPACTION_LOG.with(|log| { + log.borrow_mut().retain(|owner, _| !is_dead_owner(*owner)); + }); +} + +/// C-ABI, for the `for…of` fast path: next live raw index at or after +/// `cursor` after rebasing through squeezes since `epoch`, or `-1.0` when +/// exhausted. NaN-boxed set; cursor/epoch are the loop's Number temps. +#[no_mangle] +pub extern "C" fn js_set_cursor_next(set_boxed: f64, cursor: f64, epoch: f64) -> f64 { + let set = clean_set_ptr(crate::value::js_nanbox_get_pointer(set_boxed) as *const SetHeader); + if set.is_null() { + return -1.0; + } + let cursor = if cursor > 0.0 { cursor as u32 } else { 0 }; + let epoch = if epoch > 0.0 { epoch as u32 } else { 0 }; + match unsafe { set_cursor_next_raw(set, cursor, epoch) } { + Some(idx) => idx as f64, + None => -1.0, + } +} +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_SET_CURSOR_NEXT: extern "C" fn(f64, f64, f64) -> f64 = js_set_cursor_next; + +/// C-ABI companion: the header's compaction epoch the loop stores after +/// each step. +#[no_mangle] +pub extern "C" fn js_set_compaction_epoch(set_boxed: f64) -> f64 { + let set = clean_set_ptr(crate::value::js_nanbox_get_pointer(set_boxed) as *const SetHeader); + if set.is_null() { + return 0.0; + } + set_compaction_epoch(set) as f64 +} +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_SET_COMPACTION_EPOCH: extern "C" fn(f64) -> f64 = js_set_compaction_epoch; + unsafe fn compact_set_elements(set: *mut SetHeader) { let used = (*set).used as usize; let elements = elements_ptr_mut(set); let mut out = 0usize; + // The raw indices squeezed out, for the cursors that were walking them. + let mut removed: Vec = Vec::with_capacity(used.saturating_sub((*set).size as usize)); for i in 0..used { let v = ptr::read(elements.add(i)); if v.to_bits() == SET_HOLE_VALUE_BITS { + removed.push(i as u32); continue; } if out != i { @@ -900,6 +1103,9 @@ unsafe fn compact_set_elements(set: *mut SetHeader) { crate::gc::runtime_write_barrier_external_slot_span(set as usize, elements as usize, out); } rebuild_set_index(set); + if !removed.is_empty() { + note_set_compaction(set, SetRemovedSlots::Indices(removed)); + } } pub(crate) unsafe fn compact_if_holey_set(set: *mut SetHeader) { @@ -1035,6 +1241,13 @@ pub extern "C" fn js_set_alloc(capacity: u32) -> *mut SetHeader { // uninitialised meta edge would be a garbage pointer the GC follows. (*ptr).meta = std::ptr::null_mut(); (*ptr).used = 0; + (*ptr).compaction_epoch = 0; + // A previous tenant of this address may have left a compaction log + // behind if the dead-owner prune has not run yet; a fresh Set must + // start with no history. + SET_COMPACTION_LOG.with(|log| { + log.borrow_mut().remove(&(ptr as usize)); + }); // Register in set registry for runtime type detection register_set(ptr, elements, cap as usize); @@ -1602,12 +1815,14 @@ pub extern "C" fn js_set_clear(set: *mut SetHeader) { } unsafe { let active_foreach = set_foreach_is_active(set); + let extent = (*set).used; // The side-table mirrors the elements exactly, so an already-empty // set has nothing to reset — half of a change set's per-entity // `adds.clear(); removes.clear()` — and skips the table probe. if (*set).size == 0 { - if !active_foreach { + if !active_foreach && extent > 0 { (*set).used = 0; + note_set_compaction(set, SetRemovedSlots::Prefix(extent)); } return; } @@ -1629,7 +1844,12 @@ pub extern "C" fn js_set_clear(set: *mut SetHeader) { } } } else { + // Discarding the extent moves nothing, but a live raw-index + // cursor must restart at 0 to see what is appended next (spec: + // [[SetData]] is emptied in place, later adds are visited), so + // record it as a squeeze of the whole extent. (*set).used = 0; + note_set_compaction(set, SetRemovedSlots::Prefix(extent)); } } SET_INDEX.with(|idx| { @@ -1652,11 +1872,14 @@ pub extern "C" fn js_set_value_at(set: *const SetHeader, i: u32) -> f64 { return f64::from_bits(UNDEF); } unsafe { - if (*set).used != (*set).size { - // Raw-indexed access with holes present: compact so raw == live - // again for every external walker that loops `0..size`. - compact_set_elements(set as *mut SetHeader); - } + // LIVE-index accessor (#9462 / #9504): `i` counts live elements, the + // contract `js_array_length` (== `size`) pairs with for the array-like + // `set[i]` read, `console.table` and collection equality. Tombstones + // are squeezed first so raw index == live index — a one-off O(n) for + // those paths only, RECORDED in the compaction log so an open + // `for…of` cursor rebases exactly. The walkers read through + // `js_set_value_raw_at`, which never compacts. + compact_if_holey_set(set as *mut SetHeader); if i >= (*set).size { return f64::from_bits(UNDEF); } @@ -1665,6 +1888,29 @@ pub extern "C" fn js_set_value_at(set: *const SetHeader, i: u32) -> f64 { } } +/// RAW twin of `js_set_value_at` for the raw-index walkers (the `for…of` +/// fast path's `SetValueAt` lowering): bounded by the raw extent `used`, +/// never compacts. The index comes from `js_set_cursor_next`, which only +/// yields LIVE raw indices, so no hole reaches user code. +#[no_mangle] +pub extern "C" fn js_set_value_raw_at(set: *const SetHeader, i: u32) -> f64 { + const UNDEF: u64 = 0x7FFC_0000_0000_0001; + let set = clean_set_ptr(set); + if set.is_null() { + return f64::from_bits(UNDEF); + } + unsafe { + if i >= (*set).used { + return f64::from_bits(UNDEF); + } + let elements = (*set).elements as *const f64; + ptr::read(elements.add(i as usize)) + } +} +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_SET_VALUE_RAW_AT: extern "C" fn(*const SetHeader, u32) -> f64 = js_set_value_raw_at; + /// Convert a Set to an Array (for Array.from(set)) /// Returns a new array containing all elements of the set. /// @@ -2887,6 +3133,7 @@ mod tests { elements: std::ptr::null_mut(), meta: std::ptr::null_mut(), used: 1, + compaction_epoch: 0, }; let cases: &[(&str, *mut f64)] = &[ diff --git a/crates/perry-runtime/src/set_tombstone_tests.rs b/crates/perry-runtime/src/set_tombstone_tests.rs index b8a810fcb7..f0ebd355f8 100644 --- a/crates/perry-runtime/src/set_tombstone_tests.rs +++ b/crates/perry-runtime/src/set_tombstone_tests.rs @@ -142,7 +142,7 @@ fn emptying_a_set_stays_consistent_and_compacts() { } #[test] -fn raw_indexed_access_self_heals_by_compacting() { +fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { let set = js_set_alloc(8); for v in [1.0f64, 2.0, 3.0] { js_set_add(set, v); @@ -151,10 +151,120 @@ fn raw_indexed_access_self_heals_by_compacting() { unsafe { assert_ne!((*set).used, (*set).size, "a hole is present"); } - assert_eq!(js_set_value_at(set, 1), 3.0, "extern read compacts first"); + // The RAW twin the for-of walker uses is a plain bounded read: raw index + // 2 is still the THIRD value, the hole at 1 stays, the layout is untouched + // (the walker's reads used to compact the whole set once per observed + // hole). + assert_eq!(js_set_value_raw_at(set, 2), 3.0); + assert_eq!( + js_set_value_raw_at(set, 1).to_bits(), + SET_HOLE_VALUE_BITS, + "the raw twin exposes the hole — only the cursor ever reads it" + ); + unsafe { + assert_ne!( + (*set).used, + (*set).size, + "the raw read left the layout alone" + ); + assert_eq!( + crate::set::set_compaction_epoch(set), + 0, + "no squeeze happened" + ); + assert_eq!(crate::set::set_cursor_next_raw(set, 0, 0), Some(0)); + assert_eq!( + crate::set::set_cursor_next_raw(set, 1, 0), + Some(2), + "hole at 1 skipped" + ); + assert_eq!( + crate::set::set_cursor_next_raw(set, 3, 0), + None, + "extent exhausted" + ); + } + // The LIVE-index accessor (#9462 / #9504 — the array-like `set[i]` read) + // squeezes first: live index 1 IS the third value, never a hole, and the + // squeeze is recorded so a cursor past the hole rebases exactly. + assert_eq!( + js_set_value_at(set, 1), + 3.0, + "live index 1 is the third value" + ); + assert_eq!( + js_set_value_at(set, 2).to_bits(), + crate::value::TAG_UNDEFINED, + "past the live size is undefined, not a hole" + ); + unsafe { + assert_eq!( + (*set).used, + (*set).size, + "the live accessor squeezed the hole" + ); + assert_eq!(crate::set::set_compaction_epoch(set), 1, "…and recorded it"); + assert_eq!(crate::set::set_cursor_next_raw(set, 2, 0), Some(1)); + assert_eq!(js_set_value_raw_at(set, 1), 3.0); + } +} + +#[test] +fn cursor_rebases_exactly_across_a_multi_hole_compaction() { + // 40 values; a walk at cursor 21 while the body deletes v0..v20: holes + // outnumber the 19 live values, one compaction squeezes all 21 below the + // cursor. The rebase moves the cursor down by exactly that count. + let set = js_set_alloc(64); + for i in 0..40 { + js_set_add(set, i as f64); + } + let epoch0 = crate::set::set_compaction_epoch(set); + for i in 0..=20 { + assert_eq!(js_set_delete(set, i as f64), 1); + } unsafe { - assert_eq!((*set).used, (*set).size, "access healed the layout"); + assert_eq!((*set).used, (*set).size, "the delete path compacted"); } + assert_ne!(crate::set::set_compaction_epoch(set), epoch0); + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 21, epoch0) }, + Some(0) + ); + assert_eq!(js_set_value_at(set, 0), 21.0, "the true next value"); + let epoch1 = crate::set::set_compaction_epoch(set); + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 3, epoch1) }, + Some(3) + ); +} + +#[test] +fn cursor_rebases_through_successive_squeezes_and_clear() { + let set = js_set_alloc(64); + for i in 0..40 { + js_set_add(set, i as f64); + } + let epoch0 = crate::set::set_compaction_epoch(set); + for i in 0..=31 { + assert_eq!(js_set_delete(set, i as f64), 1); + } + unsafe { + assert_eq!((*set).used, (*set).size); + assert_eq!((*set).size, 8); + } + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 21, epoch0) }, + Some(0) + ); + assert_eq!(js_set_value_at(set, 0), 32.0); + let epoch1 = crate::set::set_compaction_epoch(set); + js_set_clear(set); + js_set_add(set, 100.0); + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 5, epoch1) }, + Some(0) + ); + assert_eq!(js_set_value_at(set, 0), 100.0); } #[test] @@ -227,3 +337,60 @@ fn clear_resets_the_extent_and_walkers_compact() { "the hole must not defeat the subset walk" ); } + +#[test] +fn cursor_stays_exact_across_forty_squeezes_in_one_body() { + // See the Map twin: a set at full capacity squeezes once per delete+re-add + // pair on the grow path; the removed-index budget keeps every record. + let set = js_set_alloc(64); + for i in 0..64 { + js_set_add(set, i as f64); + } + unsafe { + assert_eq!((*set).used, (*set).capacity, "premise: at capacity"); + } + let epoch0 = crate::set::set_compaction_epoch(set); + for i in 0..40 { + assert_eq!(js_set_delete(set, i as f64), 1); + js_set_add(set, i as f64); + } + let squeezes = crate::set::set_compaction_epoch(set).wrapping_sub(epoch0); + assert!( + squeezes >= 33, + "premise: {squeezes} squeezes happened, need > 32" + ); + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 10, epoch0) }, + Some(0) + ); + assert_eq!(js_set_value_raw_at(set, 0), 40.0); + let mut order = Vec::new(); + let mut idx = 0u32; + let epoch_now = crate::set::set_compaction_epoch(set); + while let Some(i) = unsafe { crate::set::set_cursor_next_raw(set, idx, epoch_now) } { + order.push(js_set_value_raw_at(set, i)); + idx = i + 1; + } + let expected: Vec = (40..64).chain(0..40).map(|k| k as f64).collect(); + assert_eq!(order, expected); +} + +#[test] +fn clear_truncates_the_squeeze_history() { + let set = js_set_alloc(64); + for i in 0..64 { + js_set_add(set, i as f64); + } + let epoch0 = crate::set::set_compaction_epoch(set); + for i in 0..40 { + js_set_delete(set, i as f64); + js_set_add(set, i as f64); + } + js_set_clear(set); + js_set_add(set, 7.0); + assert_eq!( + unsafe { crate::set::set_cursor_next_raw(set, 10, epoch0) }, + Some(0) + ); + assert_eq!(js_set_value_raw_at(set, 0), 7.0); +} diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 0c477994ff..7d6768c92a 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -198,6 +198,15 @@ "why": "Holds Set/Map header ADDRESSES as identity keys for in-flight forEach walks (#9082); the values are only ever compared (`contains`, `pop` equality) and truncated, never dereferenced, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc / map_header_moved_for_gc, which rewrite every matching entry when a header moves — see the SET_FOREACH_STACK/MAP_FOREACH_STACK rewrite loops there.", "reason": "Holds Set/Map header ADDRESSES as identity keys for in-flight forEach walks (#9082); the values are only ever compared (`contains`, `pop` equality) and truncated, never dereferenced, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc / map_header_moved_for_gc, which rewrite every matching entry when a header moves — see the SET_FOREACH_STACK/MAP_FOREACH_STACK rewrite loops there." }, + { + "file": "crates/perry-runtime/src/map.rs", + "name": "MAP_COMPACTION_LOG", + "count": 1, + "classification": "not_a_gc_pointer", + "verdict": "not_a_gc_pointer", + "why": "Per-Map compaction log for the epoch-based for-of/iterator cursor rebase: keyed by MapHeader ADDRESS (identity only, never dereferenced), values are VecDeque<{epoch: u32, removed: Prefix(u32) | Indices(Vec)}> — raw entry indices, no heap references, so no collector edge originates here. Correctness across evacuation is maintained by map_header_moved_for_gc, which re-keys the entry when a header moves; js_map_alloc drops any stale entry for a reused address; prune_dead_map_compaction_log_owners is registered in gc/dead_owner.rs (table MAP_COMPACTION_LOG) so a dead Map's history is dropped.", + "reason": "Per-Map compaction log for the epoch-based for-of/iterator cursor rebase: keyed by MapHeader ADDRESS (identity only, never dereferenced), values are VecDeque<{epoch: u32, removed: Prefix(u32) | Indices(Vec)}> — raw entry indices, no heap references, so no collector edge originates here. Correctness across evacuation is maintained by map_header_moved_for_gc, which re-keys the entry when a header moves; js_map_alloc drops any stale entry for a reused address; prune_dead_map_compaction_log_owners is registered in gc/dead_owner.rs (table MAP_COMPACTION_LOG) so a dead Map's history is dropped." + }, { "file": "crates/perry-runtime/src/net.rs", "name": "TCP_SERVERS", @@ -433,6 +442,15 @@ "why": "Holds Set/Map header ADDRESSES as identity keys for in-flight forEach walks (#9082); the values are only ever compared (`contains`, `pop` equality) and truncated, never dereferenced, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc / map_header_moved_for_gc, which rewrite every matching entry when a header moves — see the SET_FOREACH_STACK/MAP_FOREACH_STACK rewrite loops there.", "reason": "Holds Set/Map header ADDRESSES as identity keys for in-flight forEach walks (#9082); the values are only ever compared (`contains`, `pop` equality) and truncated, never dereferenced, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc / map_header_moved_for_gc, which rewrite every matching entry when a header moves — see the SET_FOREACH_STACK/MAP_FOREACH_STACK rewrite loops there." }, + { + "file": "crates/perry-runtime/src/set.rs", + "name": "SET_COMPACTION_LOG", + "count": 1, + "classification": "not_a_gc_pointer", + "verdict": "not_a_gc_pointer", + "why": "Per-Set compaction log for the epoch-based for-of/iterator cursor rebase: keyed by SetHeader ADDRESS (identity only, never dereferenced), values are VecDeque<{epoch: u32, removed: Prefix(u32) | Indices(Vec)}> — raw element indices, no heap references, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc, which re-keys the entry when a header moves; js_set_alloc drops any stale entry for a reused address; prune_dead_set_compaction_log_owners is registered in gc/dead_owner.rs (table SET_COMPACTION_LOG).", + "reason": "Per-Set compaction log for the epoch-based for-of/iterator cursor rebase: keyed by SetHeader ADDRESS (identity only, never dereferenced), values are VecDeque<{epoch: u32, removed: Prefix(u32) | Indices(Vec)}> — raw element indices, no heap references, so no collector edge originates here. Correctness across evacuation is maintained by set_header_moved_for_gc, which re-keys the entry when a header moves; js_set_alloc drops any stale entry for a reused address; prune_dead_set_compaction_log_owners is registered in gc/dead_owner.rs (table SET_COMPACTION_LOG)." + }, { "file": "crates/perry-runtime/src/set.rs", "name": "SET_INDEX", diff --git a/test-files/test_gap_map_set_multi_delete_during_iteration.ts b/test-files/test_gap_map_set_multi_delete_during_iteration.ts new file mode 100644 index 0000000000..3e215ba9af --- /dev/null +++ b/test-files/test_gap_map_set_multi_delete_during_iteration.ts @@ -0,0 +1,113 @@ +// Map/Set `for…of` with deletes, clears and re-adds inside the body. Every +// entry that is neither deleted nor already visited must be visited exactly +// once, in insertion order, and an entry re-added after a delete is visited +// again at the end — ECMA-262 iterates the live [[MapData]] list in place. +// +// Before the epoch-based cursor rebase, the raw-index walk recovered from a +// squeeze by reading `cursor-1`, which assumed ONE hole had been removed: +// deleting several already-visited entries plus the current one in a single +// body skipped entries, and enough of them ended the loop early. The same +// reader compaction also cost O(n) per delete while a loop was open. + +function runMap(label: string, n: number, atKey: string, del: string[]): void { + const m = new Map(); + for (let i = 0; i < n; i++) m.set("k" + i, i); + const seen: string[] = []; + for (const [k] of m) { + seen.push(k); + if (k === atKey) for (const d of del) m.delete(d); + } + console.log(label, seen.length, seen.join(",")); +} + +function runSet(label: string, n: number, atKey: string, del: string[]): void { + const s = new Set(); + for (let i = 0; i < n; i++) s.add("k" + i); + const seen: string[] = []; + for (const k of s) { + seen.push(k); + if (k === atKey) for (const d of del) s.delete(d); + } + console.log(label, seen.length, seen.join(",")); +} + +const first21: string[] = []; +for (let i = 0; i <= 20; i++) first21.push("k" + i); + +runMap("map-1hole", 10, "k3", ["k3"]); +runSet("set-1hole", 10, "k3", ["k3"]); +runMap("map-visited-only", 10, "k3", ["k0", "k1", "k2"]); +runMap("map-3holes", 10, "k3", ["k0", "k1", "k3"]); +runSet("set-3holes", 10, "k3", ["k0", "k1", "k3"]); +runMap("map-ahead", 10, "k3", ["k3", "k4"]); +runMap("map-21holes", 40, "k20", first21); +runSet("set-21holes", 40, "k20", first21); + +// delete + re-add of the current key: it moves to the end and is visited +// there, and nothing in between is skipped. +{ + const m = new Map([["a", 1], ["b", 2], ["c", 3]]); + const seen: string[] = []; + for (const [k, v] of m) { + seen.push(k + v); + if (k === "b" && v === 2) { m.delete("b"); m.set("b", 20); } + } + console.log("map-readd", seen.join(",")); +} + +// clear() mid-walk: the list is emptied in place, so the walk continues with +// whatever is appended afterwards. +{ + const m = new Map([["a", 1], ["b", 2], ["c", 3]]); + const seen: string[] = []; + for (const [k] of m) { + seen.push(k); + if (k === "a") { m.clear(); m.set("z", 9); } + } + console.log("map-clear", seen.join(",")); + const s = new Set(["a", "b", "c"]); + const seen2: string[] = []; + for (const k of s) { + seen2.push(k); + if (k === "a") { s.clear(); s.add("z"); } + } + console.log("set-clear", seen2.join(",")); +} + +// Iterator objects (not the for-of fast path) recover the same way. +{ + const m = new Map(); + for (let i = 0; i < 40; i++) m.set("k" + i, i); + const it = m.keys(); + const seen: string[] = []; + for (let r = it.next(); !r.done; r = it.next()) { + seen.push(r.value); + if (r.value === "k20") for (const d of first21) m.delete(d); + } + console.log("map-iter-21holes", seen.length, seen[seen.length - 1]); + const s = new Set(); + for (let i = 0; i < 40; i++) s.add("k" + i); + const si = s.values(); + const seen2: string[] = []; + for (let r = si.next(); !r.done; r = si.next()) { + seen2.push(r.value); + if (r.value === "k20") for (const d of first21) s.delete(d); + } + console.log("set-iter-21holes", seen2.length, seen2[seen2.length - 1]); +} + +// Mutation during iteration must stay linear: 50k entries, 12.5k +// delete+re-add pairs inside the walk. Correctness is checked by the sums; +// the timing gate lives in the benchmark suite. +{ + const m = new Map(); + for (let i = 0; i < 50_000; i++) m.set("key_" + i, i); + let rounds = 0; + for (const [k, v] of m) { + if ((v & 3) === 0 && v < 50_000) { m.delete(k); m.set(k, v + 1); } + rounds++; + } + let sum = 0; + for (const v of m.values()) sum = (sum + v) % 1_000_000_007; + console.log("map-churn", m.size, rounds, sum); +}