Merge pull request #6198 from smoogipoo/improve-aggregate-bindable-perf

Reduce allocs in `AggregateBindable`
ppy · Feb 26, 2024 · 051c483 · 051c483
2 parents 944bcb4 + 0ad9d94
commit 051c483
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 23 deletions.
diff --git a/osu.Framework.Benchmarks/BenchmarkAggregateBindable.cs b/osu.Framework.Benchmarks/BenchmarkAggregateBindable.cs
@@ -11,15 +11,27 @@ public class BenchmarkAggregateBindable
     {
         private readonly BindableInt source1 = new BindableInt();
         private readonly BindableInt source2 = new BindableInt();
+        private readonly AggregateBindable<int> boundAggregate = new AggregateBindable<int>(((i, j) => i + j));
+        private readonly AggregateBindable<int> aggregate = new AggregateBindable<int>(((i, j) => i + j));
 
-        [Benchmark]
-        public void AggregateRecalculation()
+        [GlobalSetup]
+        public void GlobalSetup()
         {
-            var aggregate = new AggregateBindable<int>(((i, j) => i + j));
+            boundAggregate.AddSource(source1);
+            boundAggregate.AddSource(source2);
+        }
 
+        [Benchmark]
+        public void AddRemoveSource()
+        {
             aggregate.AddSource(source1);
             aggregate.AddSource(source2);
+            aggregate.RemoveAllSources();
+        }
 
+        [Benchmark]
+        public void SetValue()
+        {
             for (int i = 0; i < 100; i++)
             {
                 source1.Value = i;

diff --git a/osu.Framework/Bindables/AggregateBindable.cs b/osu.Framework/Bindables/AggregateBindable.cs
@@ -5,7 +5,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 
 namespace osu.Framework.Bindables
 {
@@ -52,7 +51,7 @@ public void AddSource(IBindable<T> bindable)
                     return;
 
                 var boundCopy = bindable.GetBoundCopy();
-                sourceMapping.Add(new WeakRefPair(new WeakReference<IBindable<T>>(bindable), boundCopy));
+                sourceMapping.Add(new WeakRefPair(bindable.GetWeakReference(), boundCopy));
                 boundCopy.BindValueChanged(recalculateAggregate, true);
             }
         }
@@ -65,20 +64,26 @@ public void RemoveSource(IBindable<T> bindable)
         {
             lock (sourceMapping)
             {
-                var weak = findExistingPair(bindable);
-
-                if (weak != null)
+                if (findExistingPair(bindable) is WeakRefPair pair)
                 {
-                    weak.BoundCopy.UnbindAll();
-                    sourceMapping.Remove(weak);
+                    pair.BoundCopy.UnbindAll();
+                    sourceMapping.Remove(pair);
                 }
 
                 recalculateAggregate();
             }
         }
 
-        private WeakRefPair findExistingPair(IBindable<T> bindable) =>
-            sourceMapping.FirstOrDefault(p => p.WeakReference.TryGetTarget(out var target) && target == bindable);
+        private WeakRefPair? findExistingPair(IBindable<T> bindable)
+        {
+            foreach (var p in sourceMapping)
+            {
+                if (p.WeakReference.TryGetTarget(out var target) && target == bindable)
+                    return p;
+            }
+
+            return null;
+        }
 
         private void recalculateAggregate(ValueChangedEvent<T> obj = null)
         {
@@ -112,16 +117,6 @@ public void RemoveAllSources()
             }
         }
 
-        private class WeakRefPair
-        {
-            public readonly WeakReference<IBindable<T>> WeakReference;
-            public readonly IBindable<T> BoundCopy;
-
-            public WeakRefPair(WeakReference<IBindable<T>> weakReference, IBindable<T> boundCopy)
-            {
-                WeakReference = weakReference;
-                BoundCopy = boundCopy;
-            }
-        }
+        private readonly record struct WeakRefPair(WeakReference<Bindable<T>> WeakReference, IBindable<T> BoundCopy);
     }
 }
diff --git a/osu.Framework/Bindables/Bindable.cs b/osu.Framework/Bindables/Bindable.cs
@@ -440,6 +440,8 @@ public Bindable<T> GetUnboundCopy()
 
         IBindable IBindable.GetBoundCopy() => GetBoundCopy();
 
+        WeakReference<Bindable<T>> IBindable<T>.GetWeakReference() => weakReference;
+
         IBindable<T> IBindable<T>.GetBoundCopy() => GetBoundCopy();
 
         /// <inheritdoc cref="IBindable{T}.GetBoundCopy"/>

diff --git a/osu.Framework/Bindables/IBindable.cs b/osu.Framework/Bindables/IBindable.cs
@@ -111,5 +111,10 @@ IBindable<T> BindTarget
 
         /// <inheritdoc cref="IBindable.GetBoundCopy"/>
         IBindable<T> GetBoundCopy();
+
+        /// <summary>
+        /// Retrieves a weak reference to this bindable.
+        /// </summary>
+        internal WeakReference<Bindable<T>> GetWeakReference();
     }
 }