Skip to content

Conversation

@jonathanpeppers
Copy link
Member

Context: https://discord.com/channels/732297728826277939/732297808148824115/1219383992067821608

In a customer's application, they saw in Xcode's Instruments:

4.4% Microsoft_Maui_Platform_StokeExtensions_UpdateBackgroundLayer_CoreAnimation_CALayer_CoreGraphics_CGRect
>   2.7% CoreAnimation_CALayer_get_Sublayers

Reviewing the code, there is a performance problem:

if (layer != null && layer.Sublayers != null)
{
    foreach (var sublayer in layer.Sublayers)
    {
        //...
    }
}

This accesses CALayer.Sublayers twice:

  1. null check

  2. foreach

When C# calls into Objective-C here, the returned array is marshaled and copied to a C# array that can be consumed in managed code. We are doing this work twice!

I audited all calls to CALayer.Sublayers and found that we can avoid this in places that are called frequently.

This should improve the performance of all views on iOS or Catalyst. However, I don't have access to the customer's application to test the difference. We can have them try a nightly MAUI build after this is merged.

In a customer's application, they saw in Xcode's Instruments:

    4.4% Microsoft_Maui_Platform_StokeExtensions_UpdateBackgroundLayer_CoreAnimation_CALayer_CoreGraphics_CGRect
    >   2.7% CoreAnimation_CALayer_get_Sublayers

Reviewing the code, there is a performance problem:

    if (layer != null && layer.Sublayers != null)
    {
        foreach (var sublayer in layer.Sublayers)
        {
            //...
        }
    }

This accesses `CALayer.Sublayers` twice:

1. null check

2. `foreach`

When C# calls into Objective-C here, the returned array is marshaled
and copied to a C# array that can be consumed in managed code. We are
doing this work twice!

I audited all calls to `CALayer.Sublayers` and found that we can avoid
this in places that are called frequently.

This should improve the performance of all views on iOS or Catalyst.
However, I don't have access to the customer's application to test the
difference. We can have them try a nightly MAUI build after this is
merged.
@jonathanpeppers jonathanpeppers added platform/ios legacy-area-perf Startup / Runtime performance labels Mar 19, 2024
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner March 19, 2024 14:15
@jonathanpeppers jonathanpeppers added the partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with label Mar 19, 2024
@rmarinho rmarinho merged commit 64deb0f into dotnet:main Mar 19, 2024
@jonathanpeppers jonathanpeppers deleted the Sublayers branch March 19, 2024 18:17
jonathanpeppers added a commit that referenced this pull request Apr 10, 2024
Context: #21580
Similar to: #21308

Recording this app in `dotnet-trace`, I saw:

    (0.81%)	Microsoft.MacCatalyst!UIKit.UIView.get_Subviews()

Where this time is spent in:

* `Microsoft.Maui!Microsoft.Maui.Platform.WrapperView`

Reviewing the code, it calls `this.Subviews` twice, which would marshal
and create two arrays. It then calls `Subviews[0]`.

Storing in a local instead will avoid creating the array twice:

    var subviews = Subviews;
    if (subviews.Length == 0)
        return;
    //...
    var child = subviews[0];

This improves things a small amount, but is probably worth it:

    (0.72%)	Microsoft.MacCatalyst!UIKit.UIView.get_Subviews()

I'll think about making an analyzer for this, but  I didn't find any
similar cases that appear in `dotnet-trace` at the moment.

This doesn't fully solve #21580, but it's a small improvement.
rmarinho pushed a commit that referenced this pull request Apr 11, 2024
Context: #21580
Similar to: #21308

Recording this app in `dotnet-trace`, I saw:

    (0.81%)	Microsoft.MacCatalyst!UIKit.UIView.get_Subviews()

Where this time is spent in:

* `Microsoft.Maui!Microsoft.Maui.Platform.WrapperView`

Reviewing the code, it calls `this.Subviews` twice, which would marshal
and create two arrays. It then calls `Subviews[0]`.

Storing in a local instead will avoid creating the array twice:

    var subviews = Subviews;
    if (subviews.Length == 0)
        return;
    //...
    var child = subviews[0];

This improves things a small amount, but is probably worth it:

    (0.72%)	Microsoft.MacCatalyst!UIKit.UIView.get_Subviews()

I'll think about making an analyzer for this, but  I didn't find any
similar cases that appear in `dotnet-trace` at the moment.

This doesn't fully solve #21580, but it's a small improvement.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2024
@Eilon Eilon added the perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-8.0.20 fixed-in-9.0.0-preview.3.10457 legacy-area-perf Startup / Runtime performance partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) platform/ios

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants