From 082f699aa5d62e027d4f10a3fe095012edc1fa62 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 26 Feb 2024 19:40:37 +0900 Subject: [PATCH 1/4] Improve benchmark --- .../BenchmarkAggregateBindable.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/osu.Framework.Benchmarks/BenchmarkAggregateBindable.cs b/osu.Framework.Benchmarks/BenchmarkAggregateBindable.cs index 3e01b96c66..bb6039a658 100644 --- 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 boundAggregate = new AggregateBindable(((i, j) => i + j)); + private readonly AggregateBindable aggregate = new AggregateBindable(((i, j) => i + j)); - [Benchmark] - public void AggregateRecalculation() + [GlobalSetup] + public void GlobalSetup() { - var aggregate = new AggregateBindable(((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; From 840d401018f852ed0f2438d854d0baa650a019e6 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 26 Feb 2024 19:56:39 +0900 Subject: [PATCH 2/4] Reduce WeakReference allocs in AggregateBindable --- osu.Framework/Bindables/AggregateBindable.cs | 6 +++--- osu.Framework/Bindables/Bindable.cs | 2 ++ osu.Framework/Bindables/IBindable.cs | 5 +++++ 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/osu.Framework/Bindables/AggregateBindable.cs b/osu.Framework/Bindables/AggregateBindable.cs index 9e327752da..66afffe23a 100644 --- a/osu.Framework/Bindables/AggregateBindable.cs +++ b/osu.Framework/Bindables/AggregateBindable.cs @@ -52,7 +52,7 @@ public void AddSource(IBindable bindable) return; var boundCopy = bindable.GetBoundCopy(); - sourceMapping.Add(new WeakRefPair(new WeakReference>(bindable), boundCopy)); + sourceMapping.Add(new WeakRefPair(bindable.GetWeakReference(), boundCopy)); boundCopy.BindValueChanged(recalculateAggregate, true); } } @@ -114,10 +114,10 @@ public void RemoveAllSources() private class WeakRefPair { - public readonly WeakReference> WeakReference; + public readonly WeakReference> WeakReference; public readonly IBindable BoundCopy; - public WeakRefPair(WeakReference> weakReference, IBindable boundCopy) + public WeakRefPair(WeakReference> weakReference, IBindable boundCopy) { WeakReference = weakReference; BoundCopy = boundCopy; diff --git a/osu.Framework/Bindables/Bindable.cs b/osu.Framework/Bindables/Bindable.cs index 5f4687ba34..f213d44fc0 100644 --- a/osu.Framework/Bindables/Bindable.cs +++ b/osu.Framework/Bindables/Bindable.cs @@ -440,6 +440,8 @@ public Bindable GetUnboundCopy() IBindable IBindable.GetBoundCopy() => GetBoundCopy(); + public WeakReference> GetWeakReference() => weakReference; + IBindable IBindable.GetBoundCopy() => GetBoundCopy(); /// diff --git a/osu.Framework/Bindables/IBindable.cs b/osu.Framework/Bindables/IBindable.cs index 74b4de3c1f..dc64d96f00 100644 --- a/osu.Framework/Bindables/IBindable.cs +++ b/osu.Framework/Bindables/IBindable.cs @@ -111,5 +111,10 @@ IBindable BindTarget /// IBindable GetBoundCopy(); + + /// + /// Retrieves a weak reference to this bindable. + /// + internal WeakReference> GetWeakReference(); } } From 7a33912086243c8c6844c887092e08c9a448bea8 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 26 Feb 2024 20:04:45 +0900 Subject: [PATCH 3/4] Further reduce allocs --- osu.Framework/Bindables/AggregateBindable.cs | 33 +++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/osu.Framework/Bindables/AggregateBindable.cs b/osu.Framework/Bindables/AggregateBindable.cs index 66afffe23a..e65a66e95b 100644 --- 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 { @@ -65,20 +64,26 @@ public void RemoveSource(IBindable 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 bindable) => - sourceMapping.FirstOrDefault(p => p.WeakReference.TryGetTarget(out var target) && target == bindable); + private WeakRefPair? findExistingPair(IBindable bindable) + { + foreach (var p in sourceMapping) + { + if (p.WeakReference.TryGetTarget(out var target) && target == bindable) + return p; + } + + return null; + } private void recalculateAggregate(ValueChangedEvent obj = null) { @@ -112,16 +117,6 @@ public void RemoveAllSources() } } - private class WeakRefPair - { - public readonly WeakReference> WeakReference; - public readonly IBindable BoundCopy; - - public WeakRefPair(WeakReference> weakReference, IBindable boundCopy) - { - WeakReference = weakReference; - BoundCopy = boundCopy; - } - } + private readonly record struct WeakRefPair(WeakReference> WeakReference, IBindable BoundCopy); } } From 0ad9d94fe57efc4015f5abf5ee69d866bf24612c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 26 Feb 2024 13:35:41 +0100 Subject: [PATCH 4/4] Do not expose `GetWeakReference()` as public on `Bindable` --- osu.Framework/Bindables/Bindable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/Bindables/Bindable.cs b/osu.Framework/Bindables/Bindable.cs index f213d44fc0..4447e2a153 100644 --- a/osu.Framework/Bindables/Bindable.cs +++ b/osu.Framework/Bindables/Bindable.cs @@ -440,7 +440,7 @@ public Bindable GetUnboundCopy() IBindable IBindable.GetBoundCopy() => GetBoundCopy(); - public WeakReference> GetWeakReference() => weakReference; + WeakReference> IBindable.GetWeakReference() => weakReference; IBindable IBindable.GetBoundCopy() => GetBoundCopy();