Improve property mapper performance

Author: albyrock87

src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.Android.cs

@@ -118,16 +118,6 @@ void UpdateDetail()
 		void UpdateFlyout()
 		{
 			_ = MauiContext ?? throw new InvalidOperationException($"{nameof(MauiContext)} should have been set by base class.");
-
-			// Once this issue has been taken care of
-			// https://github.com/dotnet/maui/issues/8456
-			// we can remove this code
-			if (VirtualView.Flyout.Handler?.MauiContext != null &&
-				VirtualView.Flyout.Handler.MauiContext != MauiContext)
-			{
-				VirtualView.Flyout.Handler.DisconnectHandler();
-			}
-
 			_ = VirtualView.Flyout.ToPlatform(MauiContext);
 
 			var newFlyoutView = VirtualView.Flyout.ToPlatform();

src/Core/src/Handlers/FlyoutView/FlyoutViewHandler.cs

@@ -16,11 +16,19 @@ namespace Microsoft.Maui.Handlers
 {
 	public partial class FlyoutViewHandler : IFlyoutViewHandler
 	{
-		public static IPropertyMapper<IFlyoutView, IFlyoutViewHandler> Mapper = new PropertyMapper<IFlyoutView, IFlyoutViewHandler>(ViewHandler.ViewMapper)
+		// Like IViewHandler.ContainerView, those properties should be set with priority because other mappers depend on them (like IToolbarElement.Toolbar).
+		// So we have a separate mapper for them.
+		private static readonly IPropertyMapper<IFlyoutView, IFlyoutViewHandler> FlyoutLayoutMapper = new PropertyMapper<IFlyoutView, IFlyoutViewHandler>()
 		{
 #if ANDROID || WINDOWS || TIZEN
 			[nameof(IFlyoutView.Flyout)] = MapFlyout,
 			[nameof(IFlyoutView.Detail)] = MapDetail,
+#endif
+		};
+
+		public static IPropertyMapper<IFlyoutView, IFlyoutViewHandler> Mapper = new PropertyMapper<IFlyoutView, IFlyoutViewHandler>(ViewHandler.ViewMapper, FlyoutLayoutMapper)
+		{
+#if ANDROID || WINDOWS || TIZEN
 			[nameof(IFlyoutView.IsPresented)] = MapIsPresented,
 			[nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior,
 			[nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth,

src/Core/src/Handlers/View/AndroidBatchPropertyMapper.cs

@@ -1,56 +1,51 @@
-using System;
+#if ANDROID
+
+using System;
 using System.Collections.Generic;
 
-namespace Microsoft.Maui.Handlers
-{
-#if ANDROID
-	class AndroidBatchPropertyMapper<TVirtualView, TViewHandler> : PropertyMapper<TVirtualView, TViewHandler>
-		where TVirtualView : IElement
-		where TViewHandler : IElementHandler
-	{
-		// During mass property updates, this list of properties will be skipped
-		public static HashSet<string> SkipList = new(StringComparer.Ordinal)
-		{ 
-			nameof(IView.Visibility),
-			nameof(IView.MinimumHeight),
-			nameof(IView.MinimumWidth),
-			nameof(IView.IsEnabled),
-			nameof(IView.Opacity),
-			nameof(IView.TranslationX),
-			nameof(IView.TranslationY),
-			nameof(IView.Scale),
-			nameof(IView.ScaleX),
-			nameof(IView.ScaleY),
-			nameof(IView.Rotation),
-			nameof(IView.RotationX),
-			nameof(IView.RotationY),
-			nameof(IView.AnchorX),
-			nameof(IView.AnchorY),
-		};
+namespace Microsoft.Maui.Handlers;
 
-		public AndroidBatchPropertyMapper(params IPropertyMapper[] chained) : base(chained) { }
+file class AndroidPropertyMapperInfo
+{
+	public static readonly ISet<string> UpdatePropertiesSkipList = new HashSet<string>(StringComparer.Ordinal)
+	{ 
+		nameof(IView.Visibility),
+		nameof(IView.MinimumHeight),
+		nameof(IView.MinimumWidth),
+		nameof(IView.IsEnabled),
+		nameof(IView.Opacity),
+		nameof(IView.TranslationX),
+		nameof(IView.TranslationY),
+		nameof(IView.Scale),
+		nameof(IView.ScaleX),
+		nameof(IView.ScaleY),
+		nameof(IView.Rotation),
+		nameof(IView.RotationX),
+		nameof(IView.RotationY),
+		nameof(IView.AnchorX),
+		nameof(IView.AnchorY),
+	};
+}
 
-		public override IEnumerable<string> GetKeys()
+class AndroidBatchPropertyMapper<TVirtualView, TViewHandler>(params IPropertyMapper[] chained)
+	: PropertyMapper<TVirtualView, TViewHandler>(chained)
+	where TVirtualView : IElement
+	where TViewHandler : IElementHandler
+{
+	private protected override void UpdatePropertiesCore(IElementHandler viewHandler, IElement virtualView)
+	{
+		foreach (var mapper in MergedMappers)
 		{
-			foreach (var key in _mapper.Keys)
+			// When doing the initial properties batch updates, ignore properties in the skip list.
+			// These will be handled by ViewHandler.SetVirtualView() instead.
+			if (AndroidPropertyMapperInfo.UpdatePropertiesSkipList.Contains(mapper.Key))
 			{
-				// When reporting the key list for mass updates up the chain, ignore properties in SkipList.
-				// These will be handled by ViewHandler.SetVirtualView() instead.
-				if (SkipList.Contains(key))
-				{
-					continue;
-				}
-
-				yield return key;
+				continue;
 			}
 
-			if (Chained is not null)
-			{
-				foreach (var chain in Chained)
-					foreach (var key in chain.GetKeys())
-						yield return key;
-			}
+			mapper.Value(viewHandler, virtualView);
 		}
 	}
-#endif
 }
+
+#endif

src/Core/src/Handlers/View/ViewHandler.cs

@@ -31,6 +31,9 @@ public abstract partial class ViewHandler : ElementHandler, IViewHandler
 			new PropertyMapper<IView, IViewHandler>(ElementHandler.ElementMapper)
 #endif
 			{
+				// This property is a special one and needs to be set before other properties.
+				[nameof(IViewHandler.ContainerView)] = MapContainerView,
+
 				[nameof(IView.AutomationId)] = MapAutomationId,
 				[nameof(IView.Clip)] = MapClip,
 				[nameof(IView.Shadow)] = MapShadow,
@@ -56,7 +59,6 @@ public abstract partial class ViewHandler : ElementHandler, IViewHandler
 				[nameof(IView.RotationY)] = MapRotationY,
 				[nameof(IView.AnchorX)] = MapAnchorX,
 				[nameof(IView.AnchorY)] = MapAnchorY,
-				[nameof(IViewHandler.ContainerView)] = MapContainerView,
 #pragma warning disable CS0618 // Type or member is obsolete
 				[nameof(IBorder.Border)] = MapBorderView,
 #pragma warning restore CS0618 // Type or member is obsolete

src/Core/src/Platform/iOS/ContainerViewController.cs

@@ -68,7 +68,10 @@ public override void LoadView()
 
 		void LoadPlatformView(IElement view)
 		{
-			currentPlatformView = _pendingLoadedView ?? CreatePlatformView(view);
+			var platformView = _pendingLoadedView ?? CreatePlatformView(view);
+			platformView = platformView.Superview as WrapperView ?? platformView;
+
+			currentPlatformView = platformView;
 			_pendingLoadedView = null;
 
 			View!.AddSubview(currentPlatformView);

src/Core/src/PropertyMapper.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 
 #if IOS || MACCATALYST
 using PlatformView = UIKit.UIView;
@@ -15,13 +16,16 @@ namespace Microsoft.Maui
 {
 	public abstract class PropertyMapper : IPropertyMapper
 	{
+		// TODO: Make this private in .NET10
 		protected readonly Dictionary<string, Action<IElementHandler, IElement>> _mapper = new(StringComparer.Ordinal);
 
 		IPropertyMapper[]? _chained;
 
-		// Keep a distinct list of the keys so we don't run any duplicate (overridden) updates more than once
-		// when we call UpdateProperties
-		HashSet<string>? _updateKeys;
+		IReadOnlyDictionary<string, Action<IElementHandler, IElement>>? _mergedMappers;
+		private protected IReadOnlyDictionary<string, Action<IElementHandler, IElement>> MergedMappers => _mergedMappers ?? SnapshotMappers().Mappers;
+
+		IReadOnlyList<string>? _mergedKeys;
+		IReadOnlyList<string> MergedKeys => _mergedKeys ?? SnapshotMappers().Keys;
 
 		public PropertyMapper()
 		{
@@ -35,29 +39,48 @@ public PropertyMapper(params IPropertyMapper[]? chained)
 		protected virtual void SetPropertyCore(string key, Action<IElementHandler, IElement> action)
 		{
 			_mapper[key] = action;
+
 			ClearKeyCache();
 		}
 
+		// TODO: Remove in .NET10
 		protected virtual void UpdatePropertyCore(string key, IElementHandler viewHandler, IElement virtualView)
 		{
 			if (!viewHandler.CanInvokeMappers())
+			{
 				return;
+			}
 
-			var action = GetProperty(key);
-			action?.Invoke(viewHandler, virtualView);
+			TryUpdatePropertyCore(key, viewHandler, virtualView);
+		}
+
+		internal bool TryUpdatePropertyCore(string key, IElementHandler viewHandler, IElement virtualView)
+		{
+			if (MergedMappers.TryGetValue(key, out var action))
+			{
+				action(viewHandler, virtualView);
+				return true;
+			}
+
+			return false;
 		}
 
 		public virtual Action<IElementHandler, IElement>? GetProperty(string key)
 		{
 			if (_mapper.TryGetValue(key, out var action))
+			{
 				return action;
-			else if (Chained is not null)
+			}
+
+			if (Chained is not null)
 			{
 				foreach (var ch in Chained)
 				{
 					var returnValue = ch.GetProperty(key);
 					if (returnValue != null)
+					{
 						return returnValue;
+					}
 				}
 			}
 
@@ -66,20 +89,32 @@ protected virtual void UpdatePropertyCore(string key, IElementHandler viewHandle
 
 		public void UpdateProperty(IElementHandler viewHandler, IElement? virtualView, string property)
 		{
-			if (virtualView == null)
+			if (virtualView == null || !viewHandler.CanInvokeMappers())
+			{
 				return;
+			}
 
-			UpdatePropertyCore(property, viewHandler, virtualView);
+			if (MergedMappers.TryGetValue(property, out var action))
+			{
+				action(viewHandler, virtualView);
+			}
 		}
 
 		public void UpdateProperties(IElementHandler viewHandler, IElement? virtualView)
 		{
-			if (virtualView == null)
+			if (virtualView == null || !viewHandler.CanInvokeMappers())
+			{
 				return;
+			}
 
-			foreach (var key in UpdateKeys)
+			UpdatePropertiesCore(viewHandler, virtualView);
+		}
+
+		private protected virtual void UpdatePropertiesCore(IElementHandler viewHandler, IElement virtualView)
+		{
+			foreach (var mapper in MergedMappers)
 			{
-				UpdatePropertyCore(key, viewHandler, virtualView);
+				mapper.Value(viewHandler, virtualView);
 			}
 		}
 
@@ -93,35 +128,55 @@ public IPropertyMapper[]? Chained
 			}
 		}
 
-		private HashSet<string> PopulateKeys()
-		{
-			var keys = new HashSet<string>(StringComparer.Ordinal);
-			foreach (var key in GetKeys())
-			{
-				keys.Add(key);
-			}
-			return keys;
-		}
-
+		// TODO: Make private in .NET10 with a new name: ClearMergedMappers
 		protected virtual void ClearKeyCache()
 		{
-			_updateKeys = null;
+			_mergedMappers = null;
+			_mergedKeys = null;
 		}
 
-		public virtual IReadOnlyCollection<string> UpdateKeys => _updateKeys ??= PopulateKeys();
+		// TODO: Remove in .NET10
+		public virtual IReadOnlyCollection<string> UpdateKeys => MergedKeys;
 
 		public virtual IEnumerable<string> GetKeys()
 		{
-			foreach (var key in _mapper.Keys)
-				yield return key;
-
+			// We want to retain the initial order of the keys to avoid race conditions
+			// when a property mapping is overridden by a new instance of property mapper.
+			// As an example, the container view mapper should always run first.
+			// Siblings mapper should not have keys intersection.
 			if (Chained is not null)
 			{
-				foreach (var chain in Chained)
-					foreach (var key in chain.GetKeys())
+				for (int i = Chained.Length - 1; i >= 0; i--)
+				{
+					foreach (var key in Chained[i].GetKeys())
+					{
 						yield return key;
+					}
+				}
+			}
+
+			// Enqueue keys from this mapper.
+			foreach (var mapper in _mapper)
+			{
+				yield return mapper.Key;
 			}
 		}
+
+		private (List<string> Keys, Dictionary<string, Action<IElementHandler, IElement>> Mappers) SnapshotMappers()
+		{
+			var keys = GetKeys().Distinct().ToList();
+
+			var mappers = new Dictionary<string, Action<IElementHandler, IElement>>(keys.Count);
+			foreach (var key in keys)
+			{
+				mappers[key] = GetProperty(key)!;
+			}
+
+			_mergedKeys = keys;
+			_mergedMappers = mappers;
+
+			return (keys, mappers);
+		}
 	}
 
 	public interface IPropertyMapper
@@ -169,12 +224,22 @@ public void Add(string key, Action<TViewHandler, TVirtualView> action) =>
 			SetPropertyCore(key, (h, v) =>
 			{
 				if (v is TVirtualView vv)
+				{
 					action?.Invoke((TViewHandler)h, vv);
+				}
 				else if (Chained != null)
 				{
 					foreach (var chain in Chained)
 					{
-						if (chain.GetProperty(key) != null)
+						// Try to leverage our internal method which uses merged mappers
+						if (chain is PropertyMapper propertyMapper)
+						{
+							if (propertyMapper.TryUpdatePropertyCore(key, h, v))
+							{
+								break;
+							}
+						}
+						else if (chain.GetProperty(key) != null)
 						{
 							chain.UpdateProperty(h, v, key);
 							break;