Merge train: #9513 (Map/Set O(1) iteration compaction)

changelog.d/9513-map-set-iteration-compaction.md

@@ -0,0 +1,62 @@
+**Map/Set: mutation during `for…of` is O(1) per step again, and never skips an entry.**
+
+`#9020` made ordered `delete` O(1) by tombstoning entries in place, and moved
+the squeeze that used to happen per delete into the raw-index *readers*
+(`js_map_entry_key_at` / `js_set_value_at`) as a "self-heal": the first raw
+read that saw a hole compacted the whole collection. The `for…of` fast path
+reads raw slots on every step, so a loop that deletes while iterating paid one
+full compaction per delete — 50,000 entries with 12,500 deletes inside the walk
+took 13–32 s against Node's 0.07 s (190–460×), quadratic in the collection
+size; the identical deletes with no iterator open cost 0.5 s.
+
+Worse, every raw-index cursor (the fast path, the iterator objects) recovered
+from a squeeze by re-finding the last returned key and, when that key was
+itself deleted, reading `cursor-1` — which assumed exactly ONE hole had been
+squeezed. Deleting several already-visited entries plus the current one in a
+single loop body skipped entries, and enough holes ended the loop early
+(40 entries, 21 deleted at the 21st: Perry visited 21, Node 40).
+
+Fixed the way V8 transitions its ordered-hash-table iterators: every squeeze
+(`compact_*`, and `clear` while a walk may be open) records which raw indices
+it removed, in a per-collection log, and bumps a `compaction_epoch` in the
+header. A cursor carries the epoch it last synchronised with; one runtime call
+per step (`js_map_cursor_next` / `js_set_cursor_next`) rebases it — down by
+exactly the removed count below it, in order, through every record since —
+then steps over tombstones and returns the next live raw index. This is exact
+by the walk's own invariant (the yielded entries are precisely the live ones
+below the cursor), needs no key lookup, no "iteration active" registration
+that a `break`, `return` or abandoned generator could leak, and the walkers'
+reads no longer compact at all. The codegen inline entry read is bounded by
+the raw extent instead of requiring a dense buffer.
+
+Two reader contracts, kept apart. `js_map_entry_key_at` / `js_map_entry_value_at`
+/ `js_set_value_at` remain the LIVE-index accessors #9504 made the array-like
+`map[i]` / `set[i]` read, `console.table` and collection equality go through:
+they squeeze tombstones first so raw index == live index, never hand out a
+hole, and now record that squeeze — so a `for…of` cursor open on the same
+collection rebases exactly instead of skipping. The walkers use new RAW twins
+(`js_map_entry_key_raw_at`, `js_map_entry_value_raw_at`, `js_set_value_raw_at`),
+bounded by the raw extent, which never compact; only the cursor ever reads
+them, and it only yields live raw indices. The iterator objects use the same
+rebase (their field 3 now holds the epoch). History is retained under a budget
+of removed raw indices — `max(4096, capacity)` per collection — never a record
+count, because a delete+re-add pair on a collection at full capacity squeezes
+one hole per pair on the grow path and a single loop body can force dozens of
+those. A `clear()` record supersedes everything before it and truncates it.
+Exceeding the budget therefore takes one loop body deleting more than the
+collection's whole capacity between two of its own reads; a cursor trimmed out
+of its history is stepped over holes without the lost rebase, and the bound is
+pinned by a forty-squeezes-in-one-body test for Map and Set.
+
+Also correct now: `clear()` inside a walk restarts the cursor at 0, so entries
+added afterwards are visited, as the spec's in-place emptying of `[[MapData]]`
+requires.
+
+Tests: runtime unit tests for both reader contracts (raw twins never compact;
+live-index accessors squeeze, record, and a cursor past the hole rebases), the
+exact multi-hole rebase, successive squeezes + `clear`, forty squeezes in one
+body at full capacity, `clear` truncating the history, and address reuse (Map
+and Set); #9504's `a_tombstoned_collection_never_hands_a_hole_to_an_indexed_read`
+stays green; `test_gap_map_set_multi_delete_during_iteration.ts` covers the fast path,
+the iterator objects, re-add, `clear`, and a 50k-entry churn, all
+Node-differential.

crates/perry-codegen/src/expr/arrays_finds.rs

@@ -91,8 +91,8 @@ use super::index_get::numeric_index_has_integer_array_index_proof;
 /// other shape — a subclass instance, a plain object, an out-of-range or
 /// negative index, an unpublished handle — takes the runtime helper exactly
 /// as before, so the two paths are equivalent by construction.
-/// Byte offset of `MapHeader::used`, which the tombstoned-delete lane loads to
-/// check `used == size` (no holes) before admitting a raw entry read.
+/// Byte offset of `MapHeader::used`, the raw extent the inline entry read is
+/// bounded by (the for-of cursor only ever yields a live raw index below it).
 ///
 /// `perry-codegen` does not depend on `perry-runtime`, so nothing binds this
 /// literal to the struct it describes. The runtime pins the offset with an
@@ -156,20 +156,19 @@ fn lower_map_entry_at_inline(
         let gc_flags = blk.load(I8, &flags_ptr);
         let forwarded = blk.and(I8, &gc_flags, GC_FLAG_FORWARDED);
         let live = blk.icmp_eq(I8, &forwarded, "0");
-        let size_ptr = blk.inttoptr(I64, &m_handle);
-        let size = blk.load(I32, &size_ptr);
-        // Tombstoned deletes leave `used > size`; a raw entry read is only
-        // dense-correct with no holes present, so a holey map falls back to
-        // the runtime helper — which compacts, after which this admission
-        // holds again (the lane self-heals).
+        // The index is a LIVE raw index by construction: it comes from
+        // `js_map_cursor_next`, and the for-of desugars in perry-hir are the
+        // only producers of these reads. So the read is bounded by the raw
+        // extent `used`, not the live `size`, and holes need no admission
+        // check — the cursor already stepped over them. (This lane used to
+        // require `used == size` and fall back to a runtime helper that
+        // compacted the whole map on every holey read.)
         let used_addr = blk.add(I64, &m_handle, MAP_HEADER_USED_OFFSET);
         let used_ptr = blk.inttoptr(I64, &used_addr);
         let used = blk.load(I32, &used_ptr);
-        let dense = blk.icmp_eq(I32, &used, &size);
-        let in_range = blk.icmp_ult(I32, &i_i32, &size);
+        let in_range = blk.icmp_ult(I32, &i_i32, &used);
         let a = blk.and(I1, &is_map, &live);
-        let b = blk.and(I1, &a, &dense);
-        let admitted = blk.and(I1, &b, &in_range);
+        let admitted = blk.and(I1, &a, &in_range);
         blk.cond_br(&admitted, &fast_label, &slow_label);
     }
     ctx.current_block = fast_idx;
@@ -595,8 +594,14 @@ pub(crate) fn lower(
         // calling `js_map_entries` (which materializes N+1 small Arrays).
         Expr::MapEntryKeyAt { map, idx } | Expr::MapEntryValueAt { map, idx } => {
             let (runtime_fn, value_slot) = match expr {
-                Expr::MapEntryKeyAt { .. } => ("js_map_entry_key_at", false),
-                Expr::MapEntryValueAt { .. } => ("js_map_entry_value_at", true),
+                // The RAW twins: these nodes are produced only by the for-of
+                // desugars, whose cursor yields live raw indices, so the
+                // fallback must read raw slots bounded by `used` and never
+                // compact. (`js_map_entry_key_at` without `_raw` is the
+                // live-index accessor the array-like `map[i]` read uses,
+                // which compacts — #9504.)
+                Expr::MapEntryKeyAt { .. } => ("js_map_entry_key_raw_at", false),
+                Expr::MapEntryValueAt { .. } => ("js_map_entry_value_raw_at", true),
                 _ => unreachable!(),
             };
             rooting::with_operands_rooted(ctx, &[map, idx], |ctx, vals| {
@@ -619,7 +624,7 @@ pub(crate) fn lower(
                 let i_i32 = blk.fptosi(DOUBLE, &i_dbl, I32);
                 Ok(blk.call(
                     DOUBLE,
-                    "js_set_value_at",
+                    "js_set_value_raw_at",
                     &[(I64, &s_handle), (I32, &i_i32)],
                 ))
             })

crates/perry-codegen/src/expr/map_entry_at_tests.rs

@@ -65,13 +65,13 @@ fn map_entry_reads_are_inline_with_the_helper_as_the_fallback() {
             && key_ir.contains("map_entry_key.fast")
             && key_ir.contains("icmp eq i8")
             && key_ir.contains("load double")
-            && key_ir.contains("call double @js_map_entry_key_at("),
+            && key_ir.contains("call double @js_map_entry_key_raw_at("),
         "the key read should be inline with the helper as fallback:\n{key_ir}"
     );
     let value_ir = entry_read_ir(true);
     assert!(
         value_ir.contains("map_entry_value.fast")
-            && value_ir.contains("call double @js_map_entry_value_at("),
+            && value_ir.contains("call double @js_map_entry_value_raw_at("),
         "the value read should be inline with the helper as fallback:\n{value_ir}"
     );
     // The value slot is the second word of the 16-byte entry.

crates/perry-codegen/src/runtime_decls/strings.rs

@@ -1235,10 +1235,22 @@ pub fn declare_phase_b_strings(module: &mut LlModule) {
     // would do. (Map ptr, entry idx) → key / value.
     module.declare_function("js_map_entry_key_at", DOUBLE, &[I64, I32]);
     module.declare_function("js_map_entry_value_at", DOUBLE, &[I64, I32]);
+    // RAW twins for the for-of walkers: bounded by the raw extent, never
+    // compact (the un-suffixed accessors above are live-index and compact).
+    module.declare_function("js_map_entry_key_raw_at", DOUBLE, &[I64, I32]);
+    module.declare_function("js_map_entry_value_raw_at", DOUBLE, &[I64, I32]);
+    module.declare_function("js_set_value_raw_at", DOUBLE, &[I64, I32]);
     // #6075: current index of a key/value (or -1) for the delete-safe for-of
     // fast path. Takes the NaN-boxed collection + key; strips internally.
     module.declare_function("js_map_find_key_index", DOUBLE, &[DOUBLE, DOUBLE]);
     module.declare_function("js_set_find_value_index", DOUBLE, &[DOUBLE, DOUBLE]);
+    // The delete-safe for-of cursor: (NaN-boxed collection, cursor, epoch) →
+    // next live raw index or -1, and the collection's compaction epoch the
+    // loop stores after each step. See perry-hir `map_set_delete_safe_for_of`.
+    module.declare_function("js_map_cursor_next", DOUBLE, &[DOUBLE, DOUBLE, DOUBLE]);
+    module.declare_function("js_map_compaction_epoch", DOUBLE, &[DOUBLE]);
+    module.declare_function("js_set_cursor_next", DOUBLE, &[DOUBLE, DOUBLE, DOUBLE]);
+    module.declare_function("js_set_compaction_epoch", DOUBLE, &[DOUBLE]);
     // Map/Set forEach: (collection_ptr, callback_nanboxed_f64, thisArg_f64) -> void (#2830)
     module.declare_function("js_map_foreach", VOID, &[I64, DOUBLE, DOUBLE]);
     module.declare_function("js_set_foreach", VOID, &[I64, DOUBLE, DOUBLE]);