From dd9e0a4c52bffc76a8994d38cf7a92263f4ad1c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sat, 31 Dec 2022 11:49:46 -0500 Subject: [PATCH 1/4] Order-preserving CommonDictionaryStorage --- .../Runtime/CommonDictionaryStorage.cs | 648 ++++++------------ src/core/IronPython/Runtime/DictionaryOps.cs | 22 +- .../IronPython/Runtime/DictionaryStorage.cs | 4 +- .../Runtime/EmptyDictionaryStorage.cs | 6 +- .../IronPython/Runtime/PythonDictionary.cs | 9 +- tests/suite/test_ordered_dict_stdlib.py | 19 - 6 files changed, 216 insertions(+), 492 deletions(-) diff --git a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs index dba8f9af8..91a85dc83 100644 --- a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs +++ b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs @@ -7,16 +7,13 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.CompilerServices; -using System.Runtime.Serialization; using System.Threading; -using Microsoft.Scripting.Generation; -using Microsoft.Scripting.Runtime; -using Microsoft.Scripting.Utils; - using IronPython.Runtime.Operations; using IronPython.Runtime.Types; +using Microsoft.Scripting.Runtime; + namespace IronPython.Runtime { /// /// General purpose storage used for most PythonDictionarys. @@ -36,12 +33,14 @@ namespace IronPython.Runtime { /// the buckets and then calls a static helper function to do the read from the bucket /// array to ensure that readers are not seeing multiple bucket arrays. /// - [Serializable] - internal class CommonDictionaryStorage : DictionaryStorage, ISerializable, IDeserializationCallback { - protected Bucket[] _buckets; + internal sealed class CommonDictionaryStorage : DictionaryStorage { + private int[] _indices; + private List _buckets; private int _count; private int _version; - private NullValue _nullValue; // value stored in null bucket + + private const int FREE = -1; + private const int DUMMY = -2; private Func _hashFunc; private Func _eqFunc; @@ -58,9 +57,6 @@ internal class CommonDictionaryStorage : DictionaryStorage, ISerializable, IDese // marker type used to indicate we've gone megamorphic private static readonly Type HeterogeneousType = typeof(CommonDictionaryStorage); // a type we can never see here. - // Marker object used to indicate we have a removed value - private static readonly object _removed = new object(); - /// /// Creates a new dictionary storage with no buckets /// @@ -70,7 +66,9 @@ public CommonDictionaryStorage() { } /// Creates a new dictionary storage with buckets /// public CommonDictionaryStorage(int count) { - _buckets = new Bucket[(int)(count / Load + 2)]; + _indices = new int[(int)(count / Load + 2)]; + _indices.AsSpan().Fill(FREE); + _buckets = new List(); } /// @@ -100,47 +98,27 @@ public CommonDictionaryStorage(object[] items, bool isHomogeneous) } for (int i = 0; i < items.Length / 2; i++) { - object key = items[i * 2 + 1]; - if (key != null) { - AddOne(key, items[i * 2]); - } else { - AddNull(items[i * 2]); - } + AddOne(items[i * 2 + 1], items[i * 2]); } } - public int Version { - get { - return _version; - } - } + public int Version => _version; /// /// Creates a new dictionary storage with the given set of buckets /// and size. Used when cloning the dictionary storage. /// - private CommonDictionaryStorage(Bucket[] buckets, int count, Type keyType, Func hashFunc, Func eqFunc, NullValue nullValue) { + private CommonDictionaryStorage(int[] indices, List buckets, int count, Type keyType, Func hashFunc, Func eqFunc) { + _indices = indices; _buckets = buckets; _count = count; _keyType = keyType; _hashFunc = hashFunc; _eqFunc = eqFunc; - _nullValue = nullValue; } -#if FEATURE_SERIALIZATION - private CommonDictionaryStorage(SerializationInfo info, StreamingContext context) { - // remember the serialization info, we'll deserialize when we get the callback. This - // enables special types like DBNull.Value to successfully be deserialized inside of us. We - // store the serialization info in a special bucket so we don't have an extra field just for - // serialization - _nullValue = new DeserializationNullValue(info); - } -#endif - - public override void Add(ref DictionaryStorage storage, object key, object value) { - Add(key, value); - } + public override void Add(ref DictionaryStorage storage, object key, object value) + => Add(key, value); /// /// Adds a new item to the dictionary, replacing an existing one if it already exists. @@ -151,46 +129,38 @@ public void Add(object key, object value) { } } - private void AddNull(object value) { - if (_nullValue != null) { - _nullValue.Value = value; - } else { - _nullValue = new NullValue(value); - } - } - - public override void AddNoLock(ref DictionaryStorage storage, object key, object value) { - AddNoLock(key, value); - } + public override void AddNoLock(ref DictionaryStorage storage, object key, object value) + => AddNoLock(key, value); public void AddNoLock(object key, object value) { - if (key != null) { - if (_buckets == null) { - Initialize(); - } + if (_indices == null) { + Initialize(); + } - if (key.GetType() != _keyType && _keyType != HeterogeneousType) { + if (_keyType != HeterogeneousType) { + if (key is null) { + SetHeterogeneousSites(); + } else if (key.GetType() != _keyType) { UpdateHelperFunctions(key.GetType(), key); } - - AddOne(key, value); - } else { - AddNull(value); } + + AddOne(key, value); } private void AddOne(object key, object value) { - if (Add(_buckets, key, value)) { - _count++; - - if (_count >= (_buckets.Length * Load)) { + GetHash(key, out var hc, out var eqFunc); + if (AddWorker(_indices, _buckets, new Bucket(key, value, hc), eqFunc)) { + if (_count >= (_indices.Length * Load)) { // grow the hash table - EnsureSize((int)(_buckets.Length / Load) * ResizeMultiplier); + EnsureSize((int)(_indices.Length / Load) * ResizeMultiplier, eqFunc); } } } private void UpdateHelperFunctions(Type t, object key) { + Debug.Assert(_indices != null); + if (_keyType == null) { // first time through, get the sites for this specific type... if (t == typeof(int)) { @@ -226,7 +196,7 @@ private void UpdateHelperFunctions(Type t, object key) { // we need to clone the buckets so any lock-free readers will only see // the old buckets which are homogeneous - _buckets = (Bucket[])_buckets.Clone(); + _indices = (int[])_indices.Clone(); } // else we have already created a new site this dictionary } @@ -245,29 +215,33 @@ private void AssignSiteDelegates(CallSite> hashSite, _eqFunc = (o1, o2) => equalSite.Target(equalSite, o1, o2); } - private void EnsureSize(int newSize) { - if (_buckets.Length >= newSize) { + private void EnsureSize(int newSize, Func eqFunc) { + if (_indices.Length >= newSize) { return; } - Bucket[] oldBuckets = _buckets; - Bucket[] newBuckets = new Bucket[newSize]; + int[] newIndices = new int[newSize]; + newIndices.AsSpan().Fill(FREE); - for (int i = 0; i < oldBuckets.Length; i++) { - Bucket curBucket = oldBuckets[i]; - if (curBucket.Key != null && curBucket.Key != _removed) { - AddWorker(newBuckets, curBucket.Key, curBucket.Value, curBucket.HashCode); + // redo the indexing + for (var i = 0; i < _buckets.Count; i++) { + var bucket = _buckets[i]; + if (!bucket.IsRemoved) { + var pair = LookupIndex(newIndices, _buckets, bucket.Key, bucket.HashCode, eqFunc); + Debug.Assert(pair.Value < 0); + newIndices[pair.Key] = i; } } - _buckets = newBuckets; + _indices = newIndices; } public override void EnsureCapacityNoLock(int size) { - if (_buckets == null) { - _buckets = new Bucket[(int)(size / Load) + 1]; + if (_indices == null) { + _indices = new int[(int)(size / Load) + 1]; + _indices.AsSpan().Fill(FREE); } else { - EnsureSize((int)(size / Load)); + EnsureSize((int)(size / Load), _eqFunc); } } @@ -276,55 +250,43 @@ public override void EnsureCapacityNoLock(int size) { /// must check if the buckets are empty first. /// private void Initialize() { - _buckets = new Bucket[InitialBucketSize]; - } - - /// - /// Add helper that works over a single set of buckets. Used for - /// both the normal add case as well as the resize case. - /// - private bool Add(Bucket[] buckets, object key, object value) { - int hc = Hash(key); - - return AddWorker(buckets, key, value, hc); + _indices = new int[InitialBucketSize]; + _indices.AsSpan().Fill(FREE); + _buckets = new List(); } /// /// Add helper which adds the given key/value (where the key is not null) with /// a pre-computed hash code. /// - protected bool AddWorker(Bucket[] buckets, object/*!*/ key, object value, int hc) { - Debug.Assert(key != null); - - Debug.Assert(_count < buckets.Length); - int startIndex = hc % buckets.Length; + private static KeyValuePair LookupIndex(int[] indices, List buckets, object key, int hashCode, Func eqFunc) { + int startIndex = hashCode % indices.Length; // scan forward for matching key first int index = startIndex; int firstUsableIndex = -1; for (; ; ) { - Bucket cur = buckets[index]; - if (cur.Key == null) { + var idx = indices[index]; + if (idx == FREE) { // no entry was ever here, nothing more to probe - if (firstUsableIndex == -1) { - firstUsableIndex = index; - } - break; - } else if (cur.Key == _removed) { + return firstUsableIndex == -1 ? new KeyValuePair(index, FREE) : new KeyValuePair(firstUsableIndex, DUMMY); + } + if (idx == DUMMY) { // we recycled this bucket, so need to continue walking to see if a following bucket matches if (firstUsableIndex == -1) { // retain the index of the first recycled bucket, in case we need it later firstUsableIndex = index; } - } else if (Object.ReferenceEquals(key, cur.Key) || (cur.HashCode == hc && _eqFunc(key, cur.Key))) { - // this bucket is a key match - _version++; - buckets[index].Value = value; - return false; + } else { + Bucket cur = buckets[idx]; + if (object.ReferenceEquals(key, cur.Key) || (hashCode == cur.HashCode && eqFunc(key, cur.Key))) { + // this bucket is a key match + return new KeyValuePair(index, idx); + } } // keep walking - index = ProbeNext(buckets, index); + index = ProbeNext(indices, index); // if we ended up doing a full scan, then this means the key is not already in use and there are // only recycled buckets available -- nothing more to probe @@ -334,18 +296,31 @@ protected bool AddWorker(Bucket[] buckets, object/*!*/ key, object value, int hc } // the key wasn't found, but we did find a fresh or recycled (unused) bucket + return new KeyValuePair(firstUsableIndex, DUMMY); + } + + /// + /// Add helper which adds the given key/value with a pre-computed hash code. + /// + private bool AddWorker(int[] indices, List buckets, Bucket bucket, Func eqFunc) { + var pair = LookupIndex(indices, buckets, bucket.Key, bucket.HashCode, eqFunc); + if (pair.Value < 0) { + // the key wasn't found, but we did find a fresh or recycled (unused) bucket + indices[pair.Key] = buckets.Count; + buckets.Add(bucket); + _count++; + } else { + buckets[pair.Value] = bucket; + } _version++; - buckets[firstUsableIndex].HashCode = hc; - buckets[firstUsableIndex].Value = value; - buckets[firstUsableIndex].Key = key; return true; } - private static int ProbeNext(Bucket[] buckets, int index) { + private static int ProbeNext(int[] indices, int index) { // probe to next bucket index++; - if (index == buckets.Length) { + if (index == indices.Length) { index = 0; } return index; @@ -355,13 +330,11 @@ private static int ProbeNext(Bucket[] buckets, int index) { /// Removes an entry from the dictionary and returns true if the /// entry was removed or false. /// - public override bool Remove(ref DictionaryStorage storage, object key) { - return Remove(key); - } + public override bool Remove(ref DictionaryStorage storage, object key) + => Remove(key); - public bool Remove(object key) { - return TryRemoveValue(key, out _); - } + public bool Remove(object key) + => TryRemoveValue(key, out _); /// /// Removes an entry from the dictionary and returns true if the @@ -371,24 +344,15 @@ public bool Remove(object key) { /// internal bool RemoveAlwaysHash(object key) { lock (this) { - if (key == null) { - return TryRemoveNull(out _); - } - return TryRemoveNoLock(key, out _); } } - public override bool TryRemoveValue(ref DictionaryStorage storage, object key, out object value) { - return TryRemoveValue(key, out value); - } + public override bool TryRemoveValue(ref DictionaryStorage storage, object key, out object value) + => TryRemoveValue(key, out value); public bool TryRemoveValue(object key, out object value) { lock (this) { - if (key == null) { - return TryRemoveNull(out value); - } - if (_count == 0) { value = null; return false; @@ -398,23 +362,9 @@ public bool TryRemoveValue(object key, out object value) { } } - private bool TryRemoveNull(out object value) { - if (_nullValue != null) { - value = _nullValue.Value; - _nullValue = null; - return true; - } else { - value = null; - return false; - } - } - - private bool TryRemoveNoLock(object/*!*/ key, out object value) { - Debug.Assert(key != null); - + private void GetHash(object key, out int hc, out Func eqFunc) { Func hashFunc; - Func eqFunc; - if (key.GetType() == _keyType || _keyType == HeterogeneousType) { + if (_keyType == HeterogeneousType || key?.GetType() == _keyType) { hashFunc = _hashFunc; eqFunc = _eqFunc; } else { @@ -422,44 +372,33 @@ private bool TryRemoveNoLock(object/*!*/ key, out object value) { eqFunc = _genericEquals; } - int hc = hashFunc(key) & Int32.MaxValue; - - return TryRemoveNoLock(key, eqFunc, hc, out value); + hc = hashFunc(key) & int.MaxValue; } - protected bool TryRemoveNoLock(object/*!*/ key, Func eqFunc, int hc, out object value) { - Debug.Assert(key != null); + private bool TryRemoveNoLock(object key, out object value) { + GetHash(key, out var hc, out var eqFunc); + return TryRemoveNoLock(key, hc, eqFunc, out value); + } - if (_buckets == null) { + private bool TryRemoveNoLock(object key, int hc, Func eqFunc, out object value) { + if (_indices == null) { value = null; return false; } - int index = hc % _buckets.Length; - int startIndex = index; - do { - Bucket bucket = _buckets[index]; - if (bucket.Key == null) { - break; - } else if ( - Object.ReferenceEquals(key, bucket.Key) || - (bucket.Key != _removed && - bucket.HashCode == hc && - eqFunc(key, bucket.Key))) { - value = bucket.Value; - _version++; - _buckets[index].Key = _removed; - Thread.MemoryBarrier(); - _buckets[index].Value = null; - _count--; - - return true; - } + var pair = LookupIndex(_indices, _buckets, key, hc, eqFunc); + if (pair.Value < 0) { + value = null; + return false; + } - index = ProbeNext(_buckets, index); - } while (index != startIndex); - value = null; - return false; + value = _buckets[pair.Value].Value; + _version++; + _indices[pair.Key] = DUMMY; + _buckets[pair.Value] = Bucket.Removed; + Thread.MemoryBarrier(); + _count--; + return true; } /// @@ -482,20 +421,8 @@ public override bool Contains(object key) { /// Trys to get the value associated with the given key and returns true /// if it's found or false if it's not present. /// - public override bool TryGetValue(object key, out object value) { - if (key != null) { - return TryGetValue(_buckets, key, out value); - } - - NullValue nv = _nullValue; - if (nv != null) { - value = nv.Value; - return true; - } - - value = null; - return false; - } + public override bool TryGetValue(object key, out object value) + => TryGetValue(_indices, _buckets, key, out value); /// /// Static helper to try and get the value from the dictionary. @@ -503,96 +430,59 @@ public override bool TryGetValue(object key, out object value) { /// Used so the value lookup can run against a buckets while a writer /// replaces the buckets. /// - private bool TryGetValue(Bucket[] buckets, object/*!*/ key, out object value) { - Debug.Assert(key != null); - - if (_count > 0 && buckets != null) { - int hc; - Func eqFunc; - if (key.GetType() == _keyType || _keyType == HeterogeneousType) { - hc = _hashFunc(key) & Int32.MaxValue; - eqFunc = _eqFunc; - } else { - hc = _genericHash(key) & Int32.MaxValue; - eqFunc = _genericEquals; - } - - return TryGetValue(buckets, key, hc, eqFunc, out value); + private bool TryGetValue(int[] indices, List buckets, object key, out object value) { + if (_count > 0 && indices != null) { + GetHash(key, out var hc, out var eqFunc); + return TryGetValue(indices, buckets, key, hc, eqFunc, out value); } value = null; return false; } - protected static bool TryGetValue(Bucket[] buckets, object key, int hc, Func eqFunc, out object value) { - int index = hc % buckets.Length; - int startIndex = index; - do { - Bucket bucket = buckets[index]; - if (bucket.Key == null) { - break; - } else if ( - Object.ReferenceEquals(key, bucket.Key) || - (bucket.Key != _removed && - bucket.HashCode == hc && - eqFunc(key, bucket.Key))) { - value = bucket.Value; - return true; - } - - index = ProbeNext(buckets, index); - } while (startIndex != index); + private static bool TryGetValue(int[] indices, List buckets, object key, int hc, Func eqFunc, out object value) { + var pair = LookupIndex(indices, buckets, key, hc, eqFunc); + if (pair.Value < 0) { + value = null; + return false; + } - value = null; - return false; + value = buckets[pair.Value].Value; + return true; } /// /// Returns the number of key/value pairs currently in the dictionary. /// - public override int Count { - get { - int res = _count; - if (_nullValue != null) { - res++; - } - return res; - } - } + public override int Count => _count; /// /// Clears the contents of the dictionary. /// - public override void Clear(ref DictionaryStorage storage) { - Clear(); - } + public override void Clear(ref DictionaryStorage storage) => Clear(); public void Clear() { lock (this) { - if (_buckets != null) { + if (_indices != null) { _version++; - _buckets = new Bucket[8]; + _indices = new int[8]; + _indices.AsSpan().Fill(FREE); + _buckets.Clear(); _count = 0; } - _nullValue = null; } } public override List> GetItems() { lock (this) { - List> res = new List>(_count + (_nullValue != null ? 1 : 0)); + List> res = new List>(_count); if (_count > 0) { - for (int i = 0; i < _buckets.Length; i++) { - Bucket curBucket = _buckets[i]; - if (curBucket.Key != null && curBucket.Key != _removed) { - res.Add(new KeyValuePair(curBucket.Key, curBucket.Value)); + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved) { + res.Add(new KeyValuePair(bucket.Key, bucket.Value)); } } } - - if (_nullValue != null) { - res.Add(new KeyValuePair(null, _nullValue.Value)); - } return res; } } @@ -600,53 +490,35 @@ public override List> GetItems() { public override IEnumerator> GetEnumerator() { lock (this) { if (_count > 0) { - for (int i = 0; i < _buckets.Length; i++) { - Bucket curBucket = _buckets[i]; - if (curBucket.Key != null && curBucket.Key != _removed) { - yield return new KeyValuePair(curBucket.Key, curBucket.Value); + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved) { + yield return new KeyValuePair(bucket.Key, bucket.Value); } } } - - if (_nullValue != null) { - yield return new KeyValuePair(null, _nullValue.Value); - } } } public override IEnumerable/*!*/ GetKeys() { - Bucket[] buckets = _buckets; lock (this) { - object[] res = new object[Count]; + if (_count == 0) return Array.Empty(); + + object[] res = new object[_count]; int index = 0; - if (buckets != null) { - for (int i = 0; i < buckets.Length; i++) { - Bucket curBucket = buckets[i]; - if (curBucket.Key != null && curBucket.Key != _removed) { - res[index++] = curBucket.Key; - } + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved) { + res[index++] = bucket.Key; } } - - if (_nullValue != null) { - res[index++] = null; - } - return res; } } public override bool HasNonStringAttributes() { lock (this) { - var nullValue = _nullValue; - if (nullValue != null && !(nullValue.Value is string) && !(nullValue.Value is Extensible)) { - return true; - } if (_keyType != typeof(string) && _keyType != null && _count > 0 && _keyType != typeof(Extensible) && !_keyType.IsSubclassOf(typeof(Extensible))) { - for (int i = 0; i < _buckets.Length; i++) { - Bucket curBucket = _buckets[i]; - - if (curBucket.Key != null && curBucket.Key != _removed && !(curBucket.Key is string) && !(curBucket.Key is Extensible)) { + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved && bucket.Key is not string && bucket.Key is not Extensible) { return true; } } @@ -661,26 +533,14 @@ public override bool HasNonStringAttributes() { /// public override DictionaryStorage Clone() { lock (this) { - if (_buckets == null) { - if (_nullValue != null) { - return new CommonDictionaryStorage(null, 1, _keyType, _hashFunc, _eqFunc, new NullValue(_nullValue.Value)); - } - + if (_indices == null) { return new CommonDictionaryStorage(); } - Bucket[] resBuckets = new Bucket[_buckets.Length]; - for (int i = 0; i < _buckets.Length; i++) { - if (_buckets[i].Key != null) { - resBuckets[i] = _buckets[i]; - } - } + int[] resIndices = (int[])_indices.Clone(); + var resBuckets = new List(_buckets); - NullValue nv = null; - if (_nullValue != null) { - nv = new NullValue(_nullValue.Value); - } - return new CommonDictionaryStorage(resBuckets, _count, _keyType, _hashFunc, _eqFunc, nv); + return new CommonDictionaryStorage(resIndices, resBuckets, _count, _keyType, _hashFunc, _eqFunc); } } @@ -691,7 +551,7 @@ public override void CopyTo(ref DictionaryStorage/*!*/ into) { public DictionaryStorage CopyTo(DictionaryStorage into) { Debug.Assert(into != null); - if (_buckets != null) { + if (_indices != null) { using (new OrderedLocker(this, into)) { if (@into is CommonDictionaryStorage commonInto) { CommonCopyTo(commonInto); @@ -701,74 +561,62 @@ public DictionaryStorage CopyTo(DictionaryStorage into) { } } - var nullValue = _nullValue; - if (nullValue != null) { - into.Add(ref into, null, nullValue.Value); - } return into; - } - private void CommonCopyTo(CommonDictionaryStorage into) { - if (into._buckets == null) { - into._buckets = new Bucket[_buckets.Length]; - } else { - int curSize = into._buckets.Length; - int newSize = (int)((_count + into._count) / Load) + 2; - while (curSize < newSize) { - curSize *= ResizeMultiplier; + void CommonCopyTo(CommonDictionaryStorage into) { + if (into._indices == null) { + into._indices = new int[_indices.Length]; + into._indices.AsSpan().Fill(FREE); + into._buckets = new List(); + } else { + int curSize = into._indices.Length; + int newSize = (int)((_count + into._count) / Load) + 2; + while (curSize < newSize) { + curSize *= ResizeMultiplier; + } + into.EnsureSize(curSize, into._eqFunc); } - into.EnsureSize(curSize); - } - - if (into._keyType == null) { - into._keyType = _keyType; - into._hashFunc = _hashFunc; - into._eqFunc = _eqFunc; - } else if (into._keyType != _keyType) { - into.SetHeterogeneousSites(); - } - for (int i = 0; i < _buckets.Length; i++) { - Bucket curBucket = _buckets[i]; + if (into._keyType == null) { + into._keyType = _keyType; + into._hashFunc = _hashFunc; + into._eqFunc = _eqFunc; + } else if (into._keyType != _keyType) { + into.SetHeterogeneousSites(); + } - if (curBucket.Key != null && - curBucket.Key != _removed && - into.AddWorker(into._buckets, curBucket.Key, curBucket.Value, curBucket.HashCode)) { - into._count++; + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved) { + into.AddWorker(into._indices, into._buckets, bucket, into._eqFunc); + } } } - } - private void UncommonCopyTo(ref DictionaryStorage into) { - for (int i = 0; i < _buckets.Length; i++) { - Bucket curBucket = _buckets[i]; - if (curBucket.Key != null && curBucket.Key != _removed) { - into.AddNoLock(ref into, curBucket.Key, curBucket.Value); + void UncommonCopyTo(ref DictionaryStorage into) { + foreach (var bucket in _buckets) { + if (!bucket.IsRemoved) { + into.AddNoLock(ref into, bucket.Key, bucket.Value); + } } } } - /// - /// Helper to hash the given key w/ support for null. - /// - private int Hash(object key) { - if (key is string) return key.GetHashCode() & Int32.MaxValue; - - return _hashFunc(key) & Int32.MaxValue; - } - /// /// Used to store a single hashed key/value. /// /// Bucket is not serializable because it stores the computed hash /// code which could change between serialization and deserialization. /// - protected struct Bucket { - public object Key; // the key to be hashed - public object Value; // the value associated with the key - public int HashCode; // the hash code of the contained key. + private readonly struct Bucket { + public readonly object Key; // the key to be hashed + public readonly object Value; // the value associated with the key + public readonly int HashCode; // the hash code of the contained key. + + public static readonly Bucket Removed = new Bucket(new object(), null, 0); + + public bool IsRemoved => object.ReferenceEquals(Key, Removed.Key); - public Bucket(int hashCode, object key, object value) { + public Bucket(object key, object value, int hashCode) { HashCode = hashCode; Key = key; Value = value; @@ -777,125 +625,31 @@ public Bucket(int hashCode, object key, object value) { #region Hash/Equality Delegates - private static int PrimitiveHash(object o) { - return o.GetHashCode(); - } + private static int PrimitiveHash(object o) => o.GetHashCode(); - private static int IntHash(object o) { - return (int)o; - } + private static int IntHash(object o) => (int)o; - private static int DoubleHash(object o) { - return DoubleOps.__hash__((double)o); - } + private static int DoubleHash(object o) => DoubleOps.__hash__((double)o); - private static int GenericHash(object o) { - return PythonOps.Hash(DefaultContext.Default, o); - } + private static int GenericHash(object o) => PythonOps.Hash(DefaultContext.Default, o); - private static int TupleHash(object o) { - return ((IStructuralEquatable)o).GetHashCode( - DefaultContext.DefaultPythonContext.EqualityComparerNonGeneric - ); - } + private static int TupleHash(object o) + => ((IStructuralEquatable)o).GetHashCode(DefaultContext.DefaultPythonContext.EqualityComparerNonGeneric); - private static bool StringEquals(object o1, object o2) { - return (string)o1 == (string)o2; - } + private static bool StringEquals(object o1, object o2) => (string)o1 == (string)o2; private static bool IntEquals(object o1, object o2) { Debug.Assert(o1 is int && o2 is int); return (int)o1 == (int)o2; } - private static bool DoubleEquals(object o1, object o2) { - return (double)o1 == (double)o2; - } - - private static bool TupleEquals(object o1, object o2) { - return ((IStructuralEquatable)o1).Equals( - o2, DefaultContext.DefaultPythonContext.EqualityComparerNonGeneric - ); - } - - private static bool GenericEquals(object o1, object o2) { - return PythonOps.EqualRetBool(o1, o2); - } - - #endregion - - [Serializable] - private class NullValue { - public object Value; - - public NullValue(object value) { - Value = value; - } - } -#if FEATURE_SERIALIZATION - - /// - /// Special marker NullValue used during deserialization to not add - /// an extra field to the dictionary storage type. - /// - private class DeserializationNullValue : NullValue { - public SerializationInfo/*!*/ SerializationInfo { - get { - return (SerializationInfo)Value; - } - } - - public DeserializationNullValue(SerializationInfo info) - : base(info) { - } - } - - private DeserializationNullValue GetDeserializationBucket() { - return _nullValue as DeserializationNullValue; - } - - #region ISerializable Members - - public void GetObjectData(SerializationInfo info, StreamingContext context) { - info.AddValue("buckets", GetItems()); - info.AddValue("nullvalue", _nullValue); - } - - #endregion - - #region IDeserializationCallback Members - - void IDeserializationCallback.OnDeserialization(object sender) { - DeserializationNullValue bucket = GetDeserializationBucket(); - if (bucket == null) { - // we've received multiple OnDeserialization callbacks, only - // deserialize after the 1st one - return; - } - - SerializationInfo info = bucket.SerializationInfo; - _buckets = null; - _nullValue = null; + private static bool DoubleEquals(object o1, object o2) => (double)o1 == (double)o2; - var buckets = (List>)info.GetValue("buckets", typeof(List>)); + private static bool TupleEquals(object o1, object o2) + => ((IStructuralEquatable)o1).Equals(o2, DefaultContext.DefaultPythonContext.EqualityComparerNonGeneric); - foreach (KeyValuePair kvp in buckets) { - Add(kvp.Key, kvp.Value); - } - - NullValue nullVal = null; - try { - nullVal = (NullValue)info.GetValue("nullvalue", typeof(NullValue)); - } catch (SerializationException) { - // for compatibility with dictionary serialized in 2.6. - } - if (nullVal != null) { - _nullValue = new NullValue(nullVal); - } - } + private static bool GenericEquals(object o1, object o2) => PythonOps.EqualRetBool(o1, o2); #endregion -#endif } - } diff --git a/src/core/IronPython/Runtime/DictionaryOps.cs b/src/core/IronPython/Runtime/DictionaryOps.cs index dec342ce0..b1b804d3d 100644 --- a/src/core/IronPython/Runtime/DictionaryOps.cs +++ b/src/core/IronPython/Runtime/DictionaryOps.cs @@ -6,16 +6,14 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; -using System.Runtime.CompilerServices; +using System.Linq; using System.Text; -using Microsoft.Scripting; -using Microsoft.Scripting.Runtime; - -using IronPython.Runtime.Binding; using IronPython.Runtime.Operations; using IronPython.Runtime.Types; +using Microsoft.Scripting.Runtime; + namespace IronPython.Runtime { /// /// Provides both helpers for implementing Python dictionaries as well @@ -100,14 +98,14 @@ public static object pop(PythonDictionary self, object key, object defaultValue) } public static PythonTuple popitem(PythonDictionary self) { - using IEnumerator> ie = self.GetEnumerator(); - if (ie.MoveNext()) { - object key = ie.Current.Key; - object val = ie.Current.Value; - self.RemoveDirect(key); - return PythonTuple.MakeTuple(key, val); + try { + var pair = self._storage.GetItems().Last(); + self.RemoveDirect(pair.Key); + return PythonTuple.MakeTuple(pair.Key, pair.Value); + } + catch (InvalidOperationException) { + throw PythonOps.KeyError("dictionary is empty"); } - throw PythonOps.KeyError("dictionary is empty"); } public static object setdefault(PythonDictionary self, object key) { diff --git a/src/core/IronPython/Runtime/DictionaryStorage.cs b/src/core/IronPython/Runtime/DictionaryStorage.cs index 696da464f..b7c567a19 100644 --- a/src/core/IronPython/Runtime/DictionaryStorage.cs +++ b/src/core/IronPython/Runtime/DictionaryStorage.cs @@ -8,7 +8,6 @@ using System.Collections.Generic; using System.Diagnostics; -using Microsoft.Scripting; using Microsoft.Scripting.Runtime; namespace IronPython.Runtime { @@ -117,8 +116,7 @@ public virtual DictionaryStorage Clone() { return storage; } - public virtual void EnsureCapacityNoLock(int size) { - } + public virtual void EnsureCapacityNoLock(int size) { } public virtual IEnumerator> GetEnumerator() { return GetItems().GetEnumerator(); diff --git a/src/core/IronPython/Runtime/EmptyDictionaryStorage.cs b/src/core/IronPython/Runtime/EmptyDictionaryStorage.cs index b9b6ee061..e270c2cbb 100644 --- a/src/core/IronPython/Runtime/EmptyDictionaryStorage.cs +++ b/src/core/IronPython/Runtime/EmptyDictionaryStorage.cs @@ -17,8 +17,7 @@ namespace IronPython.Runtime { internal class EmptyDictionaryStorage : DictionaryStorage { public static EmptyDictionaryStorage Instance = new EmptyDictionaryStorage(); - private EmptyDictionaryStorage() { - } + private EmptyDictionaryStorage() { } public override void Add(ref DictionaryStorage storage, object? key, object? value) { lock (this) { @@ -49,8 +48,7 @@ public override DictionaryStorage AsMutable(ref DictionaryStorage storage) { return storage.AsMutable(ref storage); } - public override void Clear(ref DictionaryStorage storage) { - } + public override void Clear(ref DictionaryStorage storage) { } public override bool Contains(object? key) { // make sure argument is valid, do not calculate hash diff --git a/src/core/IronPython/Runtime/PythonDictionary.cs b/src/core/IronPython/Runtime/PythonDictionary.cs index c5bda3617..afd89ae9f 100644 --- a/src/core/IronPython/Runtime/PythonDictionary.cs +++ b/src/core/IronPython/Runtime/PythonDictionary.cs @@ -151,11 +151,7 @@ public bool TryGetValue(object key, out object value) { // we need to manually look up a slot to get the correct behavior when // the __missing__ function is declared on a sub-type which is an old-class if (GetType() != typeof(PythonDictionary) && - PythonTypeOps.TryInvokeBinaryOperator(DefaultContext.Default, - this, - key, - "__missing__", - out value)) { + PythonTypeOps.TryInvokeBinaryOperator(DefaultContext.Default, this, key, "__missing__", out value)) { return true; } @@ -353,8 +349,7 @@ public object setdefault(object key, object defaultValue) { public DictionaryValueView values() => new DictionaryValueView(this); [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1822:MarkMembersAsStatic")] - public void update() { - } + public void update() { } public void update(CodeContext/*!*/ context, [ParamDictionary] IDictionary other\u00F8) { DictionaryOps.update(context, this, other\u00F8); diff --git a/tests/suite/test_ordered_dict_stdlib.py b/tests/suite/test_ordered_dict_stdlib.py index 29d72ccca..cf6e4b74d 100644 --- a/tests/suite/test_ordered_dict_stdlib.py +++ b/tests/suite/test_ordered_dict_stdlib.py @@ -15,40 +15,21 @@ def load_tests(loader, standard_tests, pattern): if is_ironpython: failing_tests = [ - test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_468'), # https://github.com/IronLanguages/ironpython3/issues/1460 test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_copying'), # pickle.PicklingError test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_free_after_iterating'), # AssertionError test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_iterators_pickling'), # pickle.PicklingError test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_key_change_during_iteration'), # AssertionError: RuntimeError not raised test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_pickle_recursive'), # pickle.PicklingError - test.test_ordered_dict.CPythonOrderedDictTests('test_468'), # https://github.com/IronLanguages/ironpython3/issues/1460 test.test_ordered_dict.CPythonOrderedDictTests('test_free_after_iterating'), # AssertionError test.test_ordered_dict.CPythonOrderedDictTests('test_iterators_pickling'), # pickle.PicklingError test.test_ordered_dict.CPythonOrderedDictTests('test_key_change_during_iteration'), # AssertionError: RuntimeError not raised - test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_468'), # https://github.com/IronLanguages/ironpython3/issues/1460 test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_copying'), # pickle.PicklingError test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_free_after_iterating'), # AssertionError test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_pickle_recursive'), # pickle.PicklingError - test.test_ordered_dict.PurePythonOrderedDictTests('test_468'), # https://github.com/IronLanguages/ironpython3/issues/1460 test.test_ordered_dict.PurePythonOrderedDictTests('test_free_after_iterating'), # AssertionError ] skip_tests = [ - # https://github.com/IronLanguages/ironpython3/issues/1556 - test.test_ordered_dict.CPythonBuiltinDictTests('test_abc'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_clear'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_delitem'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_delitem_hash_collision'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_detect_deletion_during_iteration'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_highly_nested'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_highly_nested_subclass'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_init'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_override_update'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_popitem'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_reinsert'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_setitem'), - test.test_ordered_dict.CPythonBuiltinDictTests('test_update'), - test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? test.test_ordered_dict.CPythonOrderedDictTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? From 40f1755ce63de865bc468071f647e3cf4d50d1d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sat, 31 Dec 2022 23:13:05 -0500 Subject: [PATCH 2/4] Get some tests passing --- src/core/IronPython/Runtime/DictionaryOps.cs | 8 ++++-- .../Runtime/Operations/PythonOps.cs | 2 ++ .../IronPython/Runtime/PythonDictionary.cs | 5 ++++ .../Cases/CPythonCasesManifest.ini | 3 -- tests/suite/test_call_stdlib.py | 28 ------------------- tests/suite/test_functools_stdlib.py | 2 -- 6 files changed, 13 insertions(+), 35 deletions(-) delete mode 100644 tests/suite/test_call_stdlib.py diff --git a/src/core/IronPython/Runtime/DictionaryOps.cs b/src/core/IronPython/Runtime/DictionaryOps.cs index b1b804d3d..2b526456a 100644 --- a/src/core/IronPython/Runtime/DictionaryOps.cs +++ b/src/core/IronPython/Runtime/DictionaryOps.cs @@ -120,7 +120,11 @@ public static object setdefault(PythonDictionary self, object key, object defaul public static void update(CodeContext/*!*/ context, PythonDictionary/*!*/ self, object other) { if (other is PythonDictionary pyDict) { - pyDict._storage.CopyTo(ref self._storage); + if (pyDict.TreatAsMapping) { + SlowUpdate(context, self, other); + } else { + pyDict._storage.CopyTo(ref self._storage); + } } else { SlowUpdate(context, self, other); } @@ -129,7 +133,7 @@ public static void update(CodeContext/*!*/ context, PythonDictionary/*!*/ self, private static void SlowUpdate(CodeContext/*!*/ context, PythonDictionary/*!*/ self, object other) { if (other is MappingProxy proxy) { update(context, self, proxy.GetDictionary(context)); - } else if (other is IDictionary dict) { + } else if (other is IDictionary dict && other is not PythonDictionary) { IDictionaryEnumerator e = dict.GetEnumerator(); while (e.MoveNext()) { self._storage.Add(ref self._storage, e.Key, e.Value); diff --git a/src/core/IronPython/Runtime/Operations/PythonOps.cs b/src/core/IronPython/Runtime/Operations/PythonOps.cs index 82f0f75be..d39b6bbbe 100644 --- a/src/core/IronPython/Runtime/Operations/PythonOps.cs +++ b/src/core/IronPython/Runtime/Operations/PythonOps.cs @@ -2603,6 +2603,8 @@ public static PythonDictionary UserMappingToPythonDictionary(CodeContext/*!*/ co } public static PythonDictionary CopyAndVerifyPythonDictionary(PythonFunction function, PythonDictionary dict) { + if (dict.TreatAsMapping) return CopyAndVerifyUserMapping(function, dict); + if (dict._storage.HasNonStringAttributes()) { throw TypeError("{0}() keywords must be strings", function.__name__); } diff --git a/src/core/IronPython/Runtime/PythonDictionary.cs b/src/core/IronPython/Runtime/PythonDictionary.cs index afd89ae9f..9c84cc079 100644 --- a/src/core/IronPython/Runtime/PythonDictionary.cs +++ b/src/core/IronPython/Runtime/PythonDictionary.cs @@ -678,6 +678,11 @@ internal bool TryRemoveValue(object key, out object value) { return _storage.TryRemoveValue(ref _storage, key, out value); } + /// + /// If __iter__ is overridden then we should treat the dict as a mapping. + /// + internal bool TreatAsMapping => GetType() != typeof(PythonDictionary) && ((Func)__iter__).Method.DeclaringType != typeof(PythonDictionary); + #region Debugger View internal class DebugProxy { diff --git a/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini b/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini index 5251beb3d..dbadacbe3 100644 --- a/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini +++ b/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini @@ -207,9 +207,6 @@ Ignore=true RunCondition=$(IS_DEBUG) # https://github.com/IronLanguages/ironpython3/issues/1067 Timeout=600000 # 10 minute timeout -[CPython.test_call] # IronPython.test_call_stdlib -Ignore=true - [CPython.test_capi] Ignore=true Reason=ImportError: No module named _testcapi diff --git a/tests/suite/test_call_stdlib.py b/tests/suite/test_call_stdlib.py deleted file mode 100644 index e960fb6f8..000000000 --- a/tests/suite/test_call_stdlib.py +++ /dev/null @@ -1,28 +0,0 @@ -# Licensed to the .NET Foundation under one or more agreements. -# The .NET Foundation licenses this file to you under the Apache 2.0 License. -# See the LICENSE file in the project root for more information. - -## -## Run selected tests from test_call from StdLib -## - -from iptest import is_ironpython, generate_suite, run_test - -import test.test_call - -def load_tests(loader, standard_tests, pattern): - tests = loader.loadTestsFromModule(test.test_call, pattern=pattern) - - if is_ironpython: - failing_tests = [] - - skip_tests = [ - test.test_call.FunctionCalls('test_kwargs_order'), # intermittent failures due to https://github.com/IronLanguages/ironpython3/issues/1460 - ] - - return generate_suite(tests, failing_tests, skip_tests) - - else: - return tests - -run_test(__name__) diff --git a/tests/suite/test_functools_stdlib.py b/tests/suite/test_functools_stdlib.py index a5843af39..c29b3bd8c 100644 --- a/tests/suite/test_functools_stdlib.py +++ b/tests/suite/test_functools_stdlib.py @@ -43,10 +43,8 @@ def load_tests(loader, standard_tests, pattern): ] skip_tests = [ - test.test_functools.TestLRUC('test_kwargs_order'), # intermittent failures - https://github.com/IronLanguages/ironpython3/issues/1460 test.test_functools.TestLRUC('test_lru_cache_threaded2'), # intermittent failures test.test_functools.TestLRUC('test_lru_cache_threaded3'), # intermittent failures - test.test_functools.TestLRUPy('test_kwargs_order'), # intermittent failures - https://github.com/IronLanguages/ironpython3/issues/1460 test.test_functools.TestLRUPy('test_lru_cache_threaded2'), # intermittent failures test.test_functools.TestLRUPy('test_lru_cache_threaded3'), # intermittent failures test.test_functools.TestPartialC('test_recursive_pickle'), # StackOverflowException From 08b7905845a022e5b8f5172e198dbcaef99198bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 1 Jan 2023 11:18:35 -0500 Subject: [PATCH 3/4] Add back Serialization --- .../Runtime/CommonDictionaryStorage.cs | 40 ++++++++++++++++++- tests/suite/test_ordered_dict_stdlib.py | 1 + 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs index 91a85dc83..45f5f9c06 100644 --- a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs +++ b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Runtime.Serialization; using System.Threading; using IronPython.Runtime.Operations; @@ -33,7 +34,8 @@ namespace IronPython.Runtime { /// the buckets and then calls a static helper function to do the read from the bucket /// array to ensure that readers are not seeing multiple bucket arrays. /// - internal sealed class CommonDictionaryStorage : DictionaryStorage { + [Serializable] + internal sealed class CommonDictionaryStorage : DictionaryStorage, ISerializable, IDeserializationCallback { private int[] _indices; private List _buckets; private int _count; @@ -117,6 +119,16 @@ private CommonDictionaryStorage(int[] indices, List buckets, int count, _eqFunc = eqFunc; } +#if FEATURE_SERIALIZATION + private CommonDictionaryStorage(SerializationInfo info, StreamingContext context) { + // Remember the serialization info, we'll deserialize when we get the callback. This + // enables special types like DBNull.Value to successfully be deserialized inside of us. We + // store the serialization info in a special bucket so we don't have an extra field just for + // serialization. + _buckets = new List { new Bucket(null, info, 0) }; + } +#endif + public override void Add(ref DictionaryStorage storage, object key, object value) => Add(key, value); @@ -651,5 +663,31 @@ private static bool TupleEquals(object o1, object o2) private static bool GenericEquals(object o1, object o2) => PythonOps.EqualRetBool(o1, o2); #endregion + +#if FEATURE_SERIALIZATION + #region ISerializable Members + + public void GetObjectData(SerializationInfo info, StreamingContext context) { + info.AddValue("buckets", GetItems()); + } + + void IDeserializationCallback.OnDeserialization(object sender) { + if (_indices is not null) { + // we've received multiple OnDeserialization callbacks, only + // deserialize after the 1st one + return; + } + + var info = (SerializationInfo)_buckets[0].Value; + _buckets.Clear(); + + var buckets = (List>)info.GetValue("buckets", typeof(List>)); + foreach (KeyValuePair kvp in buckets) { + AddNoLock(kvp.Key, kvp.Value); + } + } + + #endregion +#endif } } diff --git a/tests/suite/test_ordered_dict_stdlib.py b/tests/suite/test_ordered_dict_stdlib.py index cf6e4b74d..1e51e76c1 100644 --- a/tests/suite/test_ordered_dict_stdlib.py +++ b/tests/suite/test_ordered_dict_stdlib.py @@ -30,6 +30,7 @@ def load_tests(loader, standard_tests, pattern): ] skip_tests = [ + test.test_ordered_dict.CPythonBuiltinDictTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? test.test_ordered_dict.CPythonOrderedDictSubclassTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? test.test_ordered_dict.CPythonOrderedDictTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? test.test_ordered_dict.PurePythonOrderedDictSubclassTests('test_highly_nested_subclass'), # intermittent AssertionError - GC related? From cabcdd062d3b0b1908a1b9c7b81d4faaf03f1572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 1 Jan 2023 20:29:41 -0500 Subject: [PATCH 4/4] Simplify --- .../Runtime/CommonDictionaryStorage.cs | 46 +++++++------------ 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs index 45f5f9c06..0f36b7b35 100644 --- a/src/core/IronPython/Runtime/CommonDictionaryStorage.cs +++ b/src/core/IronPython/Runtime/CommonDictionaryStorage.cs @@ -24,7 +24,7 @@ namespace IronPython.Runtime { /// Mutations to the dictionary involves a simple locking strategy of /// locking on the DictionaryStorage object to ensure that only one /// mutation happens at a time. - /// + /// /// Reads against the dictionary happen lock free. When the dictionary is mutated /// it is either adding or removing buckets in a thread-safe manner so that the readers /// will either see a consistent picture as if the read occured before or after the mutation. @@ -161,11 +161,12 @@ public void AddNoLock(object key, object value) { } private void AddOne(object key, object value) { - GetHash(key, out var hc, out var eqFunc); - if (AddWorker(_indices, _buckets, new Bucket(key, value, hc), eqFunc)) { + Debug.Assert(_keyType == HeterogeneousType || key?.GetType() == _keyType); + var hc = _hashFunc(key) & int.MaxValue; + if (AddWorker(_indices, _buckets, new Bucket(key, value, hc), _eqFunc)) { if (_count >= (_indices.Length * Load)) { // grow the hash table - EnsureSize((int)(_indices.Length / Load) * ResizeMultiplier, eqFunc); + EnsureSize((int)(_indices.Length / Load) * ResizeMultiplier, _eqFunc); } } } @@ -298,7 +299,9 @@ private static KeyValuePair LookupIndex(int[] indices, List bu } // keep walking - index = ProbeNext(indices, index); + if (++index == indices.Length) { + index = 0; + } // if we ended up doing a full scan, then this means the key is not already in use and there are // only recycled buckets available -- nothing more to probe @@ -329,15 +332,6 @@ private bool AddWorker(int[] indices, List buckets, Bucket bucket, Func< return true; } - private static int ProbeNext(int[] indices, int index) { - // probe to next bucket - index++; - if (index == indices.Length) { - index = 0; - } - return index; - } - /// /// Removes an entry from the dictionary and returns true if the /// entry was removed or false. @@ -389,10 +383,7 @@ private void GetHash(object key, out int hc, out Func eqFu private bool TryRemoveNoLock(object key, out object value) { GetHash(key, out var hc, out var eqFunc); - return TryRemoveNoLock(key, hc, eqFunc, out value); - } - private bool TryRemoveNoLock(object key, int hc, Func eqFunc, out object value) { if (_indices == null) { value = null; return false; @@ -445,22 +436,19 @@ public override bool TryGetValue(object key, out object value) private bool TryGetValue(int[] indices, List buckets, object key, out object value) { if (_count > 0 && indices != null) { GetHash(key, out var hc, out var eqFunc); - return TryGetValue(indices, buckets, key, hc, eqFunc, out value); - } - value = null; - return false; - } + var pair = LookupIndex(indices, buckets, key, hc, eqFunc); + if (pair.Value < 0) { + value = null; + return false; + } - private static bool TryGetValue(int[] indices, List buckets, object key, int hc, Func eqFunc, out object value) { - var pair = LookupIndex(indices, buckets, key, hc, eqFunc); - if (pair.Value < 0) { - value = null; - return false; + value = buckets[pair.Value].Value; + return true; } - value = buckets[pair.Value].Value; - return true; + value = null; + return false; } ///