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(); -}