diff --git a/osu.Framework/Bindables/BindableInstanceTracker.cs b/osu.Framework/Bindables/BindableInstanceTracker.cs new file mode 100644 index 0000000000..78195e11e7 --- /dev/null +++ b/osu.Framework/Bindables/BindableInstanceTracker.cs @@ -0,0 +1,67 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace osu.Framework.Bindables +{ + /// + /// Facilitates tracking of bindable instances through recursive invocations on bound bindables. + /// Can be provided a small array for fast/naive de-duplication while falling back to when there are too many bindable instances. + /// + internal ref struct BindableInstanceTracker + { + private readonly Span fastStorage; + private HashSet? slowStorage; + private int count; + + /// + /// Creates a bindable instance tracker. + /// + /// A small array for far de-duplication. Usually allocated on the stack. + public BindableInstanceTracker(Span fastStorage) + { + this.fastStorage = fastStorage; + } + + /// + /// Adds an instance to the tracker. + /// + /// A unique instance ID. + /// Whether the instance was previously seen. + public bool Add(int id) + { + Debug.Assert(id != 0); + + bool result = count < fastStorage.Length ? addFast(id) : addSlow(id); + count += result ? 1 : 0; + return result; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool addSlow(int id) + { + if (slowStorage == null) + { + slowStorage = []; + for (int i = 0; i < count; i++) + slowStorage.Add(fastStorage[i]); + } + + return slowStorage.Add(id); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool addFast(int id) + { + if (fastStorage[..count].Contains(id)) + return false; + + fastStorage[count] = id; + return true; + } + } +} diff --git a/osu.Framework/Bindables/BindableList.cs b/osu.Framework/Bindables/BindableList.cs index 4f770fcef9..6e594fc98e 100644 --- a/osu.Framework/Bindables/BindableList.cs +++ b/osu.Framework/Bindables/BindableList.cs @@ -9,6 +9,7 @@ using System.Collections.Specialized; using System.Globalization; using System.Linq; +using System.Runtime.CompilerServices; using JetBrains.Annotations; using osu.Framework.Caching; using osu.Framework.Extensions.TypeExtensions; @@ -37,12 +38,17 @@ public class BindableList : IBindableList, IBindable, IParseable, IList private LockedWeakList> bindings; + private readonly int instanceId; + /// /// Creates a new , optionally adding the items of the given collection. /// /// The items that are going to be contained in the newly created . public BindableList(IEnumerable items = null) { + // This will return the unique runtime identity for the object. + instanceId = RuntimeHelpers.GetHashCode(this); + if (items != null) collection.AddRange(items); } @@ -57,12 +63,17 @@ public BindableList(IEnumerable items = null) public T this[int index] { get => collection[index]; - set => setIndex(index, value, new HashSet>()); + set + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + setIndex(index, value, ref instances); + } } - private void setIndex(int index, T item, HashSet> appliedInstances) + private void setIndex(int index, T item, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -73,7 +84,7 @@ private void setIndex(int index, T item, HashSet> appliedInstanc if (bindings != null) { foreach (var b in bindings) - b.setIndex(index, item, appliedInstances); + b.setIndex(index, item, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, lastItem, index)); @@ -85,11 +96,15 @@ private void setIndex(int index, T item, HashSet> appliedInstanc /// The item to be added. /// Thrown when this is . public void Add(T item) - => add(item, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + add(item, ref instances); + } - private void add(T item, HashSet> appliedInstances) + private void add(T item, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -98,7 +113,7 @@ private void add(T item, HashSet> appliedInstances) if (bindings != null) { foreach (var b in bindings) - b.add(item, appliedInstances); + b.add(item, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, collection.Count - 1)); @@ -118,11 +133,15 @@ private void add(T item, HashSet> appliedInstances) /// The item to insert. /// Thrown when this is . public void Insert(int index, T item) - => insert(index, item, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + insert(index, item, ref instances); + } - private void insert(int index, T item, HashSet> appliedInstances) + private void insert(int index, T item, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -131,7 +150,7 @@ private void insert(int index, T item, HashSet> appliedInstances if (bindings != null) { foreach (var b in bindings) - b.insert(index, item, appliedInstances); + b.insert(index, item, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); @@ -142,11 +161,15 @@ private void insert(int index, T item, HashSet> appliedInstances /// /// Thrown when this is . public void Clear() - => clear(new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + clear(ref instances); + } - private void clear(HashSet> appliedInstances) + private void clear(ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -161,7 +184,7 @@ private void clear(HashSet> appliedInstances) if (bindings != null) { foreach (var b in bindings) - b.clear(appliedInstances); + b.clear(ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, clearedItems, 0)); @@ -182,11 +205,14 @@ public bool Contains(T item) /// true if the removal was successful. /// Thrown if this is . public bool Remove(T item) - => remove(item, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + return remove(item, ref instances); + } - private bool remove(T item, HashSet> appliedInstances) + private bool remove(T item, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) + if (!instances.Add(instanceId)) return false; ensureMutationAllowed(); @@ -208,12 +234,11 @@ private bool remove(T item, HashSet> appliedInstances) { // prevent re-removing from the callee. // That would result in a . - b.remove(listItem, appliedInstances); + b.remove(listItem, ref instances); } } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, listItem, index)); - return true; } @@ -224,12 +249,14 @@ private bool remove(T item, HashSet> appliedInstances) /// The count of items to be removed. public void RemoveRange(int index, int count) { - removeRange(index, count, new HashSet>()); + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + removeRange(index, count, ref instances); } - private void removeRange(int index, int count, HashSet> appliedInstances) + private void removeRange(int index, int count, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -247,7 +274,7 @@ private void removeRange(int index, int count, HashSet> appliedI { // Prevent re-adding the item back to the callee. // That would result in a . - b.removeRange(index, count, appliedInstances); + b.removeRange(index, count, ref instances); } } @@ -260,11 +287,15 @@ private void removeRange(int index, int count, HashSet> appliedI /// The index of the item to remove. /// Thrown if this is . public void RemoveAt(int index) - => removeAt(index, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + removeAt(index, ref instances); + } - private void removeAt(int index, HashSet> appliedInstances) + private void removeAt(int index, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -275,7 +306,7 @@ private void removeAt(int index, HashSet> appliedInstances) if (bindings != null) { foreach (var b in bindings) - b.removeAt(index, appliedInstances); + b.removeAt(index, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index)); @@ -286,11 +317,14 @@ private void removeAt(int index, HashSet> appliedInstances) /// /// The predicate. public int RemoveAll(Predicate match) - => removeAll(match, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + return removeAll(match, ref instances); + } - private int removeAll(Predicate match, HashSet> appliedInstances) + private int removeAll(Predicate match, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) + if (!instances.Add(instanceId)) return 0; ensureMutationAllowed(); @@ -306,7 +340,7 @@ private int removeAll(Predicate match, HashSet> appliedInstan if (bindings != null) { foreach (var b in bindings) - b.removeAll(match, appliedInstances); + b.removeAll(match, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removedItems)); @@ -320,11 +354,15 @@ private int removeAll(Predicate match, HashSet> appliedInstan /// The count of items to be removed. /// The items to replace the removed items with. public void ReplaceRange(int index, int count, IEnumerable newItems) - => replaceRange(index, count, newItems as ICollection ?? newItems.ToArray(), new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + replaceRange(index, count, newItems as ICollection ?? newItems.ToArray(), ref instances); + } - private void replaceRange(int index, int count, ICollection newItems, HashSet> appliedInstances) + private void replaceRange(int index, int count, ICollection newItems, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -343,26 +381,13 @@ private void replaceRange(int index, int count, ICollection newItems, HashSet { // Prevent re-adding the item back to the callee. // That would result in a . - b.replaceRange(index, count, newItems, appliedInstances); + b.replaceRange(index, count, newItems, ref instances); } } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, (IList)newItems, removedItems!, index)); } - /// - /// As we are applying non-atomic operations on a potentially complex bindable tree, care needs to be taken to not apply - /// the same operation twice to the same bindable instance. - /// - /// This call tracks all instances which an operation has already been applied to. - /// - /// A hash set to be passed down the recursive call tree tracking all applied instances. - /// Whether the current instance has already been applied. - private bool checkAlreadyApplied(HashSet> appliedInstances) - { - return !appliedInstances.Add(this); - } - /// /// Copies the contents of this to the given array, starting at the given index. /// @@ -544,11 +569,15 @@ public virtual void UnbindFrom(IUnbindable them) /// The collection whose items should be added to this collection. /// Thrown if this collection is public void AddRange(IEnumerable items) - => addRange(items as ICollection ?? items.ToArray(), new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + addRange(items as ICollection ?? items.ToArray(), ref instances); + } - private void addRange(ICollection items, HashSet> appliedInstances) + private void addRange(ICollection items, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -557,7 +586,7 @@ private void addRange(ICollection items, HashSet> appliedInst if (bindings != null) { foreach (var b in bindings) - b.addRange(items, appliedInstances); + b.addRange(items, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, (IList)items, collection.Count - items.Count)); @@ -569,11 +598,15 @@ private void addRange(ICollection items, HashSet> appliedInst /// The index of the item to move. /// The index specifying the new location of the item. public void Move(int oldIndex, int newIndex) - => move(oldIndex, newIndex, new HashSet>()); + { + BindableInstanceTracker instances = new BindableInstanceTracker(stackalloc int[16]); + move(oldIndex, newIndex, ref instances); + } - private void move(int oldIndex, int newIndex, HashSet> appliedInstances) + private void move(int oldIndex, int newIndex, ref BindableInstanceTracker instances) { - if (checkAlreadyApplied(appliedInstances)) return; + if (!instances.Add(instanceId)) + return; ensureMutationAllowed(); @@ -585,7 +618,7 @@ private void move(int oldIndex, int newIndex, HashSet> appliedIn if (bindings != null) { foreach (var b in bindings) - b.move(oldIndex, newIndex, appliedInstances); + b.move(oldIndex, newIndex, ref instances); } CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, item, newIndex, oldIndex));