Skip to content

Commit

Permalink
[ios] avoid duplicating CALayer.Sublayer arrays (dotnet#21308)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonathanpeppers authored Mar 19, 2024
1 parent fde1cff commit 64deb0f
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,11 @@ public static void RemoveBackgroundLayer(this CALayer layer)
if (layer.Name == BackgroundLayer)
layer?.RemoveFromSuperLayer();

if (layer.Sublayers == null || layer.Sublayers.Count() == 0)
var sublayers = layer.Sublayers;
if (sublayers is null || sublayers.Length == 0)
return;

foreach (var subLayer in layer.Sublayers)
foreach (var subLayer in sublayers)
{
if (subLayer.Name == BackgroundLayer)
subLayer?.RemoveFromSuperLayer();
Expand Down
10 changes: 6 additions & 4 deletions src/Controls/src/Core/Platform/iOS/Extensions/BrushExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,11 @@ public static void RemoveBackgroundLayer(this CALayer layer)
if (layer.Name == BackgroundLayer)
layer?.RemoveFromSuperLayer();

if (layer.Sublayers == null || layer.Sublayers.Count() == 0)
var sublayers = layer.Sublayers;
if (sublayers is null || sublayers.Length == 0)
return;

foreach (var subLayer in layer.Sublayers)
foreach (var subLayer in sublayers)
{
if (subLayer.Name == BackgroundLayer)
subLayer?.RemoveFromSuperLayer();
Expand All @@ -187,9 +188,10 @@ public static void UpdateBackgroundLayer(this UIView view)

static void UpdateBackgroundLayer(this CALayer layer, CGRect bounds)
{
if (layer != null && layer.Sublayers != null)
var sublayers = layer?.Sublayers;
if (sublayers is not null)
{
foreach (var sublayer in layer.Sublayers)
foreach (var sublayer in sublayers)
{
UpdateBackgroundLayer(sublayer, bounds);

Expand Down
5 changes: 3 additions & 2 deletions src/Core/src/Platform/iOS/LayerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ public static void RemoveBackgroundLayer(this UIView control)
return;
}

if (layer.Sublayers == null || layer.Sublayers.Length == 0)
var sublayers = layer.Sublayers;
if (sublayers is null || sublayers.Length == 0)
return;

foreach (var subLayer in layer.Sublayers)
foreach (var subLayer in sublayers)
{
if (subLayer.Name == ViewExtensions.BackgroundLayerName)
{
Expand Down
5 changes: 3 additions & 2 deletions src/Core/src/Platform/iOS/StrokeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,10 @@ internal static void UpdateMauiCALayer(this UIView view)

static void UpdateBackgroundLayer(this CALayer layer, CGRect bounds)
{
if (layer != null && layer.Sublayers != null)
var sublayers = layer?.Sublayers;
if (sublayers is not null)
{
foreach (var sublayer in layer.Sublayers)
foreach (var sublayer in sublayers)
{
UpdateBackgroundLayer(sublayer, bounds);

Expand Down
7 changes: 3 additions & 4 deletions src/Core/src/Platform/iOS/ViewExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,11 @@ public static void UpdateBackgroundLayerFrame(this UIView view)
if (view == null || view.Frame.IsEmpty)
return;

var layer = view.Layer;

if (layer == null || layer.Sublayers == null || layer.Sublayers.Length == 0)
var sublayers = view.Layer?.Sublayers;
if (sublayers is null || sublayers.Length == 0)
return;

foreach (var sublayer in layer.Sublayers)
foreach (var sublayer in sublayers)
{
if (sublayer.Name == BackgroundLayerName && sublayer.Frame != view.Bounds)
{
Expand Down
10 changes: 6 additions & 4 deletions src/Core/src/Platform/iOS/WrapperView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,11 @@ void DisposeBorder()

CALayer? GetLayer()
{
if (Layer is null || Layer.Sublayers is null)
var sublayers = Layer?.Sublayers;
if (sublayers is null)
return null;

foreach (var subLayer in Layer.Sublayers)
foreach (var subLayer in sublayers)
if (subLayer.Delegate is not null)
return subLayer;

Expand All @@ -267,10 +268,11 @@ void DisposeBorder()

CALayer? GetBackgroundLayer()
{
if (Layer is null || Layer.Sublayers is null)
var sublayers = Layer?.Sublayers;
if (sublayers is null)
return null;

foreach (var subLayer in Layer.Sublayers)
foreach (var subLayer in sublayers)
if (subLayer.Name == ViewExtensions.BackgroundLayerName)
return subLayer;

Expand Down
5 changes: 3 additions & 2 deletions src/Essentials/src/Screenshot/Screenshot.ios.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,11 @@ static bool TryRender(CALayer layer, CGContext ctx, bool skipChildren, out Excep

static void HideSublayers(CALayer layer, Dictionary<CALayer, bool> visibilitySnapshot)
{
if (layer.Sublayers == null)
var sublayers = layer?.Sublayers;
if (sublayers is null)
return;

foreach (var sublayer in layer.Sublayers)
foreach (var sublayer in sublayers)
{
HideSublayers(sublayer, visibilitySnapshot);

Expand Down

0 comments on commit 64deb0f

Please sign in to comment.