From 2f5878df39d974fa79696154df06ec597b24b9cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 11:53:12 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(runtime,hir,codegen):=20Map/Set=20mutat?= =?UTF-8?q?ion=20during=20for=E2=80=A6of=20is=20O(1)=20per=20step=20and=20?= =?UTF-8?q?never=20skips=20an=20entry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #9020 made ordered Map/Set deletes O(1) by tombstoning entries in place and moved the squeeze that used to run 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 observed 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 (quadratic in n), while the identical deletes with no iterator open cost 0.5 s. It was also a correctness bug. Every raw-index cursor (the fast path and 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 assumes exactly ONE hole was squeezed. Deleting several already visited entries plus the current one in a single body skipped entries, and enough holes ended the loop early: 40 entries, k0..k20 deleted at k20, perry visited 21, node 40. Map and Set, both walkers. Fixed the way V8 transitions ordered-hash-table iterators. Every squeeze (`compact_*`, and `clear` while a walk may be open) records the raw indices it removed in a per-collection log and bumps a `compaction_epoch` in the header (offset 36 for MapHeader, 28 for SetHeader — both in existing padding, so no other offset or the codegen-pinned `used` moves). A cursor carries the epoch it last synchronised with; one 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 entries below the cursor), needs no key lookup and no "iteration active" registration that a break, return or abandoned generator could leak, and the readers no longer compact at all. The codegen inline entry read is bounded by the raw extent instead of requiring a dense buffer; the iterator objects keep the epoch in their former size-sentinel field 3. `clear()` inside a walk is recorded as a prefix squeeze, so the cursor restarts at 0 and sees later appends, as the spec's in-place emptying requires. Log depth is bounded at 32 records per collection. Quiet-host numbers (node 26.5.1), warm: delete+re-add during for…of over 50k entries 13.38 s -> 0.02 s (mixed keys), 32.36 s -> 0.02 s (string keys); Set 8.14 s -> 0.02 s; the bench_map_set_tombstone_churn row from 152x node to ~5x faster than node. Every during-iteration variant now costs the same as its no-iterator control. Tests: runtime unit tests for the no-compaction read contract, the exact multi-hole rebase, successive squeezes + clear, and address reuse (Map and Set); the one test that pinned the old self-heal is rewritten to pin the new contract; `test_gap_map_set_multi_delete_during_iteration.ts` covers the fast path, the iterator objects, re-add, clear and a 50k churn, node-differential. Validation: perry-runtime 2979 passed / 0 failed (single-threaded, release); perry-hir and perry-codegen suites green; clippy --workspace 0 warnings; scripts/run_lint_gates.sh 60/60 (the two new perry_thread_local! holders are classified in gc_runtime_root_holders.json); force-evacuate / verify-evacuation / seeded schedule / from-space protect / VERIFY_MARK arms over the new fixture all match node. --- .../0000-map-set-iteration-compaction.md | 43 +++ crates/perry-codegen/src/expr/arrays_finds.rs | 23 +- .../src/runtime_decls/strings.rs | 7 + crates/perry-hir/src/lower/for_head.rs | 201 ++++--------- .../src/collection_iter_object.rs | 114 +++----- crates/perry-runtime/src/gc/dead_owner.rs | 12 + crates/perry-runtime/src/map.rs | 274 ++++++++++++++++-- .../perry-runtime/src/map_tombstone_tests.rs | 127 +++++++- crates/perry-runtime/src/set.rs | 193 +++++++++++- .../perry-runtime/src/set_tombstone_tests.rs | 83 +++++- scripts/gc_runtime_root_holders.json | 18 ++ ...p_map_set_multi_delete_during_iteration.ts | 113 ++++++++ 12 files changed, 925 insertions(+), 283 deletions(-) create mode 100644 changelog.d/0000-map-set-iteration-compaction.md create mode 100644 test-files/test_gap_map_set_multi_delete_during_iteration.ts diff --git a/changelog.d/0000-map-set-iteration-compaction.md b/changelog.d/0000-map-set-iteration-compaction.md new file mode 100644 index 0000000000..b1c796a775 --- /dev/null +++ b/changelog.d/0000-map-set-iteration-compaction.md @@ -0,0 +1,43 @@ +**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 readers no +longer compact at all. The codegen inline entry read is bounded by the raw +extent instead of requiring a dense buffer. The iterator objects use the same +rebase (their field 3 now holds the epoch). Log depth is bounded at 32 records +per collection; a cursor older than that is stepped over holes without rebase +(reaching it needs one loop body to force 32 squeezes of the same collection). + +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 the no-compaction read contract, the exact +multi-hole rebase, successive squeezes + `clear`, and address reuse (Map and +Set); `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..a4ae34b5fa 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; diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 48d75e0462..3eaa72cdca 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1239,6 +1239,13 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { // 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..fb5464d8b1 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -51,6 +51,120 @@ 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, +} + +/// Records retained per Map. A cursor older than the oldest retained record +/// can no longer rebase exactly (`rebase_map_cursor` applies what is left); +/// reaching that depth needs ONE loop body to drive 32 compactions of the +/// same Map between two of its own reads, each of which requires the holes +/// to outnumber the live entries again. +const MAP_COMPACTION_LOG_DEPTH: usize = 32; + +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; + MAP_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + let records = log.entry(map as usize).or_default(); + if records.len() == MAP_COMPACTION_LOG_DEPTH { + records.pop_front(); + } + records.push_back(CompactionRecord { epoch, removed }); + }); +} + +/// 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(records) = log.get(&(map as usize)) else { + return cursor; + }; + let mut c = cursor; + for rec in 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 +204,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 +956,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 +1144,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 +1273,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 +1291,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 +1541,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 +1613,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 +1683,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 +1737,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 +2765,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 +2810,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,15 +2852,12 @@ 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 { + // A raw read, bounded by the raw extent. The index comes from + // `js_map_cursor_next`, which only ever yields a LIVE raw index, so + // no hole reaches user code — and nothing here compacts: the + // self-heal compaction this used to perform ran once per hole + // observed, which made a delete-during-`for…of` loop O(n) per delete. + if idx >= (*map).used { return f64::from_bits(TAG_UNDEFINED); } let entries = entries_ptr(map); @@ -2667,15 +2873,9 @@ 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 { + // See `js_map_entry_key_at`: raw, bounded by the extent, never + // compacts. + if idx >= (*map).used { return f64::from_bits(TAG_UNDEFINED); } let entries = entries_ptr(map); @@ -3628,11 +3828,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_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 +3851,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..74fa98985f 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,12 +181,124 @@ 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-indexed extern is a plain bounded read: raw index 2 is still + // the THIRD key, the hole at 1 stays, and the layout is untouched — this + // used to compact the whole map on every such read, once per hole. + assert_eq!(js_map_entry_key_at(map, 2), 3.0); + assert_eq!(js_map_entry_value_at(map, 2), 3.0); unsafe { - assert_eq!((*map).used, (*map).size, "access healed the layout"); + assert_ne!((*map).used, (*map).size, "the 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" + ); + } +} + +#[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] diff --git a/crates/perry-runtime/src/set.rs b/crates/perry-runtime/src/set.rs index 3445ef7e94..21b69e946a 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,155 @@ 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, +} + +/// Records retained per Set — see `map::MAP_COMPACTION_LOG_DEPTH`. +const SET_COMPACTION_LOG_DEPTH: usize = 32; + +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; + SET_COMPACTION_LOG.with(|log| { + let mut log = log.borrow_mut(); + let records = log.entry(set as usize).or_default(); + if records.len() == SET_COMPACTION_LOG_DEPTH { + records.pop_front(); + } + records.push_back(SetCompactionRecord { epoch, removed }); + }); +} + +/// 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(records) = log.get(&(set as usize)) else { + return cursor; + }; + let mut c = cursor; + for rec in 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 +1059,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 +1197,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 +1771,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 +1800,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,12 +1828,12 @@ 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); - } - if i >= (*set).size { + // A raw read, bounded by the raw extent. The index comes from + // `js_set_cursor_next`, which only yields LIVE raw indices, so no + // hole reaches user code — and nothing here compacts (the self-heal + // compaction this used to perform ran once per hole observed, which + // made a delete-during-`for…of` loop O(n) per delete). + if i >= (*set).used { return f64::from_bits(UNDEF); } let elements = (*set).elements as *const f64; @@ -2887,6 +3063,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..a616293e65 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,87 @@ 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"); + // A plain bounded raw read: raw index 2 is still the THIRD value, the + // hole at 1 stays, the layout is untouched (this used to compact the + // whole set on every such read, once per hole). + assert_eq!(js_set_value_at(set, 2), 3.0); unsafe { - assert_eq!((*set).used, (*set).size, "access healed the layout"); + assert_ne!((*set).used, (*set).size, "the 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" + ); + } +} + +#[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, "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] 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); +} From 63b29eb5a3ea1e125ec95d74f08629e4e82a95df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 12:10:47 +0200 Subject: [PATCH 2/3] changelog: key the fragment on #9513 --- ...eration-compaction.md => 9513-map-set-iteration-compaction.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{0000-map-set-iteration-compaction.md => 9513-map-set-iteration-compaction.md} (100%) diff --git a/changelog.d/0000-map-set-iteration-compaction.md b/changelog.d/9513-map-set-iteration-compaction.md similarity index 100% rename from changelog.d/0000-map-set-iteration-compaction.md rename to changelog.d/9513-map-set-iteration-compaction.md From 9b399829140e0b972468678d5e490d55af2094b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 17:13:27 +0200 Subject: [PATCH 3/3] review: keep #9504's live-index readers, give the walkers raw twins, budget the squeeze history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto main. #9504 (merged after this branch's base) made the array-like `set[i]` / `map[i]` read go through `js_set_value_at` / `js_map_entry_key_at` and rely on their compaction to make raw index == live index — the exact compaction the first revision removed for the walkers, so `a_tombstoned_collection_never_hands_a_hole_to_an_indexed_read` failed on the merge. Two contracts wanted one function; they are now two: * `js_map_entry_key_at` / `js_map_entry_value_at` / `js_set_value_at` stay the LIVE-index accessors #9504 named them: squeeze first, never hand out a hole, and the squeeze is now recorded in the compaction log, so a for…of cursor open on the same collection rebases exactly instead of skipping. * the walkers read through 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, never compacting; the codegen MapEntryKeyAt / MapEntryValueAt / SetValueAt fallbacks name them. Review also caught that the 32-record history cap was the wrong shape: the grow-path squeeze (`ensure_capacity` at used == capacity with a hole) fires once per delete+re-add pair on a full collection, so one loop body reaches 33 records trivially. History is now budgeted by retained removed-index count, `max(4096, capacity)` per collection, a `clear()` record truncates everything before it, and a forty-squeezes-in-one-body test (Map and Set, at full capacity) pins exactness across the window. The version bump is dropped per the contributor rule. Validation on the rebased tree: perry-runtime 3012 passed / 0 failed (release, single-threaded) including #9504's test; Node-differential match on the new fixture, the existing Map/Set iteration fixture, and #9504's hole-leak-family fixtures; cargo clippy --workspace 0 warnings; cargo fmt --all --check clean. --- .../9513-map-set-iteration-compaction.md | 37 +++-- crates/perry-codegen/src/expr/arrays_finds.rs | 12 +- .../src/runtime_decls/strings.rs | 5 + crates/perry-runtime/src/map.rs | 129 +++++++++++++++--- .../perry-runtime/src/map_tombstone_tests.rs | 126 ++++++++++++++++- crates/perry-runtime/src/set.rs | 98 +++++++++++-- .../perry-runtime/src/set_tombstone_tests.rs | 100 +++++++++++++- 7 files changed, 449 insertions(+), 58 deletions(-) diff --git a/changelog.d/9513-map-set-iteration-compaction.md b/changelog.d/9513-map-set-iteration-compaction.md index b1c796a775..5020ce5f83 100644 --- a/changelog.d/9513-map-set-iteration-compaction.md +++ b/changelog.d/9513-map-set-iteration-compaction.md @@ -25,19 +25,38 @@ 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 readers no -longer compact at all. The codegen inline entry read is bounded by the raw -extent instead of requiring a dense buffer. The iterator objects use the same -rebase (their field 3 now holds the epoch). Log depth is bounded at 32 records -per collection; a cursor older than that is stepped over holes without rebase -(reaching it needs one loop body to force 32 squeezes of the same collection). +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 the no-compaction read contract, the exact -multi-hole rebase, successive squeezes + `clear`, and address reuse (Map and -Set); `test_gap_map_set_multi_delete_during_iteration.ts` covers the fast path, +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 a4ae34b5fa..c4ea2bfbbd 100644 --- a/crates/perry-codegen/src/expr/arrays_finds.rs +++ b/crates/perry-codegen/src/expr/arrays_finds.rs @@ -594,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| { @@ -618,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 3eaa72cdca..b101f9cbf9 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -1235,6 +1235,11 @@ 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]); diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index fb5464d8b1..9b6ba1a560 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -76,19 +76,43 @@ struct CompactionRecord { removed: RemovedSlots, } -/// Records retained per Map. A cursor older than the oldest retained record -/// can no longer rebase exactly (`rebase_map_cursor` applies what is left); -/// reaching that depth needs ONE loop body to drive 32 compactions of the -/// same Map between two of its own reads, each of which requires the holes -/// to outnumber the live entries again. -const MAP_COMPACTION_LOG_DEPTH: usize = 32; +/// 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>, + crate::fast_hash::PtrHashMap, > = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } @@ -98,13 +122,26 @@ crate::perry_thread_local! { 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 records = log.entry(map as usize).or_default(); - if records.len() == MAP_COMPACTION_LOG_DEPTH { - records.pop_front(); + 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(); + } } - records.push_back(CompactionRecord { epoch, removed }); }); } @@ -123,11 +160,12 @@ unsafe fn rebase_map_cursor(map: *const MapHeader, cursor: u32, loop_epoch: u32) } MAP_COMPACTION_LOG.with(|log| { let log = log.borrow(); - let Some(records) = log.get(&(map as usize)) else { + let Some(entry) = log.get(&(map as usize)) else { return cursor; }; let mut c = cursor; - for rec in records + for rec in entry + .records .iter() .filter(|r| r.epoch.wrapping_sub(loop_epoch) as i32 > 0) { @@ -2852,11 +2890,35 @@ pub extern "C" fn js_map_entry_key_at(map: *const MapHeader, idx: u32) -> f64 { return f64::from_bits(TAG_UNDEFINED); } unsafe { - // A raw read, bounded by the raw extent. The index comes from - // `js_map_cursor_next`, which only ever yields a LIVE raw index, so - // no hole reaches user code — and nothing here compacts: the - // self-heal compaction this used to perform ran once per hole - // observed, which made a delete-during-`for…of` loop O(n) per delete. + // 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 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); } @@ -2864,6 +2926,10 @@ pub extern "C" fn js_map_entry_key_at(map: *const MapHeader, idx: u32) -> f64 { 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] @@ -2873,8 +2939,25 @@ pub extern "C" fn js_map_entry_value_at(map: *const MapHeader, idx: u32) -> f64 return f64::from_bits(TAG_UNDEFINED); } unsafe { - // See `js_map_entry_key_at`: raw, bounded by the extent, never - // compacts. + // 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 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); } @@ -2882,6 +2965,10 @@ pub extern "C" fn js_map_entry_value_at(map: *const MapHeader, idx: u32) -> f64 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] @@ -3836,7 +3923,7 @@ mod tests { 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_at(map, i).to_bits()); + keys.push(js_map_entry_key_raw_at(map, i).to_bits()); idx = i + 1; } keys diff --git a/crates/perry-runtime/src/map_tombstone_tests.rs b/crates/perry-runtime/src/map_tombstone_tests.rs index 74fa98985f..3f95b3d546 100644 --- a/crates/perry-runtime/src/map_tombstone_tests.rs +++ b/crates/perry-runtime/src/map_tombstone_tests.rs @@ -181,13 +181,23 @@ fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { unsafe { assert_ne!((*map).used, (*map).size, "a hole is present"); } - // The raw-indexed extern is a plain bounded read: raw index 2 is still - // the THIRD key, the hole at 1 stays, and the layout is untouched — this - // used to compact the whole map on every such read, once per hole. - assert_eq!(js_map_entry_key_at(map, 2), 3.0); - assert_eq!(js_map_entry_value_at(map, 2), 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_ne!((*map).used, (*map).size, "the read left the layout alone"); + assert_ne!( + (*map).used, + (*map).size, + "the raw read left the layout alone" + ); assert_eq!( crate::map::map_compaction_epoch(map), 0, @@ -206,6 +216,35 @@ fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { "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] @@ -371,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 21b69e946a..6595d0e20c 100644 --- a/crates/perry-runtime/src/set.rs +++ b/crates/perry-runtime/src/set.rs @@ -917,15 +917,43 @@ struct SetCompactionRecord { removed: SetRemovedSlots, } -/// Records retained per Set — see `map::MAP_COMPACTION_LOG_DEPTH`. -const SET_COMPACTION_LOG_DEPTH: usize = 32; +/// 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>, + crate::fast_hash::PtrHashMap, > = RefCell::new(crate::fast_hash::new_ptr_hash_map()); } @@ -934,13 +962,28 @@ crate::perry_thread_local! { 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 records = log.entry(set as usize).or_default(); - if records.len() == SET_COMPACTION_LOG_DEPTH { - records.pop_front(); + 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(); + } } - records.push_back(SetCompactionRecord { epoch, removed }); }); } @@ -954,11 +997,12 @@ unsafe fn rebase_set_cursor(set: *const SetHeader, cursor: u32, loop_epoch: u32) } SET_COMPACTION_LOG.with(|log| { let log = log.borrow(); - let Some(records) = log.get(&(set as usize)) else { + let Some(entry) = log.get(&(set as usize)) else { return cursor; }; let mut c = cursor; - for rec in records + for rec in entry + .records .iter() .filter(|r| r.epoch.wrapping_sub(loop_epoch) as i32 > 0) { @@ -1828,11 +1872,34 @@ pub extern "C" fn js_set_value_at(set: *const SetHeader, i: u32) -> f64 { return f64::from_bits(UNDEF); } unsafe { - // A raw read, bounded by the raw extent. The index comes from - // `js_set_cursor_next`, which only yields LIVE raw indices, so no - // hole reaches user code — and nothing here compacts (the self-heal - // compaction this used to perform ran once per hole observed, which - // made a delete-during-`for…of` loop O(n) per delete). + // 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); + } + let elements = (*set).elements as *const f64; + ptr::read(elements.add(i as usize)) + } +} + +/// 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); } @@ -1840,6 +1907,9 @@ pub extern "C" fn js_set_value_at(set: *const SetHeader, i: u32) -> 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. diff --git a/crates/perry-runtime/src/set_tombstone_tests.rs b/crates/perry-runtime/src/set_tombstone_tests.rs index a616293e65..f0ebd355f8 100644 --- a/crates/perry-runtime/src/set_tombstone_tests.rs +++ b/crates/perry-runtime/src/set_tombstone_tests.rs @@ -151,12 +151,22 @@ fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { unsafe { assert_ne!((*set).used, (*set).size, "a hole is present"); } - // A plain bounded raw read: raw index 2 is still the THIRD value, the - // hole at 1 stays, the layout is untouched (this used to compact the - // whole set on every such read, once per hole). - assert_eq!(js_set_value_at(set, 2), 3.0); + // 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 read left the layout alone"); + assert_ne!( + (*set).used, + (*set).size, + "the raw read left the layout alone" + ); assert_eq!( crate::set::set_compaction_epoch(set), 0, @@ -174,6 +184,29 @@ fn raw_indexed_reads_never_compact_and_the_cursor_steps_over_holes() { "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] @@ -304,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); +}