Skip to content

Commit

Permalink
Avoid recomputing bindings multiple times
Browse files Browse the repository at this point in the history 
Fixes #10806
  • Loading branch information
@albyrock87
albyrock87 committed Sep 2, 2024
1 parent 5aae18c commit c568f3b
Show file tree
Hide file tree
Showing 4 changed files with 286 additions and 18 deletions.
63 changes: 47 additions & 16 deletions src/Controls/src/Core/BindableObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ internal void SetBinding(BindableProperty targetProperty, BindingBase binding, S
[EditorBrowsable(EditorBrowsableState.Never)]
public static void SetInheritedBindingContext(BindableObject bindable, object value)
{
//I wonder if we coulnd't treat bindingcoutext with specificities
// I wonder if we couldn't treat BindingContext with specificities
BindablePropertyContext bpContext = bindable.GetContext(BindingContextProperty);
if (bpContext != null && bpContext.Values.GetSpecificityAndValue().Key.CompareTo(SetterSpecificity.ManualValueSetter) >= 0)
return;
Expand All @@ -355,20 +355,33 @@ public static void SetInheritedBindingContext(BindableObject bindable, object va
{
binding.Context = value;
bindable._inheritedContext = null;
// OnBindingContextChanged fires from within BindingContextProperty propertyChanged callback
bindable.ApplyBinding(bpContext, fromBindingContextChanged: true);
}
else
{
bindable._inheritedContext = new WeakReference(value);
bindable.ApplyBindings(fromBindingContextChanged: true);
bindable.OnBindingContextChanged();
}

bindable.ApplyBindings(skipBindingContext: false, fromBindingContextChanged: true);
bindable.OnBindingContextChanged();
}

/// <summary>
/// Applies all the current bindings to <see cref="BindingContext" />.
/// </summary>
protected void ApplyBindings() => ApplyBindings(skipBindingContext: false, fromBindingContextChanged: false);
protected void ApplyBindings()
{
BindablePropertyContext bpContext = GetContext(BindingContextProperty);
var binding = bpContext?.Bindings.Values.LastOrDefault();
if (binding != null)
{
ApplyBinding(bpContext, fromBindingContextChanged: false);
}
else
{
ApplyBindings(fromBindingContextChanged: false);
}
}

/// <summary>
/// Raises the <see cref="BindingContextChanged"/> event.
Expand Down Expand Up @@ -645,25 +658,43 @@ void SetValueActual(BindableProperty property, BindablePropertyContext context,
}
}

internal void ApplyBindings(bool skipBindingContext, bool fromBindingContextChanged)
void ApplyBindings(bool fromBindingContextChanged)
{
var prop = _properties.Values.ToArray();

for (int i = 0, propLength = prop.Length; i < propLength; i++)
{
BindablePropertyContext context = prop[i];
var kvp = context.Bindings.LastOrDefault();
var specificity = kvp.Key;
var binding = kvp.Value;

if (binding == null)
if (ReferenceEquals(context.Property, BindingContextProperty))
{
// BindingContextProperty Binding is handled separately within SetInheritedBindingContext
continue;
}

if (skipBindingContext && ReferenceEquals(context.Property, BindingContextProperty))
continue;
ApplyBinding(context, fromBindingContextChanged);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
void ApplyBinding(BindablePropertyContext context, bool fromBindingContextChanged)
{
var bindings = context.Bindings;
if (bindings.Count == 0)
{
return;
}

var kvp = bindings.LastOrDefault();
var binding = kvp.Value;

binding.Unapply(fromBindingContextChanged: fromBindingContextChanged);
binding.Apply(BindingContext, this, context.Property, fromBindingContextChanged, specificity);
if (binding == null)
{
return;
}

var specificity = kvp.Key;
binding.Unapply(fromBindingContextChanged);
binding.Apply(BindingContext, this, context.Property, fromBindingContextChanged, specificity);
}

static void BindingContextPropertyBindingChanging(BindableObject bindable, BindingBase oldBindingBase, BindingBase newBindingBase)
Expand All @@ -681,7 +712,7 @@ static void BindingContextPropertyBindingChanging(BindableObject bindable, Bindi
static void BindingContextPropertyChanged(BindableObject bindable, object oldvalue, object newvalue)
{
bindable._inheritedContext = null;
bindable.ApplyBindings(skipBindingContext: true, fromBindingContextChanged: true);
bindable.ApplyBindings(fromBindingContextChanged: true);
bindable.OnBindingContextChanged();
}

Expand Down
2 changes: 0 additions & 2 deletions src/Controls/src/Core/Element/Element.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,6 @@ protected virtual void OnChildAdded(Element child)
{
child.SetParent(this);

child.ApplyBindings(skipBindingContext: false, fromBindingContextChanged: true);

ChildAdded?.Invoke(this, new ElementEventArgs(child));

VisualDiagnostics.OnChildAdded(this, child);
Expand Down
157 changes: 157 additions & 0 deletions src/Controls/tests/Core.UnitTests/BindingUnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Maui.Graphics;
using Xunit;
Expand Down Expand Up @@ -1599,6 +1600,142 @@ public void OneWayBindingsDontDoubleApplyOnSourceUpdates()

Assert.Equal(2, vm.count);
}

[Fact]
public void BindingsApplyOnlyOnceOnBindingContextInheritance()
{
var sb = new StringBuilder(50);
var bindingContext = new
{
Text = "a binding context"
};

var root = new MockBindable();
var bindableProperty = MockBindable.TextProperty;

var level1 = new MockBindable();
level1.SetBinding(
bindableProperty,
new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 1)));

var level2 = new MockBindable();
level2.SetBinding(
bindableProperty,
new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 2)));

root.AddLogicalChild(level1);
level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;

Assert.Equal("12", sb.ToString());
}

[Fact]
public void BindingsApplyOnlyOnceOnParentSet()
{
var sb = new StringBuilder(50);
var bindingContext = new
{
Text = "a binding context"
};

var root = new MockBindable();
var bindableProperty = MockBindable.TextProperty;

var level1 = new MockBindable();
level1.SetBinding(
bindableProperty,
new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 1)));

var level2 = new MockBindable();
level2.SetBinding(
bindableProperty,
new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 2)));

level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;
root.AddLogicalChild(level1);

Assert.Equal("12", sb.ToString());
}

[Fact]
public void BindingContextBindingsApplyOnlyOnceOnBindingContextInheritance()
{
var sb = new StringBuilder(50);
var bindingContext = new
{
Level1 = new
{
Level2 = new
{
Text = "a binding context"
}
}
};

var root = new MockBindable();

var level1 = new MockBindable();
level1.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level1", BindingMode.OneWay, new IdentityLoggerConverter(sb, 1)));

var level2 = new MockBindable();
level2.SetBinding(MockBindable.TextProperty, new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 2)));
level2.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level2", BindingMode.OneWay, new IdentityLoggerConverter(sb, 3)));

root.AddLogicalChild(level1);
level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;

// 3 is in the middle because the BindingContext must be set before other properties (like text)
Assert.Equal("132", sb.ToString());
Assert.Equal(bindingContext.Level1.Level2.Text, level2.GetValue(MockBindable.TextProperty));
}

[Fact]
public void BindingContextBindingsApplyOnlyOnceOnParentSet()
{
var sb = new StringBuilder(50);
var bindingContext = new
{
Level1 = new
{
Level2 = new
{
Text = "a binding context"
}
}
};

var root = new MockBindable();

var level1 = new MockBindable();
level1.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level1", BindingMode.OneWay, new IdentityLoggerConverter(sb, 1)));

var level2 = new MockBindable();
level2.SetBinding(MockBindable.TextProperty, new Binding("Text", BindingMode.OneWay, new IdentityLoggerConverter(sb, 2)));
level2.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level2", BindingMode.OneWay, new IdentityLoggerConverter(sb, 3)));

level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;
root.AddLogicalChild(level1);

// 3 is in the middle because the BindingContext must be set before other properties (like text)
Assert.Equal("132", sb.ToString());
Assert.Equal(bindingContext.Level1.Level2.Text, level2.GetValue(MockBindable.TextProperty));
}

[Fact("When there are multiple bindings, an update in one should not cause the other to udpate.")]
public void BindingsShouldNotTriggerOtherBindings()
Expand Down Expand Up @@ -2359,5 +2496,25 @@ public void DoubleSetBinding()
button.BackgroundColor = Colors.Coral;
button.SetBinding(Button.BackgroundColorProperty, new Binding("BackgroundColor", source: this));
}

private class IdentityLoggerConverter : IValueConverter
{
readonly StringBuilder _sb;
readonly int _id;

public IdentityLoggerConverter(StringBuilder sb, int id)
{
_sb = sb;
_id = id;
}

public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
{
_sb.Append(_id);
return value;
}

public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture) => throw new NotImplementedException();
}
}
}
82 changes: 82 additions & 0 deletions src/Core/tests/Benchmarks/Benchmarks/ElementBenchmarker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,89 @@ public bool SetProperty()
brush.Color = Colors.Black;
brush.Color = Colors.Green;
}

return brush.Color == Colors.Green;
}

class MockElement : Element
{
public static readonly BindableProperty TextProperty = BindableProperty.Create(
nameof(Text),
typeof(string),
typeof(MockElement));

public string Text
{
get => (string)GetValue(TextProperty);
set => SetValue(TextProperty, value);
}
}

[Benchmark]
public void ApplyPropertyViaBindingContextInheritance()
{
const int iterations = 100;

for (int i = 0; i < iterations; i++)
{
var bindingContext = "a binding context";

var root = new MockElement();
var bindableProperty = MockElement.TextProperty;

var level1 = new MockElement();
level1.SetBinding(
bindableProperty,
new Binding(".", BindingMode.OneWay));

var level2 = new MockElement();
level2.SetBinding(
bindableProperty,
new Binding(".", BindingMode.OneWay));

root.AddLogicalChild(level1);
level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;
}
}

[Benchmark]
public void ApplyBindingContextViaBindingContextInheritance()
{
const int iterations = 100;

var bindingContext = new
{
Level1 = new
{
Level2 = new
{
Text = "a binding context"
}
}
};

for (int i = 0; i < iterations; i++)
{
var root = new MockElement();

var level1 = new MockElement();
level1.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level1", BindingMode.OneWay));

var level2 = new MockElement();
level2.SetBinding(MockElement.TextProperty, new Binding("Text", BindingMode.OneWay));
level2.SetBinding(
BindableObject.BindingContextProperty,
new Binding("Level2", BindingMode.OneWay));

root.AddLogicalChild(level1);
level1.AddLogicalChild(level2);

root.BindingContext = bindingContext;
}
}
}
}

0 comments on commit c568f3b

Please sign in to comment.