Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip initial assignments in property mapper (take 4) #27042

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Jan 9, 2025

Credits to @albyrock87 for helping me refactor the PR to be acceptable for .NET 9!

Description of Change

This is very much work in progress but it seems to show promising performance implications1.

Introduction to the problem

To understand this PR, it is good to know that each virtual view (e.g., MAUI's <Entry>) is mapped to a platform view (e.g., <TextBox> on Windows) on each supported platform2.

Now when a XAML page with a single <Entry Text="Hi" /> element is to be to displayed to user by MAUI, MAUI creates a new platform view (i.e. TextBox on Windows; see doc).

In a next step, MAUI must make sure that all properties of <Entry> are set (mirrored?) to the new TextBox instance. So MAUI maps all properties of the virtual view (<Entry>) to corresponding properties of TextBox. So in simple words, what happens when MAUI initializes a platform view is the following (pseudocode):

TextBox platformView = new();

// Map things like: Width, Height, Opacity, IsVisible, Padding, Margin and many others.
foreach (string propertyName in virtualView.Properties) {
   MapProperty(propertyName, virtualView, platformView);
}

(Code Listing 1)

It's technically more complicated but this is the gist. Now what #17303 observes is that the mapping is not always perfect. One can see it here:

public static void MapTranslationX(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapTranslationY(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapScale(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapScaleX(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapScaleY(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapRotation(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapRotationX(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapRotationY(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapAnchorX(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}
public static void MapAnchorY(IViewHandler handler, IView view)
{
handler.ToPlatform().UpdateTransformation(view);
}

So suppose that a new <Entry> is to be added to screen by MAUI on Windows, then MAUI calls on a new TextBox mapping methods MapTranslationX, MapTranslationY, MapScale, etc. which all do the same work. So it's clear it can be called just once to achieve the same.

One might think that it's not a big deal to set, e.g., platformView.Projection = null (like here)3. And that's actually a big deal because each such call is an interop call. Interop calls much more costly than simple .NET calls4. In a sense, it's a death by tousand cuts because as a MAUI app gets more complex, everything takes longer and longer to display. So each removed interop call counts because it expands how big MAUI app one can implement5

How to MAUI improve performance?

To improve MAUI performance, one needs to decrease the number of interop calls needed to display a single MAUI window, that is something like:

#virtualViews * #properties ~= virtualViews * 50

Previously tried approaches

This PR

  • This PR is similar to the previous approach ([Windows] Skip initial assignments in property mapper (take 3) #23987) but the idea is to decide if a mapper is to be called in PlatformMapper itself and not in platform methods as the previous approach. So we decide "sooner" and this approach is more "centralistic". It IMO follows this comment by @PureWeen.
    • An advantage is that one can define that, e.g., opacity = 1 is default value and if a platform view is instantiated, then when property mapper is called for the first time for that platform view, we don't need to assign opacity = 1 and many others because arguably almost all properties are set to default values and just a few are set to "useful values" (e.g. Entry.Text = "Hi" but margin = 0, padding = 0, opacity =1, etc. etc.).
    • Arguably this approach has bigger impact than previous approaches, so I tend to think it's reasonable.

Performance impact

image

Main:

animation

PR:

animation

Speedscopes:

Search for Handler.Map

Issues Fixed

Fixes #17303

Footnotes

  1. As expected.

  2. Read Windows, Mac Catalyst, Android, iOS, etc.

  3. Interestingly, setting some platform view properties (like RenderTransform in WinUI) can be much more costly than others. It seems that some parts of WinUI are much less optimized than others.

  4. MAUI on Windows is based on WinUI framework and WinRT is used for interop calls (see how costly is accessing Panel.Children here). On Android, one needs to do interop calls as Android apps runs on java virtual machines (JVMs).

  5. I'm not talking about something takes a microsecond, I'm talking about a grid that one can display in 1 second or in 100 ms (see (see https://github.com/dotnet/maui/issues/21787#issuecomment-2453520427 to get some sense what I'm talking about). So this issue is about a real-world issue where even mid-size apps can suffer from poor performance.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jan 9, 2025
@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch from 279a2aa to 7467d42 Compare January 9, 2025 19:47
@bcaceiro
Copy link

Amazing work @MartyIX . This should be a top priority for the team to review + come up with a feasible solution targeting .net9. One of the major issues that everyone is always talking about is performance. CV, loading pages, etc.

This seems very promising to instantly improve performance without changes in a codebase ( again from a user/customer pov)

Also : it would be helpful I guess to have a generic method to count the interop calls and see the impact ( not sure if already exists)

@MartyIX
Copy link
Contributor Author

MartyIX commented Jan 10, 2025

Also : it would be helpful I guess to have a generic method to count the interop calls and see the impact ( not sure if already exists)

I added a simple comparison to the OP. It shows that by removing setting padding one saved about 180 ms in total when I displayed my grid 10 times. So that property alone seems to shave off 18 ms (more or less) for each display of my grid. This PR is really suboptimal but I think it saves ~50 ms to display each grid. For me

  • 50 ms is better than nothing
  • 100 ms okay-ish
  • 200 ms jackpot (because we would get to the perf of WinUI itself)

We will see what we can get. Again, this is not finished.

To answer your question, I'm not aware of a way to count interop calls precisely but https://www.speedscope.app/ gives good enough estimation of improvements so I'm currently happy with it.

@marcojak
Copy link

Interesting one and definitely something for the team to investigate further for its potential.
Good work.

@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch 12 times, most recently from b9d08bc to 84a903d Compare January 11, 2025 11:20
@MartyIX MartyIX changed the title [no merge] Skip initial assignments in property mapper (take 4) Skip initial assignments in property mapper (take 4) Jan 11, 2025
@MartyIX MartyIX marked this pull request as ready for review January 11, 2025 11:26
@MartyIX MartyIX requested a review from a team as a code owner January 11, 2025 11:26
@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch 3 times, most recently from b48688f to cbe82e5 Compare January 11, 2025 19:07
@PureWeen

This comment was marked as outdated.

This comment was marked as outdated.

@PureWeen

This comment was marked as outdated.

@rmarinho

This comment was marked as outdated.

This comment was marked as outdated.

@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch from e3801fb to 512aefc Compare January 14, 2025 20:06
@PureWeen
Copy link
Member

PureWeen commented Jan 14, 2025

/azp run

This comment was marked as outdated.

@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch from 512aefc to 55dd718 Compare January 15, 2025 07:51
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch 3 times, most recently from 38a66a6 to 9f302a7 Compare January 15, 2025 18:50
@mattleibow
Copy link
Member

mattleibow commented Jan 15, 2025

/azp run

This comment was marked as outdated.

@MartyIX MartyIX force-pushed the feature/2025-01-09-Skip-initial-assignments branch from 9f302a7 to eefd52a Compare January 16, 2025 13:33
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[performance] all PropertyMapper's set the same values over and over
7 participants