Improve PropertyMapper performance

[albyrock87]

Description of Change

• Improve PropertyMapper performance by caching mappers (same thing is currently done on mapper's keys)
  • Moves viewHandler.CanInvokeMappers() check to UpdateProperties level considering that on Android this method method uses the Java object
• Ensures properties are executed in the order they're defined (this will help with future perf developments Skip useless handler mappings calls while connecting #27259)
  • Brings to light race conditions now present on a few mappers (like MapToolbar depends on MapFlyout) and formally solves them by setting the right order of execution

Benchmarks

BenchmarkDotNet v0.13.10, macOS 15.3.1 (24D70) [Darwin 24.3.0]
Apple M3 Pro, 1 CPU, 11 logical and 11 physical cores
.NET SDK 9.0.102

Before

Method	Mean	Error	StdDev	Gen0	Allocated
BenchmarkUpdateProperties	167.67 ms	0.584 ms	0.547 ms	3333.3333	29604261 B
BenchmarkUpdateProperty	31.65 ms	0.079 ms	0.070 ms	-	46 B

After

Method	Mean	Error	StdDev	Gen0	Allocated
BenchmarkUpdateProperties	59.04 ms	0.294 ms	0.275 ms	3666.6667	31204122 B
BenchmarkUpdateProperty	12.39 ms	0.026 ms	0.020 ms	-	12 B

[MartyIX]

There is OrderedDictionary in .NET 9 and I wonder if it is useful here or somewhere else.

(I wanted to take a look at "ordering of mapper calls" for a very long time. Mostly to make sure that a wrapper view is created by the very first mapper call. I'm very happy that you beat me in working on this. :))

[albyrock87]

That one is not generic, so it comes with the cost of casting everything, and I also think it has a cost in performance compared to the generic implementation.

That said, we'd have to change _mapper too in that case (which is a public API unfortunately), or add an additional List<string> _keys property to use while inserting new mappers.

I think we can safely bet on this given it's covered by unit tests, if it changes we can switch to something else.

[MartyIX]

I modified the link. It's generic.

Anyway, it's fine by me. I just wanted to mention that the ordered variant exists now. If it used or not, it's not as relevant as if all design choices were considered.

[mattleibow]

It is late, and I am reviewing with my brain turned off. But I got some questions that maybe I wouldn't have asked in the morning. But the PR looks nice.

[albyrock87]

@mattleibow I've updated the PR, including the fix for the broken tests: I was totally missing the special AndroidBatchPropertyMapper.

Also I backported the race-condition fix on iOS ContainerViewController I've done in the other PR.

[albyrock87]

For reference: https://github.com/dotnet/maui/pull/3372/files#diff-928414507a5c749dc42759124e98c6bf08bdb6b7dff26549616bfc683a65edec

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[albyrock87]

Note: these failing tests are unrelated:

• mac: UpdatedIsEnabledProperty.png (size differs - baseline is 1920x1080 pixels, actual is 789x563 pixels)
• mac: FlyoutItemTextShouldDisplayProperly.png (size differs - baseline is 1920x1080 pixels, actual is 789x563 pixels)
• mac: RemainingItemsThresholdReachedCommandFired => passed locally

See more at: #28125

[Redth]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[albyrock87]

@mattleibow so I had to refactor the code again due to the AndroidBatchedPropertyMapper.

The introduction of that class changed the meaning of GetKeys

returns the list of defined mappers

to something like:

returns the list of keys to be updated in UpdateProperties

This is really unfortunate because now there's no way to tell what are the defined keys.

Initially I thought of simply moving the skip list into PropertyMapper and restore the original meaning of GetKeys, but turns out that I can't because a parent mapper could re-define a given mapper and therefore requiring it's execution.
For example SwipeViewHandler redefines IsEnabled which is part of the android skip list.

On top of that, if the user ever wanted to define their own IsEnabled at ViewHandler level (or even AppendToMapping), their mapper would never be executed due to the skip list, which is obviously wrong.

I've added unit tests to cover these scenarios.

#27259 will enable us to detect when we're running the initial mapping, so we will be able to remove AndroidBatchedPropertyMapper and just turn every skip-list mapper into a noop during the initial mapping, for example (see more here):

public static void MapVisibility(IViewHandler handler, IView view)
{
#if ANDROID
    if (handler.IsConnectingHandler()) return;
#else
    if (handler.IsConnectingHandler() && view.Visibility == Visibility.Visible) return;
#endif

    if (handler.HasContainer)
        ((PlatformView?)handler.ContainerView)?.UpdateVisibility(view);

This way the user can even define their own mappers ensuring they will be executed on the first mapping.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[albyrock87]

Failing tests look unrelated:

• RegisterTwoDoubleTapHandlersAndCheckNumberOfFiredEventsAsync => passes locally
• CarouselViewShouldRenderCorrectly => it's randomly mismatching by 1px (not only in this PR)
• BindingUpdatesFromInteractiveRefresh => I have no clue, and no Windows machine to verify
• CarouselItemsShouldRenderProperly (windows) => the screenshot differs only due to the horizontal scrollbar

[PureWeen]

/rebase

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

• failing tests are unrelated