From 67e809d7363f658f7c96fb238cc8b78b1edffc6b Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 9 Feb 2024 19:33:32 +0200 Subject: [PATCH 1/4] perf: Make `_childrenBinable` lazy and avoid unnecessary insertions --- .../UI/Xaml/DependencyObjectStore.Binder.cs | 102 +++++++++++------- 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs index 0bd5a69c91bf..68ecbadb476b 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs @@ -1,21 +1,23 @@ #nullable enable using System; -using System.Linq; -using Uno.Disposables; -using System.Runtime.CompilerServices; -using Uno.Foundation.Logging; -using Uno.Extensions; -using Uno.UI.DataBinding; -using Uno.UI; -using Microsoft.UI.Xaml.Data; +using System.Collections; using System.Collections.Generic; -using Microsoft.UI.Xaml; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; -using Uno.Diagnostics.Eventing; -using System.Collections; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Data; using Uno.Collections; +using Uno.Diagnostics.Eventing; +using Uno.Disposables; +using Uno.Extensions; +using Uno.Foundation.Logging; +using Uno.UI; +using Uno.UI.DataBinding; #if !IS_UNIT_TESTS using Uno.UI.Controls; @@ -35,8 +37,11 @@ public partial class DependencyObjectStore private readonly object _gate = new object(); - private readonly HashtableEx _childrenBindableMap = new HashtableEx(DependencyPropertyComparer.Default); - private readonly List _childrenBindable = new List(); + private HashtableEx? _childrenBindableMap; + private List? _childrenBindable; + + private HashtableEx ChildrenBindableMap => _childrenBindableMap ??= new HashtableEx(DependencyPropertyComparer.Default); + private List ChildrenBindable => _childrenBindable ??= new List(); private bool _isApplyingTemplateBindings; private bool _isApplyingDataContextBindings; @@ -88,6 +93,18 @@ public void SetTemplatedParent(FrameworkElement? templatedParent) } } + private bool IsCandidateChild([NotNullWhen(true)] object? child) + { + if (child is IDependencyObjectStoreProvider) + { + return true; + } + + // The property value may be an enumerable of providers + var isValidEnumerable = child is not string; + return isValidEnumerable && child is IList or IEnumerable; + } + private void ApplyChildrenBindable(object? inheritedValue, bool isTemplatedParent) { static void SetInherited(IDependencyObjectStoreProvider provider, object? inheritedValue, bool isTemplatedParent) @@ -102,10 +119,17 @@ static void SetInherited(IDependencyObjectStoreProvider provider, object? inheri } } + if (_childrenBindable is null) + { + return; + } + for (int i = 0; i < _childrenBindable.Count; i++) { var child = _childrenBindable[i]; + Debug.Assert(IsCandidateChild(child)); + var childAsStoreProvider = child as IDependencyObjectStoreProvider; // Get the parent if the child is a provider, otherwise an @@ -127,37 +151,32 @@ static void SetInherited(IDependencyObjectStoreProvider provider, object? inheri { SetInherited(childAsStoreProvider, inheritedValue, isTemplatedParent); } - else + else if (child is IList list) { - // The property value may be an enumerable of providers - var isValidEnumerable = !(child is string); + // Special case for IList where the child may not be enumerable - if (isValidEnumerable) + for (int childIndex = 0; childIndex < list.Count; childIndex++) { - if (child is IList list) + if (list[childIndex] is IDependencyObjectStoreProvider provider2) { - // Special case for IList where the child may not be enumerable - - for (int childIndex = 0; childIndex < list.Count; childIndex++) - { - if (list[childIndex] is IDependencyObjectStoreProvider provider2) - { - SetInherited(provider2, inheritedValue, isTemplatedParent); - } - } + SetInherited(provider2, inheritedValue, isTemplatedParent); } - else if (child is IEnumerable enumerable) + } + } + else if (child is IEnumerable enumerable) + { + foreach (var item in enumerable) + { + if (item is IDependencyObjectStoreProvider provider2) { - foreach (var item in enumerable) - { - if (item is IDependencyObjectStoreProvider provider2) - { - SetInherited(provider2, inheritedValue, isTemplatedParent); - } - } + SetInherited(provider2, inheritedValue, isTemplatedParent); } } } + else + { + throw new Exception("This cannot be reached. IsCandidateChild would have returned false if none of the conditions were true."); + } } } @@ -248,7 +267,7 @@ private void BinderDispose() } _properties.Dispose(); - _childrenBindableMap.Dispose(); + _childrenBindableMap?.Dispose(); } private void OnDataContextChanged(object? providedDataContext, object? actualDataContext, DependencyPropertyValuePrecedences precedence) @@ -569,7 +588,10 @@ private void OnDependencyPropertyChanged(DependencyPropertyDetails propertyDetai } private void SetChildrenBindableValue(DependencyPropertyDetails propertyDetails, object? value) - => _childrenBindable[GetOrCreateChildBindablePropertyIndex(propertyDetails.Property)] = value; + { + if (IsCandidateChild(value)) + ChildrenBindable[GetOrCreateChildBindablePropertyIndex(propertyDetails.Property)] = value; + } /// /// Gets or create an index in the list, to avoid enumerating . @@ -578,10 +600,10 @@ private int GetOrCreateChildBindablePropertyIndex(DependencyProperty property) { int index; - if (!_childrenBindableMap.TryGetValue(property, out var indexRaw)) + if (!ChildrenBindableMap.TryGetValue(property, out var indexRaw)) { - _childrenBindableMap[property] = index = _childrenBindableMap.Count; - _childrenBindable.Add(null); + ChildrenBindableMap[property] = index = ChildrenBindableMap.Count; + ChildrenBindable.Add(null!); // The caller will replace null with a non-null value. } else { From f2d21d09f495ebca2dc391580f07ba49a773948f Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sat, 10 Feb 2024 09:44:49 +0200 Subject: [PATCH 2/4] chore: Small fix --- .../UI/Xaml/DependencyObjectStore.Binder.cs | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs index 68ecbadb476b..4f2b86a25826 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs @@ -38,10 +38,10 @@ public partial class DependencyObjectStore private readonly object _gate = new object(); private HashtableEx? _childrenBindableMap; - private List? _childrenBindable; + private List? _childrenBindable; private HashtableEx ChildrenBindableMap => _childrenBindableMap ??= new HashtableEx(DependencyPropertyComparer.Default); - private List ChildrenBindable => _childrenBindable ??= new List(); + private List ChildrenBindable => _childrenBindable ??= new List(); private bool _isApplyingTemplateBindings; private bool _isApplyingDataContextBindings; @@ -590,7 +590,13 @@ private void OnDependencyPropertyChanged(DependencyPropertyDetails propertyDetai private void SetChildrenBindableValue(DependencyPropertyDetails propertyDetails, object? value) { if (IsCandidateChild(value)) + { ChildrenBindable[GetOrCreateChildBindablePropertyIndex(propertyDetails.Property)] = value; + } + else if (TryGetChildBindablePropertyIndex(propertyDetails.Property, out var index)) + { + ChildrenBindable[index] = null; + } } /// @@ -603,7 +609,7 @@ private int GetOrCreateChildBindablePropertyIndex(DependencyProperty property) if (!ChildrenBindableMap.TryGetValue(property, out var indexRaw)) { ChildrenBindableMap[property] = index = ChildrenBindableMap.Count; - ChildrenBindable.Add(null!); // The caller will replace null with a non-null value. + ChildrenBindable.Add(null); // The caller will replace null with a non-null value. } else { @@ -613,6 +619,19 @@ private int GetOrCreateChildBindablePropertyIndex(DependencyProperty property) return index; } + private bool TryGetChildBindablePropertyIndex(DependencyProperty property, out int index) + { + if (_childrenBindableMap?.TryGetValue(property, out var indexRaw) == true) + { + index = (int)indexRaw!; + return true; + } + + + index = -1; + return false; + } + public BindingExpression GetBindingExpression(DependencyProperty dependencyProperty) => _properties.GetBindingExpression(dependencyProperty); From 47f8f7fdd27ed19d15dc2018200942829623397b Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sat, 10 Feb 2024 10:24:05 +0200 Subject: [PATCH 3/4] chore: One more small fix --- src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs index 4f2b86a25826..376d1a723f0f 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs @@ -127,6 +127,10 @@ static void SetInherited(IDependencyObjectStoreProvider provider, object? inheri for (int i = 0; i < _childrenBindable.Count; i++) { var child = _childrenBindable[i]; + if (child is null) + { + continue; + } Debug.Assert(IsCandidateChild(child)); From c6752687845843d2cdd047add5e6c2ac34db7353 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sat, 10 Feb 2024 14:30:01 +0200 Subject: [PATCH 4/4] chore: Avoid redundant type check --- src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs index 376d1a723f0f..5c8c4102938b 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs @@ -102,7 +102,7 @@ private bool IsCandidateChild([NotNullWhen(true)] object? child) // The property value may be an enumerable of providers var isValidEnumerable = child is not string; - return isValidEnumerable && child is IList or IEnumerable; + return isValidEnumerable && child is IEnumerable; } private void ApplyChildrenBindable(object? inheritedValue, bool isTemplatedParent)