ソースを参照

Cleanup custom dictionary implementations (#2036)

Marko Lahma 6 ヶ月 前
コミット
c87361eb49

+ 2 - 2
Jint.Benchmark/DictionaryBenchmark.cs

@@ -31,7 +31,7 @@ public class DictionaryBenchmark
         var hybridDictionary = new HybridDictionary<object>();
         for (var i = 0; i < N; i++)
         {
-            hybridDictionary.Add(_keys[i], _keys);
+            hybridDictionary[_keys[i]] = _keys;
         }
 
         foreach (var key in _keys)
@@ -69,4 +69,4 @@ public class DictionaryBenchmark
             dictionary.ContainsKey(key);
         }
     }
-}
+}

+ 37 - 0
Jint/Collections/DictionaryBase.cs

@@ -0,0 +1,37 @@
+using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
+
+namespace Jint.Collections;
+
+internal abstract class DictionaryBase<TValue> : IEngineDictionary<Key, TValue>
+{
+    public ref TValue this[Key key]
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        get => ref GetValueRefOrAddDefault(key, out _);
+    }
+
+    public bool TryGetValue(Key key, [NotNullWhen(true)] out TValue? value)
+    {
+        value = default;
+        ref var temp = ref GetValueRefOrNullRef(key);
+        if (Unsafe.IsNullRef(ref temp))
+        {
+            return false;
+        }
+
+        value = temp!;
+        return true;
+    }
+
+    public bool ContainsKey(Key key)
+    {
+        ref var valueRefOrNullRef = ref GetValueRefOrNullRef(key);
+        return !Unsafe.IsNullRef(ref valueRefOrNullRef);
+    }
+
+    public abstract int Count { get; }
+
+    public abstract ref TValue GetValueRefOrNullRef(Key key);
+    public abstract ref TValue GetValueRefOrAddDefault(Key key, out bool exists);
+}

+ 1 - 1
Jint/Collections/DictionarySlim.cs

@@ -18,7 +18,7 @@ namespace Jint.Collections;
 /// 3) It does not accept an equality comparer (assumes Object.GetHashCode() and Object.Equals() or overridden implementation are cheap and sufficient).
 /// </summary>
 [DebuggerDisplay("Count = {Count}")]
-internal class DictionarySlim<TKey, TValue> : IReadOnlyCollection<KeyValuePair<TKey, TValue>> where TKey : IEquatable<TKey>
+internal sealed class DictionarySlim<TKey, TValue> : IReadOnlyCollection<KeyValuePair<TKey, TValue>> where TKey : IEquatable<TKey>
 {
     // We want to initialize without allocating arrays. We also want to avoid null checks.
     // Array.Empty would give divide by zero in modulo operation. So we use static one element arrays.

+ 76 - 52
Jint/Collections/HybridDictionary.cs

@@ -5,7 +5,7 @@ using System.Runtime.CompilerServices;
 
 namespace Jint.Collections;
 
-internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
+internal sealed class HybridDictionary<TValue> : IEngineDictionary<Key, TValue>, IEnumerable<KeyValuePair<Key, TValue>>
 {
     private const int CutoverPoint = 9;
     private const int InitialDictionarySize = 13;
@@ -28,40 +28,34 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
         }
     }
 
-    protected HybridDictionary(StringDictionarySlim<TValue> dictionary)
+    public HybridDictionary(StringDictionarySlim<TValue> dictionary)
     {
         _checkExistingKeys = true;
         _dictionary = dictionary;
     }
 
-    public TValue this[Key key]
+    public ref TValue this[Key key]
     {
         get
-        {
-            TryGetValue(key, out var value);
-            return value;
-        }
-        set
         {
             if (_dictionary != null)
             {
-                _dictionary[key] = value;
+                return ref _dictionary[key];
             }
-            else if (_list != null)
+
+            if (_list != null)
             {
                 if (_list.Count >= CutoverPoint - 1)
                 {
-                    SwitchToDictionary(key, value, tryAdd: false);
+                    return ref SwitchToDictionary(key);
                 }
-                else
-                {
-                    _list[key] = value;
-                }
-            }
-            else
-            {
-                _list = new ListDictionary<TValue>(key, value, _checkExistingKeys);
+
+                return ref _list[key];
             }
+
+            var head = new ListDictionary<TValue>.DictionaryNode { Key = key, Value = default };
+            _list = new ListDictionary<TValue>(head, _checkExistingKeys);
+            return ref head.Value;
         }
     }
 
@@ -81,20 +75,48 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
         return false;
     }
 
-    public void SetOrUpdateValue<TState>(Key key, Func<TValue, TState, TValue> updater, TState state)
+    public ref TValue GetValueRefOrNullRef(Key key)
     {
         if (_dictionary != null)
         {
-            _dictionary.SetOrUpdateValue(key, updater, state);
+            return ref _dictionary.GetValueRefOrNullRef(key);
+        }
+
+        if (_list != null)
+        {
+            return ref _list.GetValueRefOrNullRef(key);
         }
-        else if (_list != null)
+
+        return ref Unsafe.NullRef<TValue>();
+    }
+
+    public ref TValue GetValueRefOrAddDefault(Key key, out bool exists)
+    {
+        if (_dictionary != null)
         {
-            _list.SetOrUpdateValue(key, updater, state);
+            return ref _dictionary.GetValueRefOrAddDefault(key, out exists);
         }
-        else
+
+        if (_list != null)
         {
-            _list = new ListDictionary<TValue>(key, updater(default, state), _checkExistingKeys);
+            return ref _list.GetValueRefOrAddDefault(key, out exists);
         }
+
+        var head = new ListDictionary<TValue>.DictionaryNode
+        {
+            Key = key,
+        };
+
+        _list = new ListDictionary<TValue>(head, _checkExistingKeys);
+        exists = false;
+        return ref head.Value;
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void SetOrUpdateValue<TState>(Key key, Func<TValue, TState, TValue> updater, TState state)
+    {
+        ref var currentValue = ref GetValueRefOrAddDefault(key, out _);
+        currentValue = updater(currentValue, state);
     }
 
     private bool SwitchToDictionary(Key key, TValue value, bool tryAdd)
@@ -115,11 +137,24 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
             dictionary[key] = value;
             result = true;
         }
+
         _dictionary = dictionary;
         _list = null;
         return result;
     }
 
+    private ref TValue SwitchToDictionary(Key key)
+    {
+        var dictionary = new StringDictionarySlim<TValue>(InitialDictionarySize);
+        foreach (var pair in _list)
+        {
+            dictionary[pair.Key] = pair.Value;
+        }
+        _dictionary = dictionary;
+        _list = null;
+        return ref dictionary[key];
+    }
+
     public int Count
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -132,26 +167,25 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
         {
             return _dictionary.TryAdd(key, value);
         }
-        else
-        {
-            _list ??= new ListDictionary<TValue>(key, value, _checkExistingKeys);
 
-            if (_list.Count + 1 >= CutoverPoint)
-            {
-                return SwitchToDictionary(key, value, tryAdd: true);
-            }
-            else
-            {
-                return _list.Add(key, value, tryAdd: true);
-            }
+        _list ??= new ListDictionary<TValue>(key, value, _checkExistingKeys);
+
+        if (_list.Count + 1 >= CutoverPoint)
+        {
+            return SwitchToDictionary(key, value, tryAdd: true);
         }
+
+        return _list.Add(key, value, tryAdd: true);
     }
 
-    public void Add(Key key, TValue value)
+    /// <summary>
+    /// Adds a new item and expects key to not exist.
+    /// </summary>
+    public void AddDangerous(Key key, TValue value)
     {
         if (_dictionary != null)
         {
-            _dictionary.GetOrAddValueRef(key) = value;
+            _dictionary.AddDangerous(key, value);
         }
         else
         {
@@ -163,11 +197,11 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
             {
                 if (_list.Count + 1 >= CutoverPoint)
                 {
-                    SwitchToDictionary(key, value, tryAdd: false);
+                    SwitchToDictionary(key) = value;
                 }
                 else
                 {
-                    _list.Add(key, value);
+                    _list.AddDangerous(key, value);
                 }
             }
         }
@@ -181,17 +215,8 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
 
     public bool ContainsKey(Key key)
     {
-        if (_dictionary != null)
-        {
-            return _dictionary.ContainsKey(key);
-        }
-
-        if (_list != null)
-        {
-            return _list.ContainsKey(key);
-        }
-
-        return false;
+        ref var valueRefOrNullRef = ref GetValueRefOrNullRef(key);
+        return !Unsafe.IsNullRef(ref valueRefOrNullRef);
     }
 
     IEnumerator<KeyValuePair<Key, TValue>> IEnumerable<KeyValuePair<Key, TValue>>.GetEnumerator()
@@ -207,7 +232,6 @@ internal class HybridDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
         }
 
         return System.Linq.Enumerable.Empty<KeyValuePair<Key, TValue>>().GetEnumerator();
-
     }
 
     IEnumerator IEnumerable.GetEnumerator()

+ 19 - 0
Jint/Collections/IEngineDictionary.cs

@@ -0,0 +1,19 @@
+using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
+
+// ReSharper disable once CheckNamespace
+namespace Jint;
+
+/// <summary>
+/// Contract for custom dictionaries that Jint uses.
+/// </summary>
+internal interface IEngineDictionary<in TKey, TValue>
+{
+    int Count { get; }
+
+    ref TValue this[TKey name] { get; }
+
+    public ref TValue GetValueRefOrNullRef(TKey key);
+
+    public ref TValue GetValueRefOrAddDefault(TKey key, out bool exists);
+}

+ 38 - 62
Jint/Collections/ListDictionary.cs

@@ -6,7 +6,7 @@ using Jint.Runtime;
 
 namespace Jint.Collections;
 
-internal sealed class ListDictionary<TValue> : IEnumerable<KeyValuePair<Key, TValue>>
+internal sealed class ListDictionary<TValue> : DictionaryBase<TValue>, IEnumerable<KeyValuePair<Key, TValue>>
 {
     private DictionaryNode _head;
     private int _count;
@@ -23,77 +23,49 @@ internal sealed class ListDictionary<TValue> : IEnumerable<KeyValuePair<Key, TVa
         _count = 1;
     }
 
-    public TValue this[Key key]
+    public ListDictionary(DictionaryNode head, bool checkExistingKeys)
     {
-        get
-        {
-            TryGetValue(key, out var value);
-            return value;
-        }
-        set
-        {
-            DictionaryNode last = null;
-            DictionaryNode node;
-            var checkExistingKeys = _checkExistingKeys;
-            for (node = _head; node != null; node = node.Next)
-            {
-                var oldKey = node.Key;
-                if (checkExistingKeys && oldKey == key)
-                {
-                    break;
-                }
-
-                last = node;
-            }
-
-            if (node != null)
-            {
-                // Found it
-                node.Value = value;
-                return;
-            }
-
-            AddNode(key, value, last);
-        }
+        _checkExistingKeys = checkExistingKeys;
+        _head = head;
+        _count = 1;
     }
 
-    public bool TryGetValue(Key key, out TValue value)
+    public override ref TValue GetValueRefOrNullRef(Key key)
     {
-        var node = _head;
-        while (node != null)
+        DictionaryNode node;
+        for (node = _head; node != null; node = node.Next)
         {
             if (node.Key == key)
             {
-                value = node.Value;
-                return true;
+                return ref node.Value;
             }
-
-            node = node.Next;
         }
 
-        value = default;
-        return false;
+        return ref Unsafe.NullRef<TValue>();
     }
 
-    public void SetOrUpdateValue<TState>(Key key, Func<TValue, TState, TValue> updater, TState state)
+    public override ref TValue GetValueRefOrAddDefault(Key key, out bool exists)
     {
         DictionaryNode last = null;
         DictionaryNode node;
+        var checkExistingKeys = _checkExistingKeys;
         for (node = _head; node != null; node = node.Next)
         {
-            if (node.Key == key)
+            var oldKey = node.Key;
+            if (checkExistingKeys && oldKey == key)
             {
-                node.Value = updater(node.Value, state);
-                return;
+                exists = true;
+                return ref node.Value;
             }
 
             last = node;
         }
 
-        AddNode(key, updater(default, state), last);
+        exists = false;
+        return ref AddNode(key, default, last).Value;
     }
 
-    public int Count
+    public override int Count
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         get => _count;
@@ -123,7 +95,24 @@ internal sealed class ListDictionary<TValue> : IEnumerable<KeyValuePair<Key, TVa
         return true;
     }
 
-    private void AddNode(Key key, TValue value, DictionaryNode last)
+    /// <summary>
+    /// Adds a new item and expects key to not exist.
+    /// </summary>
+    public void AddDangerous(Key key, TValue value)
+    {
+        DictionaryNode node;
+        for (node = _head; node != null; node = node.Next)
+        {
+            if (node.Next is null)
+            {
+                AddNode(key, value, node);
+                return;
+            }
+        }
+    }
+
+
+    private DictionaryNode AddNode(Key key, TValue value, DictionaryNode last)
     {
         var newNode = new DictionaryNode
         {
@@ -139,6 +128,7 @@ internal sealed class ListDictionary<TValue> : IEnumerable<KeyValuePair<Key, TVa
             last.Next = newNode;
         }
         _count++;
+        return newNode;
     }
 
     public void Clear()
@@ -147,20 +137,6 @@ internal sealed class ListDictionary<TValue> : IEnumerable<KeyValuePair<Key, TVa
         _head = null;
     }
 
-    public bool ContainsKey(Key key)
-    {
-        for (var node = _head; node != null; node = node.Next)
-        {
-            var oldKey = node.Key;
-            if (oldKey == key)
-            {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     internal bool CheckExistingKeys
     {
         set => _checkExistingKeys = value;

+ 0 - 18
Jint/Collections/PropertyDictionary.cs

@@ -1,18 +0,0 @@
-using Jint.Runtime.Descriptors;
-
-namespace Jint.Collections;
-
-internal sealed class PropertyDictionary : HybridDictionary<PropertyDescriptor>
-{
-    public PropertyDictionary()
-    {
-    }
-
-    public PropertyDictionary(int capacity, bool checkExistingKeys) : base(capacity, checkExistingKeys)
-    {
-    }
-
-    public PropertyDictionary(StringDictionarySlim<PropertyDescriptor> properties) : base(properties)
-    {
-    }
-}

+ 27 - 40
Jint/Collections/StringDictionarySlim.cs

@@ -22,7 +22,7 @@ namespace Jint.Collections;
 /// </summary>
 [DebuggerTypeProxy(typeof(DictionarySlimDebugView<>))]
 [DebuggerDisplay("Count = {Count}")]
-internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValuePair<Key, TValue>>
+internal sealed class StringDictionarySlim<TValue> : DictionaryBase<TValue>, IReadOnlyCollection<KeyValuePair<Key, TValue>>
 {
     // We want to initialize without allocating arrays. We also want to avoid null checks.
     // Array.Empty would give divide by zero in modulo operation. So we use static one element arrays.
@@ -62,7 +62,7 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
         _entries = new Entry[capacity];
     }
 
-    public int Count
+    public override int Count
     {
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         get => _count;
@@ -79,34 +79,11 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
         _entries = InitialEntries;
     }
 
-    public bool ContainsKey(Key key)
-    {
-        Entry[] entries = _entries;
-        for (int i = _buckets[key.HashCode & (_buckets.Length-1)] - 1;
-             (uint)i < (uint)entries.Length; i = entries[i].next)
-        {
-            if (key.Name == entries[i].key.Name)
-                return true;
-        }
-
-        return false;
-    }
-
-    public bool TryGetValue(Key key, out TValue value)
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void SetOrUpdateValue<TState>(Key key, Func<TValue, TState, TValue> updater, TState state)
     {
-        Entry[] entries = _entries;
-        for (int i = _buckets[key.HashCode & (_buckets.Length - 1)] - 1;
-             (uint)i < (uint)entries.Length; i = entries[i].next)
-        {
-            if (key.Name == entries[i].key.Name)
-            {
-                value = entries[i].value;
-                return true;
-            }
-        }
-
-        value = default;
-        return false;
+        ref var currentValue = ref GetValueRefOrAddDefault(key, out _);
+        currentValue = updater(currentValue, state);
     }
 
     public bool Remove(Key key)
@@ -145,12 +122,23 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
         return false;
     }
 
-    public void SetOrUpdateValue<TState>(Key key, Func<TValue, TState, TValue> updater, TState state)
+    public override ref TValue GetValueRefOrNullRef(Key key)
     {
-        ref var currentValue = ref GetOrAddValueRef(key);
-        currentValue = updater(currentValue, state);
+        Entry[] entries = _entries;
+        int bucketIndex = key.HashCode & (_buckets.Length - 1);
+        for (int i = _buckets[bucketIndex] - 1;
+             (uint)i < (uint)entries.Length; i = entries[i].next)
+        {
+            if (key.Name == entries[i].key.Name)
+            {
+                return ref entries[i].value;
+            }
+        }
+
+        return ref Unsafe.NullRef<TValue>();
     }
 
+
     // Not safe for concurrent _reads_ (at least, if either of them add)
     // For concurrent reads, prefer TryGetValue(key, out value)
     /// <summary>
@@ -159,8 +147,9 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
     /// add or update a value in a single look up operation.
     /// </summary>
     /// <param name="key">Key to look for</param>
+    /// <param name="exists">Whether the value existed</param>
     /// <returns>Reference to the new or existing value</returns>
-    public ref TValue GetOrAddValueRef(Key key)
+    public override ref TValue GetValueRefOrAddDefault(Key key, out bool exists)
     {
         Entry[] entries = _entries;
         int bucketIndex = key.HashCode & (_buckets.Length - 1);
@@ -168,9 +157,13 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
              (uint)i < (uint)entries.Length; i = entries[i].next)
         {
             if (key.Name == entries[i].key.Name)
+            {
+                exists = true;
                 return ref entries[i].value;
+            }
         }
 
+        exists = false;
         return ref AddKey(key, bucketIndex);
     }
 
@@ -192,7 +185,7 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
     }
 
     /// <summary>
-    /// Adds a new item and expects key to not to exist.
+    /// Adds a new item and expects key to not exist.
     /// </summary>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public void AddDangerous(in Key key, TValue value)
@@ -200,12 +193,6 @@ internal sealed class StringDictionarySlim<TValue> : IReadOnlyCollection<KeyValu
         AddKey(key, key.HashCode & (_buckets.Length - 1)) = value;
     }
 
-    public ref TValue this[Key key]
-    {
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        get => ref GetOrAddValueRef(key);
-    }
-
     [MethodImpl(MethodImplOptions.NoInlining)]
     private ref TValue AddKey(Key key, int bucketIndex)
     {

+ 0 - 15
Jint/Collections/SymbolDictionary.cs

@@ -1,15 +0,0 @@
-using Jint.Native;
-using Jint.Runtime.Descriptors;
-
-namespace Jint.Collections;
-
-internal sealed class SymbolDictionary : DictionarySlim<JsSymbol, PropertyDescriptor>
-{
-    public SymbolDictionary()
-    {
-    }
-
-    public SymbolDictionary(int capacity) : base(capacity)
-    {
-    }
-}

+ 2 - 0
Jint/GlobalUsings.cs

@@ -0,0 +1,2 @@
+global using PropertyDictionary = Jint.Collections.HybridDictionary<Jint.Runtime.Descriptors.PropertyDescriptor>;
+global using SymbolDictionary = Jint.Collections.DictionarySlim<Jint.Native.JsSymbol, Jint.Runtime.Descriptors.PropertyDescriptor>;