From 04fe9a1110e377d8a81d4cb671bee93a3be4b537 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Thu, 20 Aug 2026 10:25:04 +0300 Subject: [PATCH] Give Map and Set the [[SetData]] tombstone their traversals are specified over Set.prototype.intersection and .isSubsetOf walk the receiver's [[SetData]] by index while the set-like they are handed is free to mutate that receiver from its has callback. The spec keeps the walk coherent by never removing an entry: a deleted one becomes ~empty~ in place, so every surviving entry keeps its index. Jint's ordered set compacted on delete instead, so a delete shifted later entries left underneath the walk and it skipped or repeated elements. The same List, and the same exposure, is behind Set.prototype.forEach, the Set iterator, difference, isDisjointFrom, and every one of Map's counterparts, so the representation is fixed once for both: KeyedCollectionData holds the entry list with tombstones and a key-to-slot dictionary, which also makes delete O(1) where it used to be a linear scan plus a linear shift. Deleted slots are reclaimed when the last entry is deleted and when an append finds at least half the slots dead, so add/delete churn cannot grow the list without bound; both reclaim paths move live entries, so a suspended cursor resumes by the entry's own sequence number rather than by a raw slot index. The five hand-written "adjust the position for mutations" heuristics that stood in for the tombstone are gone with it. So are two ordering defects they were no part of, both found while implementing. intersection, difference and symmetricDifference answered from an unordered hash set whose enumeration reuses the slot a delete freed, so after `s.delete(1); s.add(4)` they reported the re-added element first. And isSupersetOf read the receiver's size before GetSetRecord where step 4 reads it after, so a set-like whose size, has or keys getter grows the receiver was compared against a size taken before its own getters had run. Frees staging/sm/Set/intersection.js and staging/sm/Set/is-subset-of.js. Co-Authored-By: Claude Opus 5 (1M context) --- .../Test262Harness.settings.json | 9 - .../Runtime/KeyedCollectionMutationTests.cs | 325 ++++++++++ Jint.Tests/Runtime/SetTests.cs | 39 ++ Jint/Native/JsMap.cs | 65 +- Jint/Native/JsSet.cs | 63 +- Jint/Native/Map/MapIteratorPrototype.cs | 61 +- Jint/Native/Set/SetIteratorPrototype.cs | 119 +--- Jint/Native/Set/SetPrototype.cs | 268 +++++--- Jint/Runtime/JintOrderedDictionary.cs | 585 ------------------ Jint/Runtime/KeyedCollectionData.cs | 420 +++++++++++++ Jint/Runtime/OrderedSet.cs | 105 ---- 11 files changed, 1060 insertions(+), 999 deletions(-) create mode 100644 Jint.Tests/Runtime/KeyedCollectionMutationTests.cs delete mode 100644 Jint/Runtime/JintOrderedDictionary.cs create mode 100644 Jint/Runtime/KeyedCollectionData.cs delete mode 100644 Jint/Runtime/OrderedSet.cs diff --git a/Jint.Tests.Test262/Test262Harness.settings.json b/Jint.Tests.Test262/Test262Harness.settings.json index 017651ce05..c96410140c 100644 --- a/Jint.Tests.Test262/Test262Harness.settings.json +++ b/Jint.Tests.Test262/Test262Harness.settings.json @@ -66,15 +66,6 @@ // poisoned value's own error instead of the RangeError. Removed by fixing that ordering. "staging/sm/TypedArray/constructor-buffer-sequence.js", - // Set.prototype.intersection / .isSubsetOf let the *receiver* be mutated while they traverse it - // (a set-like's has/keys callback calling this.delete(v) or this.clear()), and the spec models - // that with the [[SetData]] tombstone: a deleted entry becomes EMPTY in place rather than being - // removed, so the index-based traversal keeps its position and still visits the right elements. - // Jint's JsSet compacts on delete, so the traversal skips or repeats entries. Removed by giving - // the ordered set a tombstone representation. - "staging/sm/Set/intersection.js", - "staging/sm/Set/is-subset-of.js", - // Acornima (the external parser) rejects a SuperCall inside a direct eval. Jint already parses // eval code with AllowSuperOutsideMethod, but that covers SuperProperty only; Acornima 1.7.0 has // no separate switch for a direct `super()`, so `eval("super()")` in a derived constructor fails diff --git a/Jint.Tests/Runtime/KeyedCollectionMutationTests.cs b/Jint.Tests/Runtime/KeyedCollectionMutationTests.cs new file mode 100644 index 0000000000..54a0a0ce0d --- /dev/null +++ b/Jint.Tests/Runtime/KeyedCollectionMutationTests.cs @@ -0,0 +1,325 @@ +using Jint.Native; + +namespace Jint.Tests.Runtime; + +/// +/// Map and Set are specified over a List of entries in which a deleted entry becomes ~empty~ +/// in place rather than being removed — https://tc39.es/ecma262/#sec-set.prototype.delete and +/// https://tc39.es/ecma262/#sec-map.prototype.delete. Every traversal the spec defines walks that List +/// by index while user code is free to mutate the collection between two steps: forEach +/// (https://tc39.es/ecma262/#sec-set.prototype.foreach), the iterators +/// (https://tc39.es/ecma262/#sec-createsetiterator, https://tc39.es/ecma262/#sec-createmapiterator) and +/// the index-walking half of difference, intersection, isDisjointFrom and +/// isSubsetOf. The tombstone is what makes those walks coherent, and these tests pin the +/// behaviour that depends on it from outside test262. +/// +public class KeyedCollectionMutationTests +{ + private static string Run(string script) => new Engine().Evaluate(script).AsString(); + + [Fact] + public void SetForEachRevisitsAValueDeletedAndReAddedWhileVisiting() + { + const string Script = """ + var s = new Set([1, 2, 3]); + var seen = []; + var once = true; + s.forEach(function (v) { + seen.push(v); + if (v === 2 && once) { once = false; s.delete(2); s.add(2); } + }); + seen.join(',') + '|' + [...s].join(','); + """; + + Run(Script).Should().Be("1,2,3,2|1,3,2"); + } + + [Fact] + public void SetForEachDoesNotVisitAValueDeletedBeforeItsTurn() + { + const string Script = """ + var s = new Set([1, 2, 3, 4]); + var seen = []; + s.forEach(function (v) { seen.push(v); if (v === 1) { s.delete(3); } }); + seen.join(','); + """; + + Run(Script).Should().Be("1,2,4"); + } + + [Fact] + public void SetForEachKeepsItsPlaceWhenAnAlreadyVisitedValueIsDeleted() + { + const string Script = """ + var s = new Set([1, 2, 3, 4]); + var seen = []; + s.forEach(function (v) { seen.push(v); if (v === 1) { s.delete(1); } }); + seen.join(','); + """; + + Run(Script).Should().Be("1,2,3,4"); + } + + [Fact] + public void SetIteratorResumesAtTheRightEntryAfterDeletesAndAdds() + { + const string Script = """ + var s = new Set([1, 2, 3]); + var it = s.values(); + var out = [it.next().value]; + s.delete(1); + s.delete(2); + out.push(it.next().value); + s.add(4); + out.push(it.next().value); + out.push(String(it.next().done)); + out.join(','); + """; + + Run(Script).Should().Be("1,3,4,true"); + } + + /// + /// The entry List is reclaimed once at least half of its slots are deleted, which moves every live + /// entry. A suspended iterator has to survive that, so it resumes by the entry's own sequence + /// number rather than by a raw slot index. + /// + [Fact] + public void SetIteratorSurvivesAnEntryListCompaction() + { + const string Script = """ + var s = new Set(); + for (var i = 0; i < 100; i++) { s.add(i); } + var it = s.values(); + var out = [it.next().value]; + for (var i = 1; i < 100; i += 2) { s.delete(i); } + for (var i = 100; i < 300; i++) { s.add(i); } + out.push(it.next().value); + out.push(it.next().value); + out.push(it.next().value); + out.join(','); + """; + + Run(Script).Should().Be("0,2,4,6"); + } + + [Fact] + public void SetIteratorSeesEntriesAddedAfterAClear() + { + const string Script = """ + var s = new Set([1, 2, 3]); + var it = s.values(); + var out = [it.next().value]; + s.clear(); + s.add(7); + s.add(8); + out.push(it.next().value); + out.push(it.next().value); + out.push(String(it.next().done)); + out.join(','); + """; + + Run(Script).Should().Be("1,7,8,true"); + } + + [Fact] + public void AnExhaustedSetIteratorStaysDone() + { + const string Script = """ + var s = new Set([1]); + var it = s.values(); + it.next(); + var first = it.next().done; + s.add(2); + String(first) + ',' + String(it.next().done); + """; + + Run(Script).Should().Be("true,true"); + } + + /// + /// The receiver's has callback removes and re-adds an element that has already been visited, + /// which the spec says makes that element visible a second time, at its new position — and the walk + /// must still reach the elements after it. + /// + [Fact] + public void IntersectionRevisitsAnElementItsHasCallbackReAdded() + { + const string Script = """ + var seen = []; + var setLike = { + size: 100, + has: function (v) { + if (v === 2 && seen.indexOf(v) < 0) { s.delete(v); s.add(v); } + seen.push(v); + return true; + }, + keys: function () { throw new Error('unexpected keys'); } + }; + var s = new Set([1, 2, 3]); + [...s.intersection(setLike)].join(',') + '|' + seen.join(','); + """; + + Run(Script).Should().Be("1,2,3|1,2,3,2"); + } + + /// + /// Each call to has deletes the element it was handed and appends a new one; the walk has to + /// keep its place across both, so every element ever in the set is visited exactly once. + /// + [Fact] + public void IsSubsetOfVisitsEveryElementWhenHasDeletesTheCurrentOne() + { + const string Script = """ + var s = new Set([1]); + var seen = []; + var newKeys = [2, 3, 4, 5]; + var setLike = { + size: 100, + has: function (v) { + seen.push(v); + s.delete(v); + if (newKeys.length) { s.add(newKeys.shift()); } + return true; + }, + keys: function () { throw new Error('unexpected keys'); } + }; + String(s.isSubsetOf(setLike)) + '|' + seen.join(',') + '|' + s.size; + """; + + Run(Script).Should().Be("true|1,2,3,4,5|0"); + } + + /// + /// A delete followed by an add moves the added element to the end, and every Set method has to + /// report that order. The combining methods used to answer from an unordered hash set, whose + /// enumeration reuses the slot a delete freed, so the re-added element came back first. + /// + [Fact] + public void SetMethodsReportInsertionOrderAfterADeleteAndAnAdd() + { + const string Script = """ + var s = new Set([1, 2, 3]); + s.delete(1); + s.add(4); + [ + [...s].join(''), + [...s.intersection(new Set([2, 3, 4]))].join(''), + [...s.union(new Set([9]))].join(''), + [...s.symmetricDifference(new Set([9]))].join(''), + [...s.difference(new Set([9]))].join(''), + [...s.difference(new Set([3]))].join('') + ].join('|'); + """; + + Run(Script).Should().Be("234|234|2349|2349|234|24"); + } + + [Fact] + public void MapForEachRevisitsAKeyDeletedAndReAddedWhileVisiting() + { + const string Script = """ + var m = new Map([[1, 'a'], [2, 'b'], [3, 'c']]); + var seen = []; + var once = true; + m.forEach(function (v, k) { + seen.push(k); + if (k === 2 && once) { once = false; m.delete(2); m.set(2, 'B'); } + }); + seen.join(',') + '|' + [...m.keys()].join(','); + """; + + Run(Script).Should().Be("1,2,3,2|1,3,2"); + } + + [Fact] + public void MapForEachKeepsItsPlaceWhenAnAlreadyVisitedKeyIsDeleted() + { + const string Script = """ + var m = new Map([[1, 'a'], [2, 'b'], [3, 'c'], [4, 'd']]); + var seen = []; + m.forEach(function (v, k) { seen.push(k + '=' + v); if (k === 1) { m.delete(1); } }); + seen.join(','); + """; + + Run(Script).Should().Be("1=a,2=b,3=c,4=d"); + } + + [Fact] + public void MapIteratorResumesAtTheRightEntryAfterDeletesAndAdds() + { + const string Script = """ + var m = new Map([[1, 'a'], [2, 'b'], [3, 'c']]); + var it = m.entries(); + var out = [it.next().value[0]]; + m.delete(1); + m.delete(2); + out.push(it.next().value[0]); + m.set(4, 'd'); + out.push(it.next().value[0]); + out.push(String(it.next().done)); + out.join(','); + """; + + Run(Script).Should().Be("1,3,4,true"); + } + + [Fact] + public void MapIteratorSurvivesAnEntryListCompaction() + { + const string Script = """ + var m = new Map(); + for (var i = 0; i < 100; i++) { m.set(i, i); } + var it = m.keys(); + var out = [it.next().value]; + for (var i = 1; i < 100; i += 2) { m.delete(i); } + for (var i = 100; i < 300; i++) { m.set(i, i); } + out.push(it.next().value); + out.push(it.next().value); + out.join(','); + """; + + Run(Script).Should().Be("0,2,4"); + } + + [Fact] + public void SettingAnExistingMapKeyLeavesItWhereItIs() + { + const string Script = """ + var m = new Map([[1, 'a'], [2, 'b']]); + m.set(1, 'z'); + [...m.keys()].join(',') + '|' + [...m.values()].join(','); + """; + + Run(Script).Should().Be("1,2|z,b"); + } + + /// + /// A tombstone that is never reclaimed turns add/delete churn into unbounded growth, so the entry + /// List has to stay within a constant factor of the live count. Deleting the last entry drops its + /// slot outright, which is what keeps this shape flat. + /// + [Fact] + public void AddDeleteChurnDoesNotGrowTheSetEntryList() + { + var engine = new Engine(); + var set = (JsSet) engine.Evaluate("var s = new Set([0]); for (var i = 1; i <= 20000; i++) { s.add(i); s.delete(i); } s;"); + + set.Size.Should().Be(1); + set._data.SlotCount.Should().BeLessThanOrEqualTo(4); + } + + /// + /// The sliding-window shape deletes from the front rather than the tail, so the slot list is held + /// down by compaction instead: it may never grow while more than half of it is live. + /// + [Fact] + public void ASlidingWindowDoesNotGrowTheMapEntryList() + { + var engine = new Engine(); + var map = (JsMap) engine.Evaluate("var m = new Map(); for (var i = 0; i < 20000; i++) { m.set(i, i); if (i >= 100) { m.delete(i - 100); } } m;"); + + map.Size.Should().Be(100); + map._data.SlotCount.Should().BeLessThanOrEqualTo(512); + } +} diff --git a/Jint.Tests/Runtime/SetTests.cs b/Jint.Tests/Runtime/SetTests.cs index c0d6efc351..3c96d2fcb1 100644 --- a/Jint.Tests/Runtime/SetTests.cs +++ b/Jint.Tests/Runtime/SetTests.cs @@ -44,4 +44,43 @@ public void HasProperIteratorPrototypeChain() engine.Evaluate("!iterator.hasOwnProperty(Symbol.iterator)").AsBoolean().Should().BeTrue(); engine.Evaluate("iterator[Symbol.iterator]() === iterator").AsBoolean().Should().BeTrue(); } + + /// + /// https://tc39.es/ecma262/#sec-set.prototype.issupersetof reads the receiver's size at step 4, + /// after GetSetRecord at step 3. The order is observable, because GetSetRecord runs the + /// set-like's own size, has and keys getters and those may add to the receiver: + /// a receiver grown from one to two elements is a superset of a two-element set-like, and reading + /// its size first answered false. + /// + [Fact] + public void IsSupersetOfReadsTheReceiverSizeAfterBuildingTheSetRecord() + { + const string Script = @" + var s = new Set([1]); + var log = []; + var setLike = { + get size() { log.push('size'); s.add(2); return 2; }, + get has() { log.push('has'); return function () { throw new Error('unexpected has'); }; }, + get keys() { log.push('keys'); return function () { return [1, 2][Symbol.iterator](); }; } + }; + return String(s.isSupersetOf(setLike)) + '|' + log.join(',') + '|' + [...s].join(',');"; + + new Engine().Evaluate(Script).AsString().Should().Be("true|size,has,keys|1,2"); + } + + /// The sibling case, where the receiver really is too small, must still be false. + [Fact] + public void IsSupersetOfIsFalseWhenTheReceiverStaysSmaller() + { + const string Script = @" + var s = new Set([1]); + var setLike = { + size: 2, + has: function () { throw new Error('unexpected has'); }, + keys: function () { throw new Error('unexpected keys'); } + }; + return s.isSupersetOf(setLike);"; + + new Engine().Evaluate(Script).AsBoolean().Should().BeFalse(); + } } diff --git a/Jint/Native/JsMap.cs b/Jint/Native/JsMap.cs index 840591fdb6..20d7cb592b 100644 --- a/Jint/Native/JsMap.cs +++ b/Jint/Native/JsMap.cs @@ -7,12 +7,12 @@ namespace Jint.Native; public sealed class JsMap : ObjectInstance, IEnumerable> { private readonly Realm _realm; - internal readonly JintOrderedDictionary _map; + internal readonly KeyedCollectionData _data; public JsMap(Engine engine, Realm realm) : base(engine) { _realm = realm; - _map = new JintOrderedDictionary(SameValueZeroComparer.Instance); + _data = new KeyedCollectionData(); // Every Map reaches this constructor, subclass instances included, so the bit is exactly the // brand MapPrototype's methods check. It is set here rather than passed through the internal // ObjectInstance constructor because that one skips DeriveAccessSemantics, and this type does @@ -25,17 +25,17 @@ public JsMap(Engine engine, Realm realm) : base(engine) // prototype getter unreachable through `m.size` — which is how it went on returning a hard-coded 0 — and // gave every map a phantom own non-configurable `size` that [[OwnPropertyKeys]] never listed. - public int Size => _map.Count; + public int Size => _data.Count; - public void Clear() => _map.Clear(); + public void Clear() => _data.Clear(); - public bool Has(JsValue key) => _map.ContainsKey(key); + public bool Has(JsValue key) => _data.ContainsKey(key); - public bool Remove(JsValue key) => _map.Remove(key); + public bool Remove(JsValue key) => _data.Remove(key); public new JsValue Get(JsValue key) { - if (!_map.TryGetValue(key, out var value)) + if (!_data.TryGetValue(key, out var value)) { return Undefined; } @@ -46,12 +46,12 @@ public JsMap(Engine engine, Realm realm) : base(engine) internal JsValue GetOrInsert(JsValue key, JsValue value) { key = SameValueZeroComparer.ToStableKey(key); - if (_map.TryGetValue(key, out var temp)) + if (_data.TryGetValue(key, out var temp)) { return temp; } - _map[key] = value; + _data.Append(key, value); return value; } @@ -60,14 +60,15 @@ internal JsValue GetOrInsertComputed(JsValue key, ICallable callbackfn) // Flatten before the callback runs: the key is the string *value* the operation was handed // (spec step 4 canonicalizes it up front), not a buffer the callback may still append to. key = SameValueZeroComparer.ToStableKey(key); - if (_map.TryGetValue(key, out var temp)) + if (_data.TryGetValue(key, out var temp)) { return temp; } var value = callbackfn.Call(Undefined, key); - _map[key] = value; + // The callback may have inserted the key itself, so this cannot assume it is still absent. + _data.Set(key, value); return value; } @@ -77,16 +78,22 @@ internal JsValue GetOrInsertComputed(JsValue key, ICallable callbackfn) { key = JsNumber.PositiveZero; } - _map[SameValueZeroComparer.ToStableKey(key)] = value; + _data.Set(SameValueZeroComparer.ToStableKey(key), value); } + /// + /// https://tc39.es/ecma262/#sec-map.prototype.foreach + /// internal void ForEach(ICallable callable, JsValue thisArg) { var invoker = CallbackInvoker.Rent(_engine, callable, 3, this); - var i = 0; + // See JsSet.ForEach: the cursor is the spec's `index` into [[MapData]] and stays exact across + // whatever the callback does to the map. + var cursor = default(KeyedCollectionCursor); var iterations = 0; - while (i < _map.Count) + int slot; + while ((slot = _data.Next(ref cursor)) >= 0) { // A native (CLR) callback does not self-throttle via statement checks; check periodically. if (++iterations % Engine.ConstraintCheckInterval == 0) @@ -94,26 +101,7 @@ internal void ForEach(ICallable callable, JsValue thisArg) _engine.Constraints.Check(); } - var key = _map.GetKey(i); - invoker.Call(thisArg, _map[key], key); - - // Adjust position for mutations during callback - if (i < _map.Count && ReferenceEquals(_map.GetKey(i), key)) - { - // Common fast path: key still at same position - i++; - } - else if (_map.ContainsKey(key)) - { - var newIndex = _map.IndexOf(key); - if (newIndex < i) - { - // Key moved backward (entries before it were deleted) - i = newIndex + 1; - } - // else: key was deleted and re-added at end, keep i (entries shifted left) - } - // else: key was deleted, entries shifted left so i now points to next entry + invoker.Call(thisArg, _data.ValueAt(slot), _data.KeyAt(slot)!); } invoker.Return(); @@ -125,7 +113,14 @@ internal void ForEach(ICallable callable, JsValue thisArg) internal ObjectInstance Values() => _realm.Intrinsics.MapIteratorPrototype.ConstructValueIterator(this); - public IEnumerator> GetEnumerator() => _map.GetEnumerator(); + public IEnumerator> GetEnumerator() + { + var enumerator = _data.GetEnumerator(); + while (enumerator.MoveNext()) + { + yield return new KeyValuePair(enumerator.Key, enumerator.Value); + } + } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Jint/Native/JsSet.cs b/Jint/Native/JsSet.cs index c88ae70be9..83940e3da0 100644 --- a/Jint/Native/JsSet.cs +++ b/Jint/Native/JsSet.cs @@ -6,15 +6,15 @@ namespace Jint.Native; public sealed class JsSet : ObjectInstance, IEnumerable { - internal readonly OrderedSet _set; + internal readonly KeyedCollectionData _data; - internal JsSet(Engine engine) : this(engine, new OrderedSet(SameValueZeroComparer.Instance)) + internal JsSet(Engine engine) : this(engine, new KeyedCollectionData()) { } - internal JsSet(Engine engine, OrderedSet set) : base(engine) + internal JsSet(Engine engine, KeyedCollectionData data) : base(engine) { - _set = set; + _data = data; _prototype = _engine.Realm.Intrinsics.Set.PrototypeObject; // Every Set reaches this constructor, subclass instances included, so the bit is exactly the // brand SetPrototype's methods check. It is set here rather than passed through the internal @@ -26,28 +26,30 @@ internal JsSet(Engine engine, OrderedSet set) : base(engine) // No `size` here: it is an accessor on Set.prototype (https://tc39.es/ecma262/#sec-get-set.prototype.size) // and an instance has no own property of that name. See the note in JsMap for what synthesizing one cost. - public int Size => _set.Count; + public int Size => _data.Count; - internal JsValue? this[int index] - { - get { return index < _set._list.Count ? _set._list[index] : null; } - } - - public void Add(JsValue value) => _set.Add(SameValueZeroComparer.ToStableKey(value)); + public void Add(JsValue value) => _data.Add(SameValueZeroComparer.ToStableKey(value)); - public void Clear() => _set.Clear(); + public void Clear() => _data.Clear(); - public bool Has(JsValue key) => _set.Contains(key); + public bool Has(JsValue key) => _data.ContainsKey(key); - public new bool Delete(JsValue key) => _set.Remove(key); + public new bool Delete(JsValue key) => _data.Remove(key); + /// + /// https://tc39.es/ecma262/#sec-set.prototype.foreach + /// internal void ForEach(ICallable callable, JsValue thisArg) { var invoker = CallbackInvoker.Rent(_engine, callable, 3, this); - var i = 0; + // The cursor is the spec's `index` into [[SetData]]: the callback may add, delete and re-add + // freely, and the tombstoned representation keeps the resume point exact without any of the + // relocate-the-last-value guesswork a compacting list needed. + var cursor = default(KeyedCollectionCursor); var iterations = 0; - while (i < _set._list.Count) + int slot; + while ((slot = _data.Next(ref cursor)) >= 0) { // A native (CLR) callback does not self-throttle via statement checks; check periodically. if (++iterations % Engine.ConstraintCheckInterval == 0) @@ -55,26 +57,8 @@ internal void ForEach(ICallable callable, JsValue thisArg) _engine.Constraints.Check(); } - var value = _set._list[i]; + var value = _data.KeyAt(slot)!; invoker.Call(thisArg, value, value); - - // Adjust position for mutations during callback - if (i < _set._list.Count && (ReferenceEquals(_set._list[i], value) || SameValueZeroComparer.Equals(_set._list[i], value))) - { - // Common fast path: value still at same position - i++; - } - else if (_set.Contains(value)) - { - var newIndex = _set.IndexOf(value); - if (newIndex < i) - { - // Value moved backward (entries before it were deleted) - i = newIndex + 1; - } - // else: value was deleted and re-added at end, keep i (entries shifted left) - } - // else: value was deleted, entries shifted left so i now points to next entry } invoker.Return(); @@ -84,7 +68,14 @@ internal void ForEach(ICallable callable, JsValue thisArg) internal ObjectInstance Values() => _engine.Realm.Intrinsics.SetIteratorPrototype.ConstructValueIterator(this); - public IEnumerator GetEnumerator() => _set.GetEnumerator(); + public IEnumerator GetEnumerator() + { + var enumerator = _data.GetEnumerator(); + while (enumerator.MoveNext()) + { + yield return enumerator.Key; + } + } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } diff --git a/Jint/Native/Map/MapIteratorPrototype.cs b/Jint/Native/Map/MapIteratorPrototype.cs index 5139f85bbd..324cf57fce 100644 --- a/Jint/Native/Map/MapIteratorPrototype.cs +++ b/Jint/Native/Map/MapIteratorPrototype.cs @@ -66,68 +66,43 @@ private enum MapIteratorKind KeyAndValue, } + /// + /// The closure of https://tc39.es/ecma262/#sec-createmapiterator, which walks [[MapData]] by index + /// and re-reads its length after every yield. is that index: + /// deleted entries are tombstones, so a mutation between two steps cannot move the resume point. + /// private sealed class MapIterator : IteratorInstance { - private readonly JintOrderedDictionary _map; + private readonly KeyedCollectionData _data; private readonly MapIteratorKind _kind; - private int _position; - private JsValue? _lastKey; + private KeyedCollectionCursor _cursor; private bool _done; public MapIterator(Engine engine, JsMap map, MapIteratorKind kind) : base(engine) { - _map = map._map; + _data = map._data; _kind = kind; - _position = 0; } public override bool TryIteratorStep(out ObjectInstance nextItem) { - if (_done) + if (!_done) { - nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); - return false; - } - - // Adjust position for mutations since last step - if (_lastKey is not null) - { - if (_position > 0 && _position - 1 < _map.Count && ReferenceEquals(_map.GetKey(_position - 1), _lastKey)) - { - // Common fast path: lastKey still at expected position - } - else if (_map.ContainsKey(_lastKey)) + var slot = _data.Next(ref _cursor); + if (slot >= 0) { - var newIndex = _map.IndexOf(_lastKey); - if (newIndex < _position - 1) + nextItem = _kind switch { - // Key moved backward (entries before it were deleted) - _position = newIndex + 1; - } - // else: key was deleted and re-added at end, keep position + MapIteratorKind.Key => IteratorResult.CreateValueIteratorPosition(_engine, _data.KeyAt(slot)!), + MapIteratorKind.Value => IteratorResult.CreateValueIteratorPosition(_engine, _data.ValueAt(slot)), + _ => IteratorResult.CreateKeyValueIteratorPosition(_engine, _data.KeyAt(slot)!, _data.ValueAt(slot)), + }; + return true; } - else - { - // Key deleted, entries shifted left - _position = System.Math.Max(0, _position - 1); - } - } - if (_position < _map.Count) - { - var key = _map.GetKey(_position); - _lastKey = key; - _position++; - nextItem = _kind switch - { - MapIteratorKind.Key => IteratorResult.CreateValueIteratorPosition(_engine, key), - MapIteratorKind.Value => IteratorResult.CreateValueIteratorPosition(_engine, _map[key]), - _ => IteratorResult.CreateKeyValueIteratorPosition(_engine, key, _map[key]), - }; - return true; + _done = true; } - _done = true; nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); return false; } diff --git a/Jint/Native/Set/SetIteratorPrototype.cs b/Jint/Native/Set/SetIteratorPrototype.cs index f534f24671..213a42cad4 100644 --- a/Jint/Native/Set/SetIteratorPrototype.cs +++ b/Jint/Native/Set/SetIteratorPrototype.cs @@ -31,121 +31,54 @@ protected override void Initialize() internal IteratorInstance ConstructEntryIterator(JsSet set) { - var instance = new SetEntryIterator(Engine, set); + var instance = new SetIterator(Engine, set, keyAndValue: true); return instance; } internal IteratorInstance ConstructValueIterator(JsSet set) { - var instance = new SetValueIterator(Engine, set); + var instance = new SetIterator(Engine, set, keyAndValue: false); return instance; } - private sealed class SetEntryIterator : IteratorInstance + /// + /// The closure of https://tc39.es/ecma262/#sec-createsetiterator, which walks [[SetData]] by index + /// and re-reads its length after every yield. is that index: + /// deleted entries are tombstones, so a mutation between two steps cannot move the resume point. + /// + private sealed class SetIterator : IteratorInstance { - private readonly JsSet _set; - private int _position; - private JsValue? _lastValue; + private readonly KeyedCollectionData _data; + private readonly bool _keyAndValue; + private KeyedCollectionCursor _cursor; private bool _done; - public SetEntryIterator(Engine engine, JsSet set) : base(engine) + public SetIterator(Engine engine, JsSet set, bool keyAndValue) : base(engine) { _prototype = engine.Realm.Intrinsics.SetIteratorPrototype; - _set = set; - _position = 0; + _data = set._data; + _keyAndValue = keyAndValue; } public override bool TryIteratorStep(out ObjectInstance nextItem) { - if (_done) + if (!_done) { - nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); - return false; + var slot = _data.Next(ref _cursor); + if (slot >= 0) + { + var value = _data.KeyAt(slot)!; + nextItem = _keyAndValue + ? IteratorResult.CreateKeyValueIteratorPosition(_engine, value, value) + : IteratorResult.CreateValueIteratorPosition(_engine, value); + return true; + } + + _done = true; } - // Adjust position for mutations since last step - AdjustPosition(ref _position, _lastValue, _set._set); - - if (_position < _set._set._list.Count) - { - var value = _set._set[_position]; - _lastValue = value; - _position++; - nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine, value, value); - return true; - } - - _done = true; nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); return false; } } - - private sealed class SetValueIterator : IteratorInstance - { - private readonly JsSet _set; - private int _position; - private JsValue? _lastValue; - private bool _done; - - public SetValueIterator(Engine engine, JsSet set) : base(engine) - { - _prototype = engine.Realm.Intrinsics.SetIteratorPrototype; - _set = set; - _position = 0; - } - - public override bool TryIteratorStep(out ObjectInstance nextItem) - { - if (_done) - { - nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); - return false; - } - - // Adjust position for mutations since last step - AdjustPosition(ref _position, _lastValue, _set._set); - - if (_position < _set._set._list.Count) - { - var value = _set._set[_position]; - _lastValue = value; - _position++; - nextItem = IteratorResult.CreateValueIteratorPosition(_engine, value); - return true; - } - - _done = true; - nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine); - return false; - } - } - - private static void AdjustPosition(ref int position, JsValue? lastValue, OrderedSet set) - { - if (lastValue is null) - { - return; - } - - if (position > 0 && position - 1 < set._list.Count && ReferenceEquals(set._list[position - 1], lastValue)) - { - // Common fast path: lastValue still at expected position - } - else if (set.Contains(lastValue)) - { - var newIndex = set.IndexOf(lastValue); - if (newIndex < position - 1) - { - // Value moved backward (entries before it were deleted) - position = newIndex + 1; - } - // else: value was deleted and re-added at end, keep position - } - else - { - // Value deleted, entries shifted left - position = System.Math.Max(0, position - 1); - } - } } diff --git a/Jint/Native/Set/SetPrototype.cs b/Jint/Native/Set/SetPrototype.cs index bae76b6ede..1e2fb955b4 100644 --- a/Jint/Native/Set/SetPrototype.cs +++ b/Jint/Native/Set/SetPrototype.cs @@ -95,44 +95,52 @@ private JsBoolean Delete(JsValue thisObject, JsValue value) : JsBoolean.False; } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.difference + /// [JsFunction] private JsSet Difference(JsValue thisObject, JsValue other) { var set = AssertSetInstance(thisObject); var otherRec = GetSetRecord(other); - var resultSetData = new JsSet(_engine, new OrderedSet(set._set._set)); + + // "Let resultSetData be a copy of set.[[SetData]]", taken before anything else can run. + var resultData = set._data.Clone(); if (set.Size <= otherRec.Size) { + // The copy is private to this call, so nothing can append to it; walking it with a cursor + // is the spec's `index < thisSize` walk, with the entries this loop empties left behind it. + var cursor = default(KeyedCollectionCursor); + int slot; if (other is JsSet otherSet) { // fast path - var result = new HashSet(set._set._set, SameValueZeroComparer.Instance); - result.ExceptWith(otherSet._set._set); - return new JsSet(_engine, new OrderedSet(result)); + while ((slot = resultData.Next(ref cursor)) >= 0) + { + if (otherSet._data.ContainsKey(resultData.KeyAt(slot)!)) + { + resultData.RemoveAt(slot); + } + } } - - var index = 0; - var args = new JsValue[1]; - while (index < set.Size) + else { - var e = resultSetData[index]; - if (e is not null) + var args = new JsValue[1]; + while ((slot = resultData.Next(ref cursor)) >= 0) { - args[0] = e; + args[0] = resultData.KeyAt(slot)!; var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); if (inOther) { - resultSetData.Delete(e); - index--; + resultData.RemoveAt(slot); } } - - index++; } - return resultSetData; + resultData.TrimTombstones(); + return new JsSet(_engine, resultData); } var keysIter = otherRec.Set.GetIteratorFromMethod(_realm, otherRec.Keys); @@ -149,12 +157,16 @@ private JsSet Difference(JsValue thisObject, JsValue other) nextValue = JsNumber.PositiveZero; } - resultSetData.Delete(nextValue); + resultData.Remove(nextValue); } - return resultSetData; + resultData.TrimTombstones(); + return new JsSet(_engine, resultData); } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.isdisjointfrom + /// [JsFunction] private JsBoolean IsDisjointFrom(JsValue thisObject, JsValue other) { @@ -165,25 +177,39 @@ private JsBoolean IsDisjointFrom(JsValue thisObject, JsValue other) { if (other is JsSet otherSet) { - // fast path - return set._set._set.Overlaps(otherSet._set._set) ? JsBoolean.False : JsBoolean.True; - } + // fast path: walk the smaller side, probing the larger + var smaller = set._data; + var larger = otherSet._data; + if (smaller.Count > larger.Count) + { + var swap = smaller; + smaller = larger; + larger = swap; + } - var index = 0; - var args = new JsValue[1]; - while (index < set.Size) - { - var e = set[index]; - index++; - if (e is not null) + var enumerator = smaller.GetEnumerator(); + while (enumerator.MoveNext()) { - args[0] = e; - var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); - if (inOther) + if (larger.ContainsKey(enumerator.Key)) { return JsBoolean.False; } } + + return JsBoolean.True; + } + + var args = new JsValue[1]; + var cursor = default(KeyedCollectionCursor); + int slot; + while ((slot = set._data.Next(ref cursor)) >= 0) + { + args[0] = set._data.KeyAt(slot)!; + var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); + if (inOther) + { + return JsBoolean.False; + } } return JsBoolean.True; @@ -208,49 +234,53 @@ private JsBoolean IsDisjointFrom(JsValue thisObject, JsValue other) return JsBoolean.True; } - + /// + /// https://tc39.es/ecma262/#sec-set.prototype.intersection + /// [JsFunction] private JsSet Intersection(JsValue thisObject, JsValue other) { var set = AssertSetInstance(thisObject); var otherRec = GetSetRecord(other); - var resultSetData = new JsSet(_engine); - var thisSize = set.Size; + var resultData = new KeyedCollectionData(); - if (thisSize <= otherRec.Size) + if (set.Size <= otherRec.Size) { + // The receiver's own [[SetData]] is walked by index, and otherRec.[[Has]] is free to add to + // it, delete from it, or both — the cursor over the tombstoned List is what keeps the walk + // on the entries the spec says it visits, in the order it says. + var cursor = default(KeyedCollectionCursor); + int slot; + if (other is JsSet otherSet) { // fast path - var result = new HashSet(set._set._set, SameValueZeroComparer.Instance); - result.IntersectWith(otherSet._set._set); - return new JsSet(_engine, new OrderedSet(result)); + while ((slot = set._data.Next(ref cursor)) >= 0) + { + var entry = set._data.KeyAt(slot)!; + if (otherSet._data.ContainsKey(entry)) + { + resultData.Append(entry, null); + } + } } - - var index = 0; - var args = new JsValue[1]; - while (index < thisSize) + else { - var e = set[index]; - index++; - if (e is not null) + var args = new JsValue[1]; + while ((slot = set._data.Next(ref cursor)) >= 0) { - args[0] = e; + var entry = set._data.KeyAt(slot)!; + args[0] = entry; var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); - if (inOther) + if (inOther && !resultData.ContainsKey(entry)) { - var alreadyInResult = resultSetData.Has(e); - if (!alreadyInResult) - { - resultSetData.Add(e); - } + resultData.Append(entry, null); } - thisSize = set.Size; } } - return resultSetData; + return new JsSet(_engine, resultData); } var keysIter = otherRec.Set.GetIteratorFromMethod(_realm, otherRec.Keys); @@ -267,17 +297,18 @@ private JsSet Intersection(JsValue thisObject, JsValue other) nextValue = JsNumber.PositiveZero; } - var alreadyInResult = resultSetData.Has(nextValue); - var inThis = set.Has(nextValue); - if (!alreadyInResult && inThis) + if (set.Has(nextValue) && !resultData.ContainsKey(nextValue)) { - resultSetData.Add(nextValue); + resultData.Append(SameValueZeroComparer.ToStableKey(nextValue), null); } } - return resultSetData; + return new JsSet(_engine, resultData); } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.symmetricdifference + /// [JsFunction] private JsSet SymmetricDifference(JsValue thisObject, JsValue other) { @@ -286,14 +317,28 @@ private JsSet SymmetricDifference(JsValue thisObject, JsValue other) if (other is JsSet otherSet) { // fast path - var result = new HashSet(set._set._set, SameValueZeroComparer.Instance); - result.SymmetricExceptWith(otherSet._set._set); - return new JsSet(_engine, new OrderedSet(result)); + var fastResult = set._data.Clone(); + var enumerator = otherSet._data.GetEnumerator(); + while (enumerator.MoveNext()) + { + var key = enumerator.Key; + if (set._data.ContainsKey(key)) + { + fastResult.Remove(key); + } + else + { + fastResult.Add(key); + } + } + + fastResult.TrimTombstones(); + return new JsSet(_engine, fastResult); } var otherRec = GetSetRecord(other); var keysIter = otherRec.Set.GetIteratorFromMethod(_realm, otherRec.Keys); - var resultSetData = new JsSet(_engine, new OrderedSet(set._set._set)); + var resultData = set._data.Clone(); while (true) { if (!keysIter.TryIteratorStep(out var next)) @@ -307,26 +352,30 @@ private JsSet SymmetricDifference(JsValue thisObject, JsValue other) nextValue = JsNumber.PositiveZero; } - var inResult = resultSetData.Has(nextValue); + var alreadyInResult = resultData.ContainsKey(nextValue); if (set.Has(nextValue)) { - if (inResult) + if (alreadyInResult) { - resultSetData.Delete(nextValue); + resultData.Remove(nextValue); } } else { - if (!inResult) + if (!alreadyInResult) { - resultSetData.Add(nextValue); + resultData.Append(SameValueZeroComparer.ToStableKey(nextValue), null); } } } - return resultSetData; + resultData.TrimTombstones(); + return new JsSet(_engine, resultData); } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.issubsetof + /// [JsFunction] private JsBoolean IsSubsetOf(JsValue thisObject, JsValue other) { @@ -335,39 +384,49 @@ private JsBoolean IsSubsetOf(JsValue thisObject, JsValue other) if (other is JsSet otherSet) { // fast path - return set._set._set.IsSubsetOf(otherSet._set._set) ? JsBoolean.True : JsBoolean.False; + if (set.Size > otherSet.Size) + { + return JsBoolean.False; + } + + var enumerator = set._data.GetEnumerator(); + while (enumerator.MoveNext()) + { + if (!otherSet._data.ContainsKey(enumerator.Key)) + { + return JsBoolean.False; + } + } + + return JsBoolean.True; } var otherRec = GetSetRecord(other); - var thisSize = set.Size; - if (thisSize > otherRec.Size) + if (set.Size > otherRec.Size) { return JsBoolean.False; } - var index = 0; var args = new JsValue[1]; - while (index < thisSize) + var cursor = default(KeyedCollectionCursor); + int slot; + while ((slot = set._data.Next(ref cursor)) >= 0) { - var e = set[index]; - if (e is not null) + args[0] = set._data.KeyAt(slot)!; + var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); + if (!inOther) { - args[0] = e; - var inOther = TypeConverter.ToBoolean(otherRec.Has.Call(otherRec.Set, args)); - if (!inOther) - { - return JsBoolean.False; - } + return JsBoolean.False; } - - thisSize = set.Size; - index++; } return JsBoolean.True; } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.issupersetof + /// [JsFunction] private JsBoolean IsSupersetOf(JsValue thisObject, JsValue other) { @@ -376,13 +435,29 @@ private JsBoolean IsSupersetOf(JsValue thisObject, JsValue other) if (other is JsSet otherSet) { // fast path - return set._set._set.IsSupersetOf(otherSet._set._set) ? JsBoolean.True : JsBoolean.False; + if (set.Size < otherSet.Size) + { + return JsBoolean.False; + } + + var enumerator = otherSet._data.GetEnumerator(); + while (enumerator.MoveNext()) + { + if (!set._data.ContainsKey(enumerator.Key)) + { + return JsBoolean.False; + } + } + + return JsBoolean.True; } - var thisSize = set.Size; var otherRec = GetSetRecord(other); - if (thisSize < otherRec.Size) + // Step 4 reads the receiver's size *after* GetSetRecord, and the difference is observable: + // reading it first meant a set-like whose `size`, `has` or `keys` getter adds to the receiver + // was compared against a size taken before its own getters had run. + if (set.Size < otherRec.Size) { return JsBoolean.False; } @@ -439,13 +514,16 @@ private JsValue ForEach(JsValue thisObject, JsValue callbackfn, JsValue thisArg) return Undefined; } + /// + /// https://tc39.es/ecma262/#sec-set.prototype.union + /// [JsFunction] private JsSet Union(JsValue thisObject, JsValue other) { var set = AssertSetInstance(thisObject); var otherRec = GetSetRecord(other); var keysIter = otherRec.Set.GetIteratorFromMethod(_realm, otherRec.Keys); - var resultSetData = set._set.Clone(); + var resultData = set._data.Clone(); while (keysIter.TryIteratorStep(out var next)) { var nextValue = next.Get(CommonProperties.Value); @@ -453,12 +531,16 @@ private JsSet Union(JsValue thisObject, JsValue other) { nextValue = JsNumber.PositiveZero; } - // this is the one adder that reaches the ordering set directly rather than through - // JsSet.Add, so it has to flatten a concatenated key itself - resultSetData.Add(SameValueZeroComparer.ToStableKey(nextValue)); + + if (!resultData.ContainsKey(nextValue)) + { + // this is the one adder that reaches the ordering set directly rather than through + // JsSet.Add, so it has to flatten a concatenated key itself + resultData.Append(SameValueZeroComparer.ToStableKey(nextValue), null); + } } - var result = new JsSet(_engine, resultSetData); + var result = new JsSet(_engine, resultData); return result; } diff --git a/Jint/Runtime/JintOrderedDictionary.cs b/Jint/Runtime/JintOrderedDictionary.cs deleted file mode 100644 index ebfc1bb8cc..0000000000 --- a/Jint/Runtime/JintOrderedDictionary.cs +++ /dev/null @@ -1,585 +0,0 @@ -#pragma warning disable CA1863 // Cache a 'CompositeFormat' for repeated use in this formatting operation - -#nullable disable - -// based on https://github.com/jehugaleahsa/truncon.collections.OrderedDictionary -// https://github.com/jehugaleahsa/truncon.collections.OrderedDictionary/blob/master/UNLICENSE.txt - - -using System.Collections; -using System.ComponentModel; -using System.Diagnostics; -using System.Globalization; - -namespace Jint.Runtime; - -/// -/// Represents a dictionary that tracks the order that items were added. -/// -/// The type of the dictionary keys. -/// The type of the dictionary values. -/// -/// This dictionary makes it possible to get the index of a key and a key based on an index. -/// It can be costly to find the index of a key because it must be searched for linearly. -/// It can be costly to insert a key/value pair because other key's indexes must be adjusted. -/// It can be costly to remove a key/value pair because other keys' indexes must be adjusted. -/// -[DebuggerDisplay("Count = {Count}")] -internal sealed class JintOrderedDictionary - : IDictionary, IList> where TKey : class where TValue : class -{ - private readonly Dictionary dictionary; - private readonly List keys; - - private const string ArrayTooSmall = "The given array was too small to hold the items."; - private const string EditReadOnlyList = "An attempt was made to edit a read-only list."; - private const string IndexOutOfRange = "The index is negative or outside the bounds of the collection."; - private const string TooSmall = "The given value cannot be less than {0}."; - - /// - /// Initializes a new instance of an OrderedDictionary. - /// - public JintOrderedDictionary() - { - dictionary = new Dictionary(); - keys = new List(); - } - - /// - /// Initializes a new instance of an OrderedDictionary. - /// - /// The initial capacity of the dictionary. - /// The capacity is less than zero. - public JintOrderedDictionary(int capacity) - { - dictionary = new Dictionary(capacity); - keys = new List(capacity); - } - - /// - /// Initializes a new instance of an OrderedDictionary. - /// - /// The equality comparer to use to compare keys. - public JintOrderedDictionary(IEqualityComparer comparer) - { - dictionary = new Dictionary(comparer); - keys = new List(); - } - - /// - /// Initializes a new instance of an OrderedDictionary. - /// - /// The initial capacity of the dictionary. - /// The equality comparer to use to compare keys. - public JintOrderedDictionary(int capacity, IEqualityComparer comparer) - { - dictionary = new Dictionary(capacity, comparer); - keys = new List(capacity); - } - - /// - /// Adds the given key/value pair to the dictionary. - /// - /// The key to add to the dictionary. - /// The value to associated with the key. - /// The given key already exists in the dictionary. - /// The key is null. - public void Add(TKey key, TValue value) - { - dictionary.Add(key, value); - keys.Add(key); - } - - /// - /// Inserts the given key/value pair at the specified index. - /// - /// The index to insert the key/value pair. - /// The key to insert. - /// The value to insert. - /// The given key already exists in the dictionary. - /// The key is null. - /// The index is negative -or- larger than the size of the dictionary. - public void Insert(int index, TKey key, TValue value) - { - if (index < 0 || index > dictionary.Count) - { - Throw.ArgumentOutOfRangeException(nameof(index), IndexOutOfRange); - } - dictionary.Add(key, value); - keys.Insert(index, key); - } - - /// - /// Determines whether the given key exists in the dictionary. - /// - /// The key to look for. - /// True if the key exists in the dictionary; otherwise, false. - /// The key is null. - public bool ContainsKey(TKey key) - { - return dictionary.ContainsKey(key); - } - - /// - /// Gets the key at the given index. - /// - /// The index of the key to get. - /// The key at the given index. - /// The index is negative -or- larger than the number of keys. - public TKey GetKey(int index) - { - return keys[index]; - } - - /// - /// Gets the index of the given key. - /// - /// The key to get the index of. - /// The index of the key in the dictionary -or- -1 if the key is not found. - /// The operation runs in O(n). - public int IndexOf(TKey key) - { - if (!dictionary.ContainsKey(key)) - { - return -1; - } - - var keysCount = keys.Count; - for (int i = 0; i < keysCount; ++i) - { - if (dictionary.Comparer.Equals(keys[i], key)) - { - return i; - } - } - return -1; - } - - /// - /// Gets the keys in the dictionary in the order they were added. - /// - public KeyCollection Keys => new KeyCollection(this); - - /// - /// Removes the key/value pair with the given key from the dictionary. - /// - /// The key of the pair to remove. - /// True if the key was found and the pair removed; otherwise, false. - /// The key is null. - /// This operation runs in O(n). - public bool Remove(TKey key) - { - if (dictionary.Remove(key)) - { - var keysCount = keys.Count; - for (int i = 0; i < keysCount; ++i) - { - if (dictionary.Comparer.Equals(keys[i], key)) - { - keys.RemoveAt(i); - break; - } - } - return true; - } - return false; - } - - /// - /// Removes the key/value pair at the given index. - /// - /// The index of the key/value pair to remove. - /// The index is negative -or- larger than the size of the dictionary. - /// This operation runs in O(n). - public void RemoveAt(int index) - { - TKey key = keys[index]; - dictionary.Remove(key); - keys.RemoveAt(index); - } - - /// - /// Tries to get the value associated with the given key. If the key is not found, - /// default(TValue) value is stored in the value. - /// - /// The key to get the value for. - /// The value used to hold the results. - /// True if the key was found; otherwise, false. - /// The key is null. - public bool TryGetValue(TKey key, out TValue value) - { - return dictionary.TryGetValue(key, out value); - } - - /// - /// Gets the values in the dictionary. - /// - public ValueCollection Values => new ValueCollection(this); - - /// - /// Gets or sets the value at the given index. - /// - /// The index of the value to get. - /// The value at the given index. - /// The index is negative -or- beyond the length of the dictionary. - public TValue this[int index] - { - get => dictionary[keys[index]]; - set => dictionary[keys[index]] = value; - } - - /// - /// Gets or sets the value associated with the given key. - /// - /// The key to get the associated value by or to associate with the value. - /// The value associated with the given key. - /// The key is null. - /// The key is not in the dictionary. - public TValue this[TKey key] - { - get => dictionary[key]; - set - { - if (!dictionary.ContainsKey(key)) - { - keys.Add(key); - } - dictionary[key] = value; - } - } - - /// - /// Removes all key/value pairs from the dictionary. - /// - public void Clear() - { - dictionary.Clear(); - keys.Clear(); - } - - /// - /// Gets the number of key/value pairs in the dictionary. - /// - public int Count => dictionary.Count; - - /// - /// Gets the key/value pairs in the dictionary in the order they were added. - /// - /// An enumerator over the key/value pairs in the dictionary. - public IEnumerator> GetEnumerator() - { - foreach (TKey key in keys) - { - yield return new KeyValuePair(key, dictionary[key]); - } - } - - int IList>.IndexOf(KeyValuePair item) - { - if (!dictionary.TryGetValue(item.Key, out var value)) - { - return -1; - } - if (!Equals(item.Value, value)) - { - return -1; - } - - var keysCount = keys.Count; - for (int i = 0; i < keysCount; ++i) - { - if (dictionary.Comparer.Equals(keys[i], item.Key)) - { - return i; - } - } - - return -1; - } - - void IList>.Insert(int index, KeyValuePair item) - { - if (index < 0 || index > dictionary.Count) - { - Throw.ArgumentOutOfRangeException(nameof(index), IndexOutOfRange); - } - dictionary.Add(item.Key, item.Value); - keys.Insert(index, item.Key); - } - - KeyValuePair IList>.this[int index] - { - get - { - TKey key = keys[index]; - TValue value = dictionary[key]; - return new KeyValuePair(key, value); - } - set - { - TKey key = keys[index]; - if (dictionary.Comparer.Equals(key, value.Key)) - { - dictionary[value.Key] = value.Value; - } - else - { - dictionary.Add(value.Key, value.Value); - dictionary.Remove(key); - keys[index] = value.Key; - } - } - } - - ICollection IDictionary.Keys => Keys; - - ICollection IDictionary.Values => Values; - - void ICollection>.Add(KeyValuePair item) - { - dictionary.Add(item.Key, item.Value); - keys.Add(item.Key); - } - - bool ICollection>.Contains(KeyValuePair item) - { - if (!dictionary.TryGetValue(item.Key, out var value)) - { - return false; - } - return Equals(value, item.Value); - } - - void ICollection>.CopyTo(KeyValuePair[] array, int arrayIndex) - { - if (array == null) - { - Throw.ArgumentNullException(nameof(array)); - return; - } - if (arrayIndex < 0) - { - Throw.ArgumentOutOfRangeException(nameof(arrayIndex), string.Format(CultureInfo.InvariantCulture, TooSmall, 0)); - } - if (dictionary.Count > array.Length - arrayIndex) - { - Throw.ArgumentException(ArrayTooSmall, nameof(array)); - } - foreach (TKey key in keys) - { - TValue value = dictionary[key]; - array[arrayIndex] = new KeyValuePair(key, value); - ++arrayIndex; - } - } - - bool ICollection>.IsReadOnly => false; - - bool ICollection>.Remove(KeyValuePair item) - { - if (!dictionary.TryGetValue(item.Key, out var value)) - { - return false; - } - if (!Equals(item.Value, value)) - { - return false; - } - // O(n) - dictionary.Remove(item.Key); - - var keysCount = keys.Count; - for (int i = 0; i < keysCount; ++i) - { - if (dictionary.Comparer.Equals(keys[i], item.Key)) - { - keys.RemoveAt(i); - } - } - return true; - } - - IEnumerator IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } - - /// - /// Wraps the keys in an OrderDictionary. - /// - public sealed class KeyCollection : ICollection - { - private readonly JintOrderedDictionary parent; - - /// - /// Initializes a new instance of a KeyCollection. - /// - /// The OrderedDictionary whose keys to wrap. - /// The dictionary is null. - public KeyCollection(JintOrderedDictionary dictionary) - { - parent = dictionary; - } - - /// - /// Copies the keys from the OrderedDictionary to the given array, starting at the given index. - /// - /// The array to copy the keys to. - /// The index into the array to start copying the keys. - /// The array is null. - /// The arrayIndex is negative. - /// The array, starting at the given index, is not large enough to contain all the keys. - public void CopyTo(TKey[] array, int arrayIndex) - { - parent.keys.CopyTo(array, arrayIndex); - } - - /// - /// Gets the number of keys in the OrderedDictionary. - /// - public int Count - { - get { return parent.dictionary.Count; } - } - - /// - /// Gets an enumerator over the keys in the OrderedDictionary. - /// - /// The enumerator. - public IEnumerator GetEnumerator() - { - return parent.keys.GetEnumerator(); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.Contains(TKey item) - { - return parent.dictionary.ContainsKey(item); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - void ICollection.Add(TKey item) - { - Throw.NotSupportedException(EditReadOnlyList); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - void ICollection.Clear() - { - Throw.NotSupportedException(EditReadOnlyList); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.IsReadOnly - { - get { return true; } - } - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.Remove(TKey item) - { - Throw.NotSupportedException(EditReadOnlyList); - return false; - } - - IEnumerator IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } - } - - /// - /// Wraps the keys in an OrderDictionary. - /// - public sealed class ValueCollection : ICollection - { - private readonly JintOrderedDictionary parent; - - /// - /// Initializes a new instance of a ValueCollection. - /// - /// The OrderedDictionary whose keys to wrap. - /// The dictionary is null. - public ValueCollection(JintOrderedDictionary dictionary) - { - parent = dictionary; - } - - /// - /// Copies the values from the OrderedDictionary to the given array, starting at the given index. - /// - /// The array to copy the values to. - /// The index into the array to start copying the values. - /// The array is null. - /// The arrayIndex is negative. - /// The array, starting at the given index, is not large enough to contain all the values. - public void CopyTo(TValue[] array, int arrayIndex) - { - if (arrayIndex < 0) - { - Throw.ArgumentOutOfRangeException(nameof(arrayIndex), string.Format(CultureInfo.InvariantCulture, TooSmall, 0)); - } - if (parent.dictionary.Count > array.Length - arrayIndex) - { - Throw.ArgumentException(ArrayTooSmall, nameof(array)); - } - foreach (TKey key in parent.keys) - { - TValue value = parent.dictionary[key]; - array[arrayIndex] = value; - ++arrayIndex; - } - } - - /// - /// Gets the number of values in the OrderedDictionary. - /// - public int Count => parent.dictionary.Count; - - /// - /// Gets an enumerator over the values in the OrderedDictionary. - /// - /// The enumerator. - public IEnumerator GetEnumerator() - { - foreach (TKey key in parent.keys) - { - TValue value = parent.dictionary[key]; - yield return value; - } - } - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.Contains(TValue item) - { - return parent.dictionary.ContainsValue(item); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - void ICollection.Add(TValue item) - { - Throw.NotSupportedException(EditReadOnlyList); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - void ICollection.Clear() - { - Throw.NotSupportedException(EditReadOnlyList); - } - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.IsReadOnly => true; - - [EditorBrowsable(EditorBrowsableState.Never)] - bool ICollection.Remove(TValue item) - { - Throw.NotSupportedException(EditReadOnlyList); - return false; - } - - IEnumerator IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } - } -} diff --git a/Jint/Runtime/KeyedCollectionData.cs b/Jint/Runtime/KeyedCollectionData.cs new file mode 100644 index 0000000000..e45d6b10c2 --- /dev/null +++ b/Jint/Runtime/KeyedCollectionData.cs @@ -0,0 +1,420 @@ +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using Jint.Native; + +namespace Jint.Runtime; + +/// +/// A resume point in a traversal. A default instance starts at the +/// beginning, so a traversal needs no explicit initialization. +/// +/// +/// is only meaningful while matches the collection's, which is +/// what makes a cursor survive a compaction: names the resume point in terms the +/// entries carry themselves, so it stays exact however the slots move underneath. +/// +[StructLayout(LayoutKind.Auto)] +internal struct KeyedCollectionCursor +{ + /// The slot to resume examining at. + internal int Slot; + + /// The collection's compaction epoch was computed against. + internal int Epoch; + + /// The sequence number the next entry visited must be at or after. + internal long NextSeq; +} + +/// +/// The ordered entry list behind a Map's [[MapData]] and a Set's [[SetData]] +/// (https://tc39.es/ecma262/#sec-map-objects, https://tc39.es/ecma262/#sec-set-objects), holding the +/// spec's ~empty~ tombstone directly. +/// +/// +/// +/// Every keyed-collection traversal the spec defines — forEach, the Map and Set iterators, and +/// the index-walking half of difference, intersection, isDisjointFrom and +/// isSubsetOf — walks the entry List by index while user code is free to mutate the +/// collection between two steps. The spec keeps that coherent by never removing an element: a deleted +/// entry becomes ~empty~ in place, so every surviving entry keeps its index and a suspended +/// traversal resumes exactly where it left off. Compacting on delete instead — which is what this type +/// used to do — shifts later entries left under a live cursor, which is how a traversal came to skip +/// an entry or visit one twice. +/// +/// +/// Storage is an append-only array of slots plus a from key to +/// slot, so membership, lookup and delete are all O(1); the delete previously cost a linear +/// scan plus a linear shift. Deleted slots are reclaimed at two points, both of which are invisible to +/// a traversal in flight (see ): trailing tombstones are dropped as they appear, and +/// an append that finds the array full compacts in place when at least half the slots are dead rather +/// than doubling. So an add/delete churn loop cannot grow the List without bound — the +/// slot count stays within a constant factor of the live count — while an append still amortizes to +/// O(1). +/// +/// +/// Both reclaim paths move live entries, so both bump the compaction epoch; a cursor whose epoch is +/// stale re-derives its slot from by binary search. Entry +/// sequence numbers are strictly increasing across slots, are never renumbered, and are 64-bit +/// precisely so that a suspended iterator cannot be defeated by a counter wrapping. +/// +/// +internal sealed class KeyedCollectionData +{ + [StructLayout(LayoutKind.Auto)] + private struct Entry + { + /// The entry's key, or for the spec's ~empty~. + internal JsValue? Key; + + /// The entry's value. Always when the collection backs a Set. + internal JsValue? Value; + + internal long Seq; + } + + private static readonly Entry[] _emptyEntries = []; + + private Entry[] _entries; + private int _slotCount; + private int _tombstones; + private long _nextSeq; + private int _epoch; + private readonly Dictionary _index; + + internal KeyedCollectionData() : this(0) + { + } + + internal KeyedCollectionData(int capacity) + { + _entries = capacity > 0 ? new Entry[capacity] : _emptyEntries; + _index = new Dictionary(capacity, SameValueZeroComparer.Instance); + } + + /// The number of live entries — the spec's SetDataSize. + internal int Count => _index.Count; + + /// + /// The number of elements in the spec's List, tombstones included. This is the bound every + /// index-walking algorithm in the spec re-reads as "the number of elements in set.[[SetData]]". + /// + internal int SlotCount => _slotCount; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal bool ContainsKey(JsValue key) => _index.ContainsKey(key); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal bool TryGetValue(JsValue key, [NotNullWhen(true)] out JsValue? value) + { + if (_index.TryGetValue(key, out var slot)) + { + value = _entries[slot].Value!; + return true; + } + + value = null; + return false; + } + + /// The key at , or when it is a tombstone. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal JsValue? KeyAt(int slot) => _entries[slot].Key; + + /// The value at , which must not be a tombstone. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal JsValue ValueAt(int slot) => _entries[slot].Value!; + + /// The spec's SetDataIndex: the slot holding , or -1. + internal int IndexOf(JsValue key) => _index.TryGetValue(key, out var slot) ? slot : -1; + + /// + /// Appends when it is absent, or overwrites the value of the existing + /// entry — which per https://tc39.es/ecma262/#sec-map.prototype.set leaves it where it is. + /// + /// when a new entry was appended. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal bool Set(JsValue key, JsValue? value) + { + if (_index.TryGetValue(key, out var slot)) + { + _entries[slot].Value = value; + return false; + } + + Append(key, value); + return true; + } + + /// Appends unless it is already present. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void Add(JsValue key) + { + if (!_index.ContainsKey(key)) + { + Append(key, null); + } + } + + /// Appends without checking for an existing entry. The caller must know the key is absent. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void Append(JsValue key, JsValue? value) + { + if (_slotCount == _entries.Length) + { + EnsureRoom(); + } + + ref var entry = ref _entries[_slotCount]; + entry.Key = key; + entry.Value = value; + entry.Seq = _nextSeq++; + _index[key] = _slotCount; + _slotCount++; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal bool Remove(JsValue key) + { + if (!_index.TryGetValue(key, out var slot)) + { + return false; + } + + _index.Remove(key); + Tombstone(slot); + return true; + } + + /// + /// Replaces the entry at with the spec's ~empty~. This is the + /// "Set resultSetData[index] to ~empty~" step of difference and symmetricDifference. + /// + internal void RemoveAt(int slot) + { + var key = _entries[slot].Key; + if (key is null) + { + return; + } + + _index.Remove(key); + Tombstone(slot); + } + + private void Tombstone(int slot) + { + _entries[slot].Key = null; + _entries[slot].Value = null; + _tombstones++; + + if (slot != _slotCount - 1) + { + return; + } + + // Trailing tombstones are free to drop: nothing can be appended into them, so no cursor can + // ever need to resume inside the range. A cursor parked past the new end is handled by the + // epoch bump below, which sends it back through NextSeq — without it, an entry appended after + // the truncation would land at a slot the cursor has already passed and go unvisited. + var entries = _entries; + var count = _slotCount; + do + { + count--; + entries[count] = default; + _tombstones--; + } while (count > 0 && entries[count - 1].Key is null); + + _slotCount = count; + _epoch++; + } + + /// + /// https://tc39.es/ecma262/#sec-set.prototype.clear — every entry becomes ~empty~. + /// + /// + /// The spec preserves the List itself "because there may be existing Set Iterator objects that are + /// suspended midway through iterating over that List", but a List of nothing but tombstones is + /// indistinguishable from an empty one: a suspended cursor resumes by sequence number, every entry + /// appended afterwards has a higher one than anything it has visited, and the spec's own index walk + /// would reach exactly those entries after skipping the tombstones. So the slots are dropped, which + /// is the "physically removing the entry from internal data structures" the delete note allows. + /// + internal void Clear() + { + if (_slotCount == 0) + { + return; + } + + Array.Clear(_entries, 0, _slotCount); + _slotCount = 0; + _tombstones = 0; + _index.Clear(); + _epoch++; + } + + /// + /// A copy holding the live entries in order — the spec's "let resultSetData be a copy of + /// set.[[SetData]]". Tombstones are not carried over; no algorithm that takes such a copy can tell, + /// because each one either only appends to it or walks it skipping ~empty~ anyway. + /// + internal KeyedCollectionData Clone() + { + var clone = new KeyedCollectionData(Count); + var entries = _entries; + for (var i = 0; i < _slotCount; i++) + { + var key = entries[i].Key; + if (key is not null) + { + clone.Append(key, entries[i].Value); + } + } + + return clone; + } + + /// + /// Drops every tombstone. Only safe on a collection no traversal can be suspended in — a result + /// object a Set method is about to hand back and nothing else has seen yet. + /// + internal void TrimTombstones() + { + if (_tombstones > 0) + { + Compact(); + } + } + + /// + /// Advances to the next live entry and returns its slot, or -1 when the + /// List is exhausted. Re-reads the slot count on every call, so an entry appended by user code + /// running between two steps is visited, exactly as "Set entriesCount to the number of elements in + /// entries" requires. + /// + internal int Next(ref KeyedCollectionCursor cursor) + { + if (cursor.Epoch != _epoch) + { + cursor.Slot = FindSlot(cursor.NextSeq); + cursor.Epoch = _epoch; + } + + var entries = _entries; + var slotCount = _slotCount; + for (var i = cursor.Slot; i < slotCount; i++) + { + var key = entries[i].Key; + if (key is not null) + { + cursor.Slot = i + 1; + cursor.NextSeq = entries[i].Seq + 1; + return i; + } + } + + cursor.Slot = slotCount; + return -1; + } + + /// + /// The first slot whose sequence number is at least , or . + /// Sequence numbers increase strictly across slots — entries are only ever appended, and compaction + /// preserves both their order and their numbers — so a plain binary search is exact. + /// + private int FindSlot(long seq) + { + var entries = _entries; + var lo = 0; + var hi = _slotCount; + while (lo < hi) + { + var mid = (int) (((uint) lo + (uint) hi) >> 1); + if (entries[mid].Seq < seq) + { + lo = mid + 1; + } + else + { + hi = mid; + } + } + + return lo; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private void EnsureRoom() + { + // Reclaiming rather than doubling once half the slots are dead is what bounds the List: the + // array can only grow while live entries occupy more than half of it, so the slot count stays + // within a constant factor of the live count however hard a script churns add/delete. + if (_tombstones > 0 && _tombstones >= _slotCount / 2) + { + Compact(); + if (_slotCount < _entries.Length) + { + return; + } + } + + Array.Resize(ref _entries, _entries.Length == 0 ? 4 : _entries.Length * 2); + } + + private void Compact() + { + var entries = _entries; + var target = 0; + for (var i = 0; i < _slotCount; i++) + { + var key = entries[i].Key; + if (key is null) + { + continue; + } + + if (target != i) + { + entries[target] = entries[i]; + _index[key] = target; + } + + target++; + } + + Array.Clear(entries, target, _slotCount - target); + _slotCount = target; + _tombstones = 0; + _epoch++; + } + + internal Enumerator GetEnumerator() => new Enumerator(this); + + /// Walks the live entries in order, and stays coherent if the collection is mutated meanwhile. + internal struct Enumerator + { + private readonly KeyedCollectionData _data; + private KeyedCollectionCursor _cursor; + private int _slot; + + internal Enumerator(KeyedCollectionData data) + { + _data = data; + _cursor = default; + _slot = -1; + } + + internal bool MoveNext() + { + _slot = _data.Next(ref _cursor); + return _slot >= 0; + } + + internal JsValue Key => _data.KeyAt(_slot)!; + + internal JsValue Value => _data.ValueAt(_slot); + } +} diff --git a/Jint/Runtime/OrderedSet.cs b/Jint/Runtime/OrderedSet.cs deleted file mode 100644 index f3877860dc..0000000000 --- a/Jint/Runtime/OrderedSet.cs +++ /dev/null @@ -1,105 +0,0 @@ -using System.Collections; - -namespace Jint.Runtime; - -internal sealed class OrderedSet : IEnumerable -{ - internal List _list; - internal HashSet _set; - - public OrderedSet(HashSet values) - { - _list = new List(values); - // carry the source comparer over: the copy has to keep answering equality the same way, - // otherwise a derived set silently falls back to default equality - _set = new HashSet(values, values.Comparer); - } - - public OrderedSet(IEqualityComparer comparer) - { - _list = []; - _set = new HashSet(comparer); - } - - public T this[int index] - { - get => _list[index]; - set - { - if (_set.Add(value)) - { - _list[index] = value; - } - } - } - - public OrderedSet Clone() - { - return new OrderedSet(EqualityComparer.Default) - { - _set = new HashSet(this._set, this._set.Comparer), - _list = [.. this._list] - }; - } - - public void Add(T item) - { - if (_set.Add(item)) - { - _list.Add(item); - } - } - - public void Clear() - { - _list.Clear(); - _set.Clear(); - } - - public bool Contains(T item) => _set.Contains(item); - - /// - /// Position of in insertion order, or -1. Scans with the set's own - /// comparer: would use the default one, which for - /// JsValue reports NaN as equal to nothing at all and so contradicts - /// for a value the set demonstrably holds. - /// - public int IndexOf(T item) - { - var comparer = _set.Comparer; - var list = _list; - for (var i = 0; i < list.Count; i++) - { - if (comparer.Equals(list[i], item)) - { - return i; - } - } - - return -1; - } - - public int Count => _list.Count; - - public bool Remove(T item) - { - if (!_set.Remove(item)) - { - return false; - } - - // List.Remove would compare with the default comparer, which can disagree with the set's - // one and leave the ordering list holding a value the set no longer has - var index = IndexOf(item); - if (index >= 0) - { - _list.RemoveAt(index); - } - - return true; - } - - public IEnumerator GetEnumerator() => _list.GetEnumerator(); - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); -}