From 66c2d6dcce2efaf050ea89173d1ff74c180ea800 Mon Sep 17 00:00:00 2001 From: pilaoda <793493083@qq.com> Date: Tue, 16 Jun 2026 05:56:45 +0800 Subject: [PATCH] fix(lualib): rewrite Map/Set with flat array storage for safe iteration during deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace linked list (nextKey/previousKey) with flat arrays + V8-style version chain compaction. Deleted entries become tombstones (nil slots); iterators skip them via idx++. Compaction creates new arrays and links old → new; iterators transition lazily by adjusting their index. Changes: - Map/Set use keyIndex (hash), orderedKeys/orderedValues (arrays) - delete() marks tombstone, triggers compact when deletedCount > size - compact() records hole positions, links old arrays to new via index 0 - Iterator transitions through version chain (V8 Transition algorithm) - has() uses keyIndex (works correctly with null/undefined values) Based on V8's OrderedHashTable: https://chromium.googlesource.com/v8/v8/+/main/src/objects/ordered-hash-table.cc Tests: 166 Map/Set tests including V8 stress tests, memory leak tests, delete-during-iteration, re-add, and consecutive delete scenarios. Adapted from V8's collection-iterator.js: https://chromium.googlesource.com/v8/v8/+/main/test/mjsunit/es6/collection-iterator.js --- src/lualib/Map.ts | 248 ++++++++++++++++++++++----------- src/lualib/Set.ts | 193 +++++++++++++++---------- test/unit/builtins/map.spec.ts | 240 +++++++++++++++++++++++++++++++ test/unit/builtins/set.spec.ts | 220 +++++++++++++++++++++++++++++ 4 files changed, 746 insertions(+), 155 deletions(-) diff --git a/src/lualib/Map.ts b/src/lualib/Map.ts index c2bcb43a4..1ff0ad474 100644 --- a/src/lualib/Map.ts +++ b/src/lualib/Map.ts @@ -1,15 +1,28 @@ +// Insertion-ordered Map using flat arrays with tombstone deletion and +// V8-style version chain compaction. Deleted entries become nil (tombstones); +// iterators skip them via idx++. Compaction creates new arrays and links +// old → new at index 0; iterators transition lazily by adjusting their +// index (subtracting holes before current position). +// +// Using integer-indexed arrays (orderedKeys/orderedValues) for the insertion +// order leverages Lua's array part: sequential integer keys are stored in +// contiguous memory, making iteration a simple pointer offset (~2-3 cycles) +// rather than a hash table lookup (~15-50 cycles per step). +// +// Based on V8's OrderedHashTable (ordered-hash-table.cc: Rehash, Transition): +// https://chromium.googlesource.com/v8/v8/+/main/src/objects/ordered-hash-table.cc export class Map { public static [Symbol.species] = Map; public [Symbol.toStringTag] = "Map"; - private items = new LuaTable(); public size = 0; - // Key-order variables - private firstKey: K | undefined; - private lastKey: K | undefined; - private nextKey = new LuaTable(); - private previousKey = new LuaTable(); + // Flat array storage (1-based indices for Lua) + private keyIndex = new LuaTable(); + private orderedKeys = new LuaTable(); + private orderedValues = new LuaTable(); + private nextSlot = 1; + private deletedCount = 0; constructor(entries?: Iterable | Array) { if (entries === undefined) return; @@ -36,75 +49,103 @@ export class Map { } public clear(): void { - this.items = new LuaTable(); - this.nextKey = new LuaTable(); - this.previousKey = new LuaTable(); - this.firstKey = undefined; - this.lastKey = undefined; + this.keyIndex = new LuaTable(); + this.orderedKeys = new LuaTable(); + this.orderedValues = new LuaTable(); + this.nextSlot = 1; this.size = 0; + this.deletedCount = 0; } public delete(key: K): boolean { - const contains = this.has(key); - if (contains) { - this.size--; - - // Do order bookkeeping - const next = this.nextKey.get(key); - const previous = this.previousKey.get(key); - if (next !== undefined && previous !== undefined) { - this.nextKey.set(previous, next); - this.previousKey.set(next, previous); - } else if (next !== undefined) { - this.firstKey = next; - this.previousKey.set(next, undefined!); - } else if (previous !== undefined) { - this.lastKey = previous; - this.nextKey.set(previous, undefined!); + const idx = this.keyIndex.get(key); + if (idx === undefined) return false; + this.size--; + this.deletedCount++; + this.keyIndex.delete(key); + this.orderedKeys.delete(idx); + this.orderedValues.delete(idx); + + if (this.deletedCount > this.size) { + this.compact(); + } + return true; + } + + // Compaction is needed because nextSlot grows monotonically — deleted + // entries leave nil tombstones but Lua tables don't shrink on nil + // assignment. Lua only resizes a table during rehash, and rehash only + // triggers when a hash-part insert exhausts free nodes (ltable.c: + // luaH_newkey → getfreepos → rehash). Since orderedKeys uses sequential + // integer indices (array part), normal inserts rarely touch the hash + // part, so rehash won't trigger naturally to reclaim the space. + // + // V8-style: copy live entries to new arrays, record hole positions in + // the old array at negative indices, link old[0] → new. Active iterators + // hold old arrays and transition lazily on next() by adjusting their + // index (subtracting holes before current position). + // + // Index 0 and negative indices go to Lua's hash part (not array part), + // so they don't conflict with the 1-based data entries. + private compact(): void { + const oldKeys = this.orderedKeys; + const oldValues = this.orderedValues; + const oldNextSlot = this.nextSlot; + const newKeys = new LuaTable(); + const newValues = new LuaTable(); + let newSlot = 1; + let holeCount = 0; + + for (let i = 1; i < oldNextSlot; i++) { + const k = oldKeys.get(i); + if (k !== undefined) { + newKeys.set(newSlot, k); + newValues.set(newSlot, oldValues.get(i)); + this.keyIndex.set(k, newSlot); + newSlot++; } else { - this.firstKey = undefined; - this.lastKey = undefined; + holeCount++; + oldKeys.set(-holeCount, i as any); } - - this.nextKey.set(key, undefined!); - this.previousKey.set(key, undefined!); } - this.items.set(key, undefined!); - return contains; + oldKeys.set(0, newKeys as any); + oldValues.set(0, newValues as any); + + this.orderedKeys = newKeys; + this.orderedValues = newValues; + this.nextSlot = newSlot; + this.deletedCount = 0; } public forEach(callback: (value: V, key: K, map: Map) => any): void { for (const key of this.keys()) { - callback(this.items.get(key), key, this); + callback(this.orderedValues.get(this.keyIndex.get(key)), key, this); } } public get(key: K): V | undefined { - return this.items.get(key); + const idx = this.keyIndex.get(key); + if (idx === undefined) return undefined; + return this.orderedValues.get(idx); } public has(key: K): boolean { - return this.nextKey.get(key) !== undefined || this.lastKey === key; + return this.keyIndex.get(key) !== undefined; } public set(key: K, value: V): this { - const isNewValue = !this.has(key); - if (isNewValue) { + const existingIdx = this.keyIndex.get(key); + if (existingIdx !== undefined) { + this.orderedValues.set(existingIdx, value); + } else { this.size++; + const idx = this.nextSlot; + this.nextSlot = idx + 1; + this.keyIndex.set(key, idx); + this.orderedKeys.set(idx, key); + this.orderedValues.set(idx, value); } - this.items.set(key, value); - - // Do order bookkeeping - if (this.firstKey === undefined) { - this.firstKey = key; - this.lastKey = key; - } else if (isNewValue) { - this.nextKey.set(this.lastKey!, key); - this.previousKey.set(key, this.lastKey!); - this.lastKey = key; - } - return this; } @@ -112,65 +153,110 @@ export class Map { return this.entries(); } + // entries/keys/values are intentionally kept inline (not refactored into + // a shared helper) to avoid per-step function call overhead on this hot path. public entries(): IterableIterator<[K, V]> { - const getFirstKey = () => this.firstKey; - const { items, nextKey } = this; - let key: K | undefined; - let started = false; + let keys = this.orderedKeys; + let vals = this.orderedValues; + const map = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator<[K, V]> { return this; }, next(): IteratorResult<[K, V]> { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + vals = vals.get(0) as any; + keys = keys.get(0) as any; + } + while (idx < map.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= map.nextSlot) { + return { done: true, value: undefined! }; } - return { done: !key, value: [key!, items.get(key!)] as [K, V] }; + const i = idx; + idx++; + return { done: false, value: [keys.get(i), vals.get(i)] as [K, V] }; }, }; } public keys(): IterableIterator { - const getFirstKey = () => this.firstKey; - const nextKey = this.nextKey; - let key: K | undefined; - let started = false; + let keys = this.orderedKeys; + const map = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator { return this; }, next(): IteratorResult { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + keys = keys.get(0) as any; } - return { done: !key, value: key! }; + while (idx < map.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= map.nextSlot) { + return { done: true, value: undefined! }; + } + const i = idx; + idx++; + return { done: false, value: keys.get(i) }; }, }; } public values(): IterableIterator { - const getFirstKey = () => this.firstKey; - const { items, nextKey } = this; - let key: K | undefined; - let started = false; + let keys = this.orderedKeys; + let vals = this.orderedValues; + const map = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator { return this; }, next(): IteratorResult { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + vals = vals.get(0) as any; + keys = keys.get(0) as any; + } + while (idx < map.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= map.nextSlot) { + return { done: true, value: undefined! }; } - return { done: !key, value: items.get(key!) }; + const i = idx; + idx++; + return { done: false, value: vals.get(i) }; }, }; } diff --git a/src/lualib/Set.ts b/src/lualib/Set.ts index e302e4094..e878154ba 100644 --- a/src/lualib/Set.ts +++ b/src/lualib/Set.ts @@ -1,13 +1,15 @@ +// See Map.ts for design overview and V8 source references. export class Set { public static [Symbol.species] = Set; public [Symbol.toStringTag] = "Set"; public size = 0; - private firstKey: T | undefined; - private lastKey: T | undefined; - private nextKey = new LuaTable(); - private previousKey = new LuaTable(); + // Flat array storage (1-based indices for Lua) + private keyIndex = new LuaTable(); + private orderedKeys = new LuaTable(); + private nextSlot = 1; + private deletedCount = 0; constructor(values?: Iterable | T[]) { if (values === undefined) return; @@ -32,59 +34,63 @@ export class Set { } public add(value: T): Set { - const isNewValue = !this.has(value); - if (isNewValue) { + if (!this.has(value)) { this.size++; + const idx = this.nextSlot; + this.nextSlot = idx + 1; + this.keyIndex.set(value, idx); + this.orderedKeys.set(idx, value); } - - // Do order bookkeeping - if (this.firstKey === undefined) { - this.firstKey = value; - this.lastKey = value; - } else if (isNewValue) { - this.nextKey.set(this.lastKey!, value); - this.previousKey.set(value, this.lastKey!); - this.lastKey = value; - } - return this; } public clear(): void { - this.nextKey = new LuaTable(); - this.previousKey = new LuaTable(); - this.firstKey = undefined; - this.lastKey = undefined; + this.keyIndex = new LuaTable(); + this.orderedKeys = new LuaTable(); + this.nextSlot = 1; this.size = 0; + this.deletedCount = 0; } public delete(value: T): boolean { - const contains = this.has(value); - if (contains) { - this.size--; - - // Do order bookkeeping - const next = this.nextKey.get(value); - const previous = this.previousKey.get(value); - if (next !== undefined && previous !== undefined) { - this.nextKey.set(previous, next); - this.previousKey.set(next, previous); - } else if (next !== undefined) { - this.firstKey = next; - this.previousKey.set(next, undefined!); - } else if (previous !== undefined) { - this.lastKey = previous; - this.nextKey.set(previous, undefined!); + const idx = this.keyIndex.get(value); + if (idx === undefined) return false; + this.size--; + this.deletedCount++; + this.keyIndex.delete(value); + this.orderedKeys.delete(idx); + + if (this.deletedCount > this.size) { + this.compact(); + } + return true; + } + + // See Map.compact() for design explanation. + private compact(): void { + const oldKeys = this.orderedKeys; + const oldNextSlot = this.nextSlot; + const newKeys = new LuaTable(); + let newSlot = 1; + let holeCount = 0; + + for (let i = 1; i < oldNextSlot; i++) { + const k = oldKeys.get(i); + if (k !== undefined) { + newKeys.set(newSlot, k); + this.keyIndex.set(k, newSlot); + newSlot++; } else { - this.firstKey = undefined; - this.lastKey = undefined; + holeCount++; + oldKeys.set(-holeCount, i as any); } - - this.nextKey.set(value, undefined!); - this.previousKey.set(value, undefined!); } - return contains; + oldKeys.set(0, newKeys as any); + + this.orderedKeys = newKeys; + this.nextSlot = newSlot; + this.deletedCount = 0; } public forEach(callback: (value: T, key: T, set: Set) => any): void { @@ -94,7 +100,7 @@ export class Set { } public has(value: T): boolean { - return this.nextKey.get(value) !== undefined || this.lastKey === value; + return this.keyIndex.get(value) !== undefined; } public [Symbol.iterator](): IterableIterator { @@ -102,64 +108,103 @@ export class Set { } public entries(): IterableIterator<[T, T]> { - const getFirstKey = () => this.firstKey; - const nextKey = this.nextKey; - let key: T | undefined; - let started = false; + let keys = this.orderedKeys; + const set = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator<[T, T]> { return this; }, next(): IteratorResult<[T, T]> { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + keys = keys.get(0) as any; + } + while (idx < set.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= set.nextSlot) { + return { done: true, value: undefined! }; } - return { done: !key, value: [key!, key!] as [T, T] }; + const val = keys.get(idx); + idx++; + return { done: false, value: [val, val] as [T, T] }; }, }; } public keys(): IterableIterator { - const getFirstKey = () => this.firstKey; - const nextKey = this.nextKey; - let key: T | undefined; - let started = false; + let keys = this.orderedKeys; + const set = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator { return this; }, next(): IteratorResult { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + keys = keys.get(0) as any; } - return { done: !key, value: key! }; + while (idx < set.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= set.nextSlot) { + return { done: true, value: undefined! }; + } + const val = keys.get(idx); + idx++; + return { done: false, value: val }; }, }; } public values(): IterableIterator { - const getFirstKey = () => this.firstKey; - const nextKey = this.nextKey; - let key: T | undefined; - let started = false; + let keys = this.orderedKeys; + const set = this; // eslint-disable-line @typescript-eslint/no-this-alias + let idx = 1; return { [Symbol.iterator](): IterableIterator { return this; }, next(): IteratorResult { - if (!started) { - started = true; - key = getFirstKey(); - } else { - key = nextKey.get(key!); + while (keys.get(0) !== undefined) { + let adj = 0; + let h = 1; + while (true) { + const holePos = keys.get(-h) as any as number | undefined; + if (holePos === undefined || holePos >= idx) break; + adj++; + h++; + } + idx -= adj; + keys = keys.get(0) as any; + } + while (idx < set.nextSlot && keys.get(idx) === undefined) { + idx++; + } + if (idx >= set.nextSlot) { + return { done: true, value: undefined! }; } - return { done: !key, value: key! }; + const val = keys.get(idx); + idx++; + return { done: false, value: val }; }, }; } diff --git a/test/unit/builtins/map.spec.ts b/test/unit/builtins/map.spec.ts index c9f6c1a2b..5d689afe9 100644 --- a/test/unit/builtins/map.spec.ts +++ b/test/unit/builtins/map.spec.ts @@ -246,6 +246,246 @@ describe.each(iterationMethods)("map.%s() handles mutation", iterationMethod => return results; `.expectToMatchJsResult(); }); + + test("for-of delete current entry continues to next", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + map.delete(key); + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current entry with only two entries", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + map.delete(key); + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete other entry during iteration", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + if (key === "a") { map.delete("b"); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current entry continues to next", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + map.forEach((_, key) => { + visited.push(key); + map.delete(key); + }); + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current and next entry", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + map.forEach((_, key) => { + visited.push(key); + if (key === "a") { map.delete("a"); map.delete("b"); } + }); + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current then re-add", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + map.forEach((_, key) => { + visited.push(key); + if (key === "a") { map.delete("a"); map.delete("b"); map.set("b", 9); } + }); + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current then add new entry", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + map.delete(key); + if (key === "a") { map.set("c", 3); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete then re-add same key", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + if (key === "a") { map.delete("b"); map.set("b", 9); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and next entry", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + if (key === "a") { map.delete("a"); map.delete("b"); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and all remaining entries", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3], ["d", 4]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + if (key === "a") { map.delete("a"); map.delete("b"); map.delete("c"); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and next then re-add next", () => { + util.testFunction` + const map = new Map([["a", 1], ["b", 2], ["c", 3]]); + const visited: string[] = []; + for (const [key] of map) { + visited.push(key); + if (key === "a") { map.delete("a"); map.delete("b"); map.set("b", 9); } + } + return { visited, size: map.size }; + `.expectToMatchJsResult(); + }); +}); + +// Adapted from V8's collection-iterator.js (TestSetIteratorMutations2/3) +// https://chromium.googlesource.com/v8/v8/+/main/test/mjsunit/es6/collection-iterator.js +describe("map iterator stress (v8-style)", () => { + test("iterator survives mass add+delete between next() calls", () => { + util.testFunction` + const m = new Map(); + m.set(1, 11); + m.set(2, 22); + const iter = m.entries(); + const r1 = iter.next(); + m.delete(2); + m.delete(1); + for (let x = 2; x < 500; ++x) m.set(x, x * 10); + for (let x = 2; x < 500; ++x) m.delete(x); + for (let x = 2; x < 1000; ++x) m.set(x, x * 10); + const r2 = iter.next(); + for (let x = 1001; x < 2000; ++x) m.set(x, x * 10); + m.delete(3); + for (let x = 6; x < 2000; ++x) m.delete(x); + const r3 = iter.next(); + m.delete(5); + const r4 = iter.next(); + return [r1.value, r2.value, r3.value, r4.done]; + `.expectToMatchJsResult(); + }); + + test("delete all then re-add, iterator finds re-added", () => { + util.testFunction` + const m = new Map(); + m.set(1, 11); + m.set(2, 22); + const iter = m.entries(); + const r1 = iter.next(); + m.delete(2); + m.delete(1); + m.set(2, 99); + const r2 = iter.next(); + const r3 = iter.next(); + return [r1.value, r2.value, r3.done]; + `.expectToMatchJsResult(); + }); + + test("mass delete during for-of", () => { + util.testFunction` + const m = new Map(); + for (let i = 0; i < 100; i++) m.set(i, i); + const visited: number[] = []; + for (const [k] of m) { + visited.push(k); + m.delete(k); + } + return { count: visited.length, size: m.size, first: visited[0], last: visited[visited.length - 1] }; + `.expectToMatchJsResult(); + }); + + test("mass delete every other during for-of", () => { + util.testFunction` + const m = new Map(); + for (let i = 0; i < 100; i++) m.set(i, i); + const visited: number[] = []; + for (const [k] of m) { + visited.push(k); + m.delete(k); + m.delete(k + 1); + } + return { count: visited.length, size: m.size }; + `.expectToMatchJsResult(); + }); +}); + +// Lua tables don't shrink when entries are set to nil — they only resize on +// the next insert that exhausts all free hash nodes (luaH_newkey → getfreepos +// → rehash). Keys 1..10000 go to the array part (positive integers, >50% +// density). After deleting all, the array part stays allocated. Inserting -1 +// (negative → hash part, which is empty/dummy after only positive keys) +// triggers rehash via the isdummy(t) check in luaH_newkey, causing +// computesizes to recalculate: 0 live integer keys → array shrinks to 0. +// Compaction during deletion already replaces orderedKeys/orderedValues +// with fresh arrays; the insert triggers rehash on the remaining keyIndex. +// See: https://github.com/lua/lua/blob/master/ltable.c (luaH_newkey, rehash, computesizes) +describe("map memory", () => { + test("deleting keys should not leak memory", () => { + const result = util.testFunction` + /** @noSelf */ declare function collectgarbage(opt?: string): number; + collectgarbage("collect"); + const baseline = collectgarbage("count"); + + const map = new Map(); + for (let i = 1; i <= 10000; i++) map.set(i, i); + for (let i = 1; i <= 10000; i++) map.delete(i); + // Trigger Lua table rehash to shrink internal tables (see comment above) + map.set(-1, -1); + map.delete(-1); + + collectgarbage("collect"); + const after = collectgarbage("count"); + + return { + size: map.size, + retained: Math.floor(after - baseline), + }; + `.getLuaExecutionResult(); + expect(result.size).toBe(0); + expect(result.retained).toBe(0); + }); }); describe("Map.groupBy", () => { diff --git a/test/unit/builtins/set.spec.ts b/test/unit/builtins/set.spec.ts index 234be91a7..dd0f60c72 100644 --- a/test/unit/builtins/set.spec.ts +++ b/test/unit/builtins/set.spec.ts @@ -235,6 +235,226 @@ describe.each(iterationMethods)("set.%s() handles mutation", iterationMethod => return results; `.expectToMatchJsResult(); }); + + test("for-of delete current entry continues to next", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + set.delete(value); + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current entry with only two entries", () => { + util.testFunction` + const set = new Set([1, 2]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + set.delete(value); + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete other entry during iteration", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + if (value === 1) { set.delete(2); } + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current entry continues to next", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + set.forEach(value => { + visited.push(value); + set.delete(value); + }); + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current and next entry", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + set.forEach(value => { + visited.push(value); + if (value === 1) { set.delete(1); set.delete(2); } + }); + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("forEach delete current then re-add", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + set.forEach(value => { + visited.push(value); + if (value === 1) { set.delete(1); set.delete(2); set.add(2); } + }); + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and next entry", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + if (value === 1) { set.delete(1); set.delete(2); } + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and all remaining entries", () => { + util.testFunction` + const set = new Set([1, 2, 3, 4]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + if (value === 1) { set.delete(1); set.delete(2); set.delete(3); } + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete current and next then re-add next", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + if (value === 1) { set.delete(1); set.delete(2); set.add(2); } + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); + + test("for-of delete then re-add same value", () => { + util.testFunction` + const set = new Set([1, 2, 3]); + const visited: number[] = []; + for (const value of set) { + visited.push(value); + if (value === 1) { set.delete(2); set.add(2); } + } + return { visited, size: set.size }; + `.expectToMatchJsResult(); + }); +}); + +// Adapted from V8's collection-iterator.js (TestSetIteratorMutations2/3) +// https://chromium.googlesource.com/v8/v8/+/main/test/mjsunit/es6/collection-iterator.js +describe("set iterator stress (v8-style)", () => { + test("iterator survives mass add+delete between next() calls", () => { + util.testFunction` + const s = new Set(); + s.add(1); + s.add(2); + const iter = s.values(); + const r1 = iter.next(); + s.delete(2); + s.delete(1); + for (let x = 2; x < 500; ++x) s.add(x); + for (let x = 2; x < 500; ++x) s.delete(x); + for (let x = 2; x < 1000; ++x) s.add(x); + const r2 = iter.next(); + for (let x = 1001; x < 2000; ++x) s.add(x); + s.delete(3); + for (let x = 6; x < 2000; ++x) s.delete(x); + const r3 = iter.next(); + s.delete(5); + const r4 = iter.next(); + return [r1.value, r2.value, r3.value, r4.done]; + `.expectToMatchJsResult(); + }); + + test("delete all then re-add, iterator finds re-added", () => { + util.testFunction` + const s = new Set(); + s.add(1); + s.add(2); + const iter = s.values(); + const r1 = iter.next(); + s.delete(2); + s.delete(1); + s.add(2); + const r2 = iter.next(); + const r3 = iter.next(); + return [r1.value, r2.value, r3.done]; + `.expectToMatchJsResult(); + }); + + test("mass delete during for-of", () => { + util.testFunction` + const s = new Set(); + for (let i = 0; i < 100; i++) s.add(i); + const visited: number[] = []; + for (const v of s) { + visited.push(v); + s.delete(v); + } + return { count: visited.length, size: s.size, first: visited[0], last: visited[visited.length - 1] }; + `.expectToMatchJsResult(); + }); + + test("mass delete every other during for-of", () => { + util.testFunction` + const s = new Set(); + for (let i = 0; i < 100; i++) s.add(i); + const visited: number[] = []; + for (const v of s) { + visited.push(v); + s.delete(v); + s.delete(v + 1); + } + return { count: visited.length, size: s.size }; + `.expectToMatchJsResult(); + }); +}); + +// See map.spec.ts "map memory" describe block for detailed explanation of +// the Lua table rehash trick with negative keys. +// https://github.com/lua/lua/blob/master/ltable.c (luaH_newkey, rehash, computesizes) +describe("set memory", () => { + test("deleting values should not leak memory", () => { + const result = util.testFunction` + /** @noSelf */ declare function collectgarbage(opt?: string): number; + collectgarbage("collect"); + const baseline = collectgarbage("count"); + + const set = new Set(); + for (let i = 1; i <= 10000; i++) set.add(i); + for (let i = 1; i <= 10000; i++) set.delete(i); + // Trigger Lua table rehash to shrink internal tables + set.add(-1); + set.delete(-1); + + collectgarbage("collect"); + const after = collectgarbage("count"); + + return { + size: set.size, + retained: Math.floor(after - baseline), + }; + `.getLuaExecutionResult(); + expect(result.size).toBe(0); + expect(result.retained).toBe(0); + }); }); test("instanceof Set without creating set", () => {