diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e405f..09f5d87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Fixed scriptable value editor breaking if the value is null - Fixed scriptable value editor having the wrong height in newer Unity versions - Fixed the package not having an author +- Fixed allocation when using `foreach` on scriptable lists and dictionaries ## [1.2.0] - 2023-05-20 diff --git a/Packages/se.hertzole.scriptable-values/Runtime/Collections/Core/ScriptableDictionary.cs b/Packages/se.hertzole.scriptable-values/Runtime/Collections/Core/ScriptableDictionary.cs index 7fd3d8c..e342360 100644 --- a/Packages/se.hertzole.scriptable-values/Runtime/Collections/Core/ScriptableDictionary.cs +++ b/Packages/se.hertzole.scriptable-values/Runtime/Collections/Core/ScriptableDictionary.cs @@ -27,13 +27,18 @@ internal virtual bool IsIndexUnique(int index) /// /// The key of the dictionary. /// The value of the dictionary. - public abstract class ScriptableDictionary : ScriptableDictionary, ISerializationCallbackReceiver, IDictionary, IReadOnlyDictionary, IDictionary where TKey : notnull + public abstract class ScriptableDictionary : ScriptableDictionary, + ISerializationCallbackReceiver, + IDictionary, + IReadOnlyDictionary, + IDictionary where TKey : notnull { [SerializeField] [Tooltip("If read only, the dictionary cannot be changed at runtime and won't be cleared on start.")] private bool isReadOnly = false; [SerializeField] - [Tooltip("If true, an equality check will be run before setting an item through the indexer to make sure the new object is not the same as the old one.")] + [Tooltip( + "If true, an equality check will be run before setting an item through the indexer to make sure the new object is not the same as the old one.")] private bool setEqualityCheck = true; [SerializeField] [Tooltip("If true, the dictionary will be cleared on play mode start/game boot.")] @@ -58,7 +63,11 @@ object IDictionary.this[object key] } } - public TValue this[TKey key] { get { return dictionary[key]; } set { SetValue(key, value); } } + public TValue this[TKey key] + { + get { return dictionary[key]; } + set { SetValue(key, value); } + } /// /// Gets or sets the that is used to determine equality of keys for the dictionary. @@ -72,7 +81,8 @@ public IEqualityComparer Comparer // Make sure it's a new comparer than the old one. if (!EqualityComparer>.Default.Equals(dictionary.Comparer, value)) { - Dictionary newDictionary = value == null ? new Dictionary(dictionary) : new Dictionary(dictionary, value); + Dictionary newDictionary = + value == null ? new Dictionary(dictionary) : new Dictionary(dictionary, value); dictionary = newDictionary; } @@ -83,40 +93,92 @@ public IEqualityComparer Comparer /// If true, an equality check will be run before setting an item through the indexer to make sure the new object is /// not the same as the old one. /// - public bool SetEqualityCheck { get { return setEqualityCheck; } set { setEqualityCheck = value; } } + public bool SetEqualityCheck + { + get { return setEqualityCheck; } + set { setEqualityCheck = value; } + } /// /// If true, the dictionary will be cleared on play mode start/game boot. /// - public bool ClearOnStart { get { return clearOnStart; } set { clearOnStart = value; } } - - bool IDictionary.IsFixedSize { get { return isReadOnly; } } - bool ICollection.IsSynchronized { get { return false; } } - object ICollection.SyncRoot { get { return this; } } - ICollection IDictionary.Values { get { return dictionary.Values; } } - ICollection IDictionary.Keys { get { return dictionary.Keys; } } + public bool ClearOnStart + { + get { return clearOnStart; } + set { clearOnStart = value; } + } /// - /// If read only, the dictionary cannot be changed at runtime and won't be cleared on start. + /// Gets a collection containing the keys in the dictionary. /// - public bool IsReadOnly { get { return isReadOnly; } set { isReadOnly = value; } } + public Dictionary.KeyCollection Keys + { + get { return dictionary.Keys; } + } /// - /// Gets the number of key/value pairs contained in the dictionary. + /// Gets a collection containing the values in the dictionary. /// - public int Count { get { return dictionary.Count; } } + public Dictionary.ValueCollection Values + { + get { return dictionary.Values; } + } + + bool IDictionary.IsFixedSize + { + get { return isReadOnly; } + } + bool ICollection.IsSynchronized + { + get { return false; } + } + object ICollection.SyncRoot + { + get { return this; } + } + ICollection IDictionary.Values + { + get { return dictionary.Values; } + } + ICollection IDictionary.Keys + { + get { return dictionary.Keys; } + } /// - /// Gets a collection containing the keys in the dictionary. + /// If read only, the dictionary cannot be changed at runtime and won't be cleared on start. /// - public ICollection Keys { get { return dictionary.Keys; } } + public bool IsReadOnly + { + get { return isReadOnly; } + set { isReadOnly = value; } + } /// - /// Gets a collection containing the values in the dictionary. + /// Gets the number of key/value pairs contained in the dictionary. /// - public ICollection Values { get { return dictionary.Values; } } + public int Count + { + get { return dictionary.Count; } + } + + ICollection IDictionary.Keys + { + get { return dictionary.Keys; } + } - IEnumerable IReadOnlyDictionary.Keys { get { return dictionary.Keys; } } - IEnumerable IReadOnlyDictionary.Values { get { return dictionary.Values; } } + ICollection IDictionary.Values + { + get { return dictionary.Values; } + } + + IEnumerable IReadOnlyDictionary.Keys + { + get { return dictionary.Keys; } + } + IEnumerable IReadOnlyDictionary.Values + { + get { return dictionary.Values; } + } /// /// Called when an item was added. Gives you the key and value of the newly added item. @@ -300,7 +362,7 @@ public bool ContainsValue(TValue value) public bool TryFindKey(Predicate predicate, out TKey key) { ThrowHelper.ThrowIfNull(predicate, nameof(predicate)); - + foreach (TKey dictionaryKey in dictionary.Keys) { if (predicate(dictionaryKey)) @@ -323,7 +385,7 @@ public bool TryFindKey(Predicate predicate, out TKey key) public bool TryFindValue(Predicate predicate, out TValue value) { ThrowHelper.ThrowIfNull(predicate, nameof(predicate)); - + foreach (TValue dictionaryValue in dictionary.Values) { if (predicate(dictionaryValue)) @@ -388,9 +450,9 @@ protected override void OnStart() #if UNITY_EDITOR ResetStackTraces(); #endif - + ClearSubscribers(); - + if (!isReadOnly && clearOnStart) { dictionary.Clear(); @@ -398,7 +460,7 @@ protected override void OnStart() values.Clear(); } } - + /// /// Removes any subscribers from the event. /// @@ -461,6 +523,15 @@ private void AddFastPath(TKey key, TValue value) OnChanged?.Invoke(DictionaryChangeType.Added); } + /// + /// Returns an enumerator that iterates through the dictionary. + /// + /// A enumerator for the dictionary. + public Dictionary.Enumerator GetEnumerator() + { + return dictionary.GetEnumerator(); + } + /// /// Determines whether the dictionary contains the specified key. /// @@ -694,6 +765,7 @@ void ISerializationCallbackReceiver.OnAfterDeserialize() // Update the dictionary with the serialized keys and values. dictionary.Clear(); + dictionary.EnsureCapacity(keys.Count); for (int i = 0; i < keys.Count; i++) { diff --git a/Packages/se.hertzole.scriptable-values/Tests/Runtime/ScriptableDictionaryTests.cs b/Packages/se.hertzole.scriptable-values/Tests/Runtime/ScriptableDictionaryTests.cs index 1e764bf..6575686 100644 --- a/Packages/se.hertzole.scriptable-values/Tests/Runtime/ScriptableDictionaryTests.cs +++ b/Packages/se.hertzole.scriptable-values/Tests/Runtime/ScriptableDictionaryTests.cs @@ -1,10 +1,13 @@ -using System.Collections; +using System; +using System.Collections; using System.Collections.Generic; using NUnit.Framework; using UnityEngine; using UnityEngine.TestTools; +using UnityEngine.TestTools.Constraints; using Assert = UnityEngine.Assertions.Assert; using AssertionException = UnityEngine.Assertions.AssertionException; +using Is = UnityEngine.TestTools.Constraints.Is; namespace Hertzole.ScriptableValues.Tests { @@ -12,7 +15,11 @@ public class ScriptableDictionaryTests : BaseRuntimeTest { private TestScriptableDictionary dictionary; - public bool IsReadOnly { get { return dictionary.IsReadOnly; } set { dictionary.IsReadOnly = value; } } + public bool IsReadOnly + { + get { return dictionary.IsReadOnly; } + set { dictionary.IsReadOnly = value; } + } protected override void OnSetup() { @@ -1165,12 +1172,147 @@ public void TrimExcess_Capacity_ReadOnly() } [Test] - public void GetKeyValuePairEnumerator() + public void GetEnumerator_IEnumerable() { dictionary.Add(0, 42); dictionary.Add(1, 43); dictionary.Add(2, 44); - IEnumerator> enumerator = ((ICollection>) dictionary).GetEnumerator(); + + // This generates garbage because of boxing. + NUnit.Framework.Assert.That(() => + { + IEnumerator enumerator = ((IEnumerable) dictionary).GetEnumerator(); + if (enumerator is IDisposable d) + { + d.Dispose(); + } + }, Is.AllocatingGCMemory()); + + IEnumerator enumerator = ((IEnumerable) dictionary).GetEnumerator(); + + try + { + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(0, ((KeyValuePair) enumerator.Current!).Key); + Assert.AreEqual(42, ((KeyValuePair) enumerator.Current).Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(1, ((KeyValuePair) enumerator.Current).Key); + Assert.AreEqual(43, ((KeyValuePair) enumerator.Current).Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(2, ((KeyValuePair) enumerator.Current).Key); + Assert.AreEqual(44, ((KeyValuePair) enumerator.Current).Value); + Assert.IsFalse(enumerator.MoveNext()); + } + + catch (AssertionException) { } + finally + { + if (enumerator is IDisposable disposable) + { + disposable.Dispose(); + } + } + } + + [Test] + public void GetEnumerator_IEnumerableGeneric() + { + dictionary.Add(0, 42); + dictionary.Add(1, 43); + dictionary.Add(2, 44); + + // This generates garbage because of boxing. + NUnit.Framework.Assert.That(() => + { + IEnumerator> enumerator = ((IEnumerable>) dictionary).GetEnumerator(); + enumerator.Dispose(); + }, Is.AllocatingGCMemory()); + + IEnumerator enumerator = ((IEnumerable) dictionary).GetEnumerator(); + + try + { + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(0, ((KeyValuePair) enumerator.Current!).Key); + Assert.AreEqual(42, ((KeyValuePair) enumerator.Current).Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(1, ((KeyValuePair) enumerator.Current).Key); + Assert.AreEqual(43, ((KeyValuePair) enumerator.Current).Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(2, ((KeyValuePair) enumerator.Current).Key); + Assert.AreEqual(44, ((KeyValuePair) enumerator.Current).Value); + Assert.IsFalse(enumerator.MoveNext()); + } + + catch (AssertionException) { } + finally + { + if (enumerator is IDisposable disposable) + { + disposable.Dispose(); + } + } + } + + [Test] + public void GetEnumerator_IDictionary() + { + dictionary.Add(0, 42); + dictionary.Add(1, 43); + dictionary.Add(2, 44); + + // This generates garbage because of boxing. + NUnit.Framework.Assert.That(() => + { + IDictionaryEnumerator enumerator = ((IDictionary) dictionary).GetEnumerator(); + + if (enumerator is IDisposable d) + { + d.Dispose(); + } + }, Is.AllocatingGCMemory()); + + IDictionaryEnumerator enumerator = ((IDictionary) dictionary).GetEnumerator(); + + try + { + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(0, enumerator.Key); + Assert.AreEqual(42, enumerator.Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(1, enumerator.Key); + Assert.AreEqual(43, enumerator.Value); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(2, enumerator.Key); + Assert.AreEqual(44, enumerator.Value); + Assert.IsFalse(enumerator.MoveNext()); + } + + catch (AssertionException) { } + finally + { + if (enumerator is IDisposable disposable) + { + disposable.Dispose(); + } + } + } + + [Test] + public void GetEnumerator() + { + dictionary.Add(0, 42); + dictionary.Add(1, 43); + dictionary.Add(2, 44); + + // This does not generate garbage because of the pure struct. + NUnit.Framework.Assert.That(() => + { + Dictionary.Enumerator enumerator = dictionary.GetEnumerator(); + enumerator.Dispose(); + }, NUnit.Framework.Is.Not.AllocatingGCMemory()); + + Dictionary.Enumerator enumerator = dictionary.GetEnumerator(); try { @@ -1185,6 +1327,7 @@ public void GetKeyValuePairEnumerator() Assert.AreEqual(44, enumerator.Current.Value); Assert.IsFalse(enumerator.MoveNext()); } + catch (AssertionException) { } finally { @@ -1193,43 +1336,68 @@ public void GetKeyValuePairEnumerator() } [Test] - public void GetEnumerator() + public void ForEach_NoAlloc() { dictionary.Add(0, 42); dictionary.Add(1, 43); dictionary.Add(2, 44); - IEnumerator enumerator = ((IEnumerable) dictionary).GetEnumerator(); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(0, ((KeyValuePair) enumerator.Current!).Key); - Assert.AreEqual(42, ((KeyValuePair) enumerator.Current).Value); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(1, ((KeyValuePair) enumerator.Current).Key); - Assert.AreEqual(43, ((KeyValuePair) enumerator.Current).Value); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(2, ((KeyValuePair) enumerator.Current).Key); - Assert.AreEqual(44, ((KeyValuePair) enumerator.Current).Value); - Assert.IsFalse(enumerator.MoveNext()); + // This does not generate garbage because of the pure struct. + NUnit.Framework.Assert.That(() => + { + int keySum = 0; + int valueSum = 0; + + foreach (KeyValuePair pair in dictionary) + { + keySum += pair.Key; + valueSum += pair.Value; + } + }, NUnit.Framework.Is.Not.AllocatingGCMemory()); } [Test] - public void GetEnumerator_Object() + public void ForEach_Keys_NoAlloc() { dictionary.Add(0, 42); dictionary.Add(1, 43); dictionary.Add(2, 44); - IDictionaryEnumerator enumerator = ((IDictionary) dictionary).GetEnumerator(); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(0, enumerator.Key); - Assert.AreEqual(42, enumerator.Value); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(1, enumerator.Key); - Assert.AreEqual(43, enumerator.Value); - Assert.IsTrue(enumerator.MoveNext()); - Assert.AreEqual(2, enumerator.Key); - Assert.AreEqual(44, enumerator.Value); - Assert.IsFalse(enumerator.MoveNext()); + // Mostly here to just make sure Keys gets created. Or else it will caught as gc alloc in the foreach. + Assert.AreEqual(3, dictionary.Keys.Count); + + // This does not generate garbage because of the pure struct. + NUnit.Framework.Assert.That(() => + { + int keySum = 0; + + foreach (int key in dictionary.Keys) + { + keySum += key; + } + }, NUnit.Framework.Is.Not.AllocatingGCMemory()); + } + + [Test] + public void ForEach_Values_NoAlloc() + { + dictionary.Add(0, 42); + dictionary.Add(1, 43); + dictionary.Add(2, 44); + + // Mostly here to just make sure Values gets created. Or else it will caught as gc alloc in the foreach. + Assert.AreEqual(3, dictionary.Values.Count); + + // This does not generate garbage because of the pure struct. + NUnit.Framework.Assert.That(() => + { + int valueSum = 0; + + foreach (int value in dictionary.Values) + { + valueSum += value; + } + }, NUnit.Framework.Is.Not.AllocatingGCMemory()); } [Test]