perf: Reduce memory allocations for inherited properties management

Remove CompositeDisposable, Disposable, remove lambda allocation.
unoplatform · Mar 31, 2020 · d659c3d · d659c3d
1 parent e4aade2
commit d659c3d
Showing 1 changed file with 69 additions and 44 deletions.
diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
@@ -710,35 +710,49 @@ internal IDisposable RegisterPropertyChangedCallback(ExplicitPropertyChangedCall
 			);
 		}
 
+		private readonly struct InheritedPropertyChangedCallbackDisposable : IDisposable
+		{
+			public InheritedPropertyChangedCallbackDisposable(ManagedWeakReference objectStoreWeak, DependencyObjectStore childStore)
+			{
+				ChildStore = childStore;
+				ObjectStoreWeak = objectStoreWeak;
+			}
+
+			private readonly DependencyObjectStore ChildStore;
+			private readonly ManagedWeakReference ObjectStoreWeak;
+
+			public void Dispose()
+				=> CleanupInheritedPropertyChangedCallback(ObjectStoreWeak, ChildStore);
+		}
+
 		/// <summary>
 		/// Register for changes all dependency properties changes notifications for the specified instance.
 		/// </summary>
 		/// <param name="instance">The instance for which to observe properties changes</param>
 		/// <param name="callback">The callback</param>
 		/// <returns>A disposable that will unregister the callback when disposed.</returns>
-		internal IDisposable RegisterInheritedPropertyChangedCallback(DependencyObjectStore childStore)
+		private InheritedPropertyChangedCallbackDisposable RegisterInheritedPropertyChangedCallback(DependencyObjectStore childStore)
 		{
 			_childrenStores = _childrenStores.Add(childStore);
 
 			PropagateInheritedProperties(childStore);
 
-			// This weak reference ensure that the closure will not link
+			// This weak reference ensure that the disposable will not link
 			// the caller and the callee, in the same way "newValueActionWeak"
 			// does not link the callee to the caller.
-			var instanceRef = _thisWeakRef;
-
-			return Disposable.Create(() =>
-			{
-				var that = instanceRef.Target as DependencyObjectStore;
+			var objectStoreWeak = _thisWeakRef;
 
-				if (that != null)
-				{
-					// Delegates integrate a null check when removing new delegates.
-					that._childrenStores = that._childrenStores.Remove(childStore);
-				}
-			});
+			return new InheritedPropertyChangedCallbackDisposable(objectStoreWeak, childStore);
 		}
 
+		private static void CleanupInheritedPropertyChangedCallback(ManagedWeakReference objectStoreWeak, DependencyObjectStore childStore)
+		{
+			if (objectStoreWeak.Target is DependencyObjectStore that)
+			{
+				// Delegates integrate a null check when removing new delegates.
+				that._childrenStores = that._childrenStores.Remove(childStore);
+			}
+		}
 
 		/// <summary>
 		/// Register for changes all dependency properties changes notifications for the specified instance.
@@ -919,13 +933,26 @@ private void TryRegisterInheritedProperties(IDependencyObjectStoreProvider paren
 			}
 		}
 
-		internal IDisposable RegisterInheritedProperties(IDependencyObjectStoreProvider parentProvider)
+		private readonly struct InheritedPropertiesDisposable : IDisposable
 		{
+			private readonly IDisposable InheritedPropertiesCallback;
+			private readonly DependencyObjectStore Owner;
 
-			// Initialize at two as there is at most two disposables added below, and
-			// there is no need to allocate for more internally.
-			var disposable = new CompositeDisposable(2);
+			public InheritedPropertiesDisposable(DependencyObjectStore owner, IDisposable inheritedPropertiesCallback)
+			{
+				Owner = owner;
+				InheritedPropertiesCallback = inheritedPropertiesCallback;
+			}
 
+			public void Dispose()
+			{
+				InheritedPropertiesCallback.Dispose();
+				Owner.CleanupInheritedProperties();
+			}
+		}
+
+		private InheritedPropertiesDisposable RegisterInheritedProperties(IDependencyObjectStoreProvider parentProvider)
+		{
 			_parentTemplatedParentProperty = parentProvider.Store.TemplatedParentProperty;
 			_parentDataContextProperty = parentProvider.Store.DataContextProperty;
 
@@ -937,47 +964,45 @@ internal IDisposable RegisterInheritedProperties(IDependencyObjectStoreProvider
 			//    - By forcing a property update notification down to a DependencyObject's children when the parent is set.
 
 			// Subscribe to the parent's notifications
-			parentProvider
+			var inheritedPropertiesCallback = parentProvider
 				.Store
-				.RegisterInheritedPropertyChangedCallback(this)
-				.DisposeWith(disposable);
+				.RegisterInheritedPropertyChangedCallback(this);
 
 			// Force propagation for inherited properties defined on the current instance.
 			PropagateInheritedProperties();
 
-			// Register for unset values
-			disposable.Add(() =>
-			{
+			return new InheritedPropertiesDisposable(this, inheritedPropertiesCallback);
+		}
+
+		private void CleanupInheritedProperties()
+		{
 #if !HAS_EXPENSIVE_TRYFINALLY
-				// The try/finally incurs a very large performance hit in mono-wasm, and SetValue is in a very hot execution path.
-				// See https://github.com/mono/mono/issues/13653 for more details.
-				try
+			// The try/finally incurs a very large performance hit in mono-wasm, and SetValue is in a very hot execution path.
+			// See https://github.com/mono/mono/issues/13653 for more details.
+			try
 #endif
-				{
-					_unregisteringInheritedProperties = true;
+			{
+				_unregisteringInheritedProperties = true;
 
-					_inheritedForwardedProperties.Clear();
+				_inheritedForwardedProperties.Clear();
 
-					if (ActualInstance != null)
+				if (ActualInstance != null)
+				{
+					foreach (var dp in _updatedProperties)
 					{
-						foreach (var dp in _updatedProperties)
-						{
-							SetValue(dp, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
-						}
-
-						SetValue(_dataContextProperty, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
-						SetValue(_templatedParentProperty, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
+						SetValue(dp, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
 					}
+
+					SetValue(_dataContextProperty, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
+					SetValue(_templatedParentProperty, DependencyProperty.UnsetValue, DependencyPropertyValuePrecedences.Inheritance);
 				}
+			}
 #if !HAS_EXPENSIVE_TRYFINALLY
-				finally
+			finally
 #endif
-				{
-					_unregisteringInheritedProperties = false;
-				}
-			});
-
-			return disposable;
+			{
+				_unregisteringInheritedProperties = false;
+			}
 		}
 
 		[MethodImpl(MethodImplOptions.AggressiveInlining)]