From 24263d47405fd82729dde5e71b0c9978eb3276b5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 22 Jun 2022 14:24:42 -0500 Subject: [PATCH] [layout] avoid IView[] creation in LayoutExtensions Context: https://github.com/unoplatform/performance/tree/master/src/dopes/DopeTestMaui Reviewing `dotnet trace` output of the "dopes" test sample, 7% of CPU time is spent in `LayoutExtensions.OrderByZIndex()`: 3.42s (16%) microsoft.maui!Microsoft.Maui.Handlers.LayoutHandler.Add(Microsoft.Maui.IView) 1.52s (7.3%) microsoft.maui!Microsoft.Maui.Handlers.LayoutExtensions.GetLayoutHandlerIndex(Microsoft.Maui.ILayout,Microsoft.Maui.IVie... 1.50s (7.2%) microsoft.maui!Microsoft.Maui.Handlers.LayoutExtensions.OrderByZIndex(Microsoft.Maui.ILayout) Reviewing the code in `OrderByZIndex()`: * Makes a `record ViewAndIndex(IView View, int Index)` for each child * Makes a `ViewAndIndex[]` the size of the number of children * Call `Array.Sort()` * Makes a `IView[]` the size of the number of children * Iterating twice over the arrays in the process Then if you look at the `GetLayoutHandlerIndex()` method, it does all the same work to create an `IView[]`, get the index, then throws the array away. I removed the above code and used a System.Linq `OrderBy()` with faster paths for 0 or 1 children in `GetLayoutHandlerIndex()`. I also made `OrderByZIndex()` return `IOrderedEnumerable` for use by `foreach` loops. This avoids creating arrays in those cases. The `ZIndexTests` still pass for me, which proves out `OrderBy()`'s stable sort: https://docs.microsoft.com/dotnet/api/system.linq.enumerable.orderby After this change, the % time spent in these methods went down: 1.84s (13%) microsoft.maui!Microsoft.Maui.Handlers.LayoutHandler.Add(Microsoft.Maui.IView) 352.24ms (2.50%) microsoft.maui!Microsoft.Maui.Handlers.LayoutExtensions.GetLayoutHandlerIndex(Microsoft.Maui.ILayout,Microsoft.Maui.IVie... 181.27ms (1.30%) microsoft.maui!Microsoft.Maui.Handlers.LayoutExtensions.<>c.b__0_0(Microsoft.Maui.IView) 2.78ms (0.02%) microsoft.maui!Microsoft.Maui.Handlers.LayoutExtensions.EnumerateByZIndex(Microsoft.Maui.ILayout) This kind of goes against the guidance "avoid System.Linq". But `OrderBy()` generally seems to be fine? ~~ Results ~~ A `Release` build on a Pixel 5 device, I was getting: Before: 103.04 Dopes/s After: 230.87 Dopes/s I suspect that was a lot of `IView[]`'s before! --- .../src/Handlers/Layout/LayoutExtensions.cs | 56 ++++--------------- .../tests/UnitTests/Layouts/ZIndexTests.cs | 9 +-- 2 files changed, 15 insertions(+), 50 deletions(-) diff --git a/src/Core/src/Handlers/Layout/LayoutExtensions.cs b/src/Core/src/Handlers/Layout/LayoutExtensions.cs index 14d5acff2749..d7ee7809fbff 100644 --- a/src/Core/src/Handlers/Layout/LayoutExtensions.cs +++ b/src/Core/src/Handlers/Layout/LayoutExtensions.cs @@ -1,59 +1,23 @@ using System; -using System.Collections.Generic; +using System.Linq; namespace Microsoft.Maui.Handlers { internal static class LayoutExtensions { - record ViewAndIndex(IView View, int Index); + public static IOrderedEnumerable OrderByZIndex(this ILayout layout) => layout.OrderBy(v => v.ZIndex); - class ZIndexComparer : IComparer - { - public int Compare(ViewAndIndex? x, ViewAndIndex? y) - { - if (x == null || y == null) - { - return 0; - } - - var zIndexCompare = x.View.ZIndex.CompareTo(y.View.ZIndex); - - if (zIndexCompare == 0) - { - return x.Index.CompareTo(y.Index); - } - - return zIndexCompare; - } - } - - static ZIndexComparer s_comparer = new(); - - public static IView[] OrderByZIndex(this ILayout layout) + public static int GetLayoutHandlerIndex(this ILayout layout, IView view) { - var count = layout.Count; - var indexedViews = new ViewAndIndex[count]; - - for (int n = 0; n < count; n++) + switch (layout.Count) { - indexedViews[n] = new ViewAndIndex(layout[n], n); + case 0: + return -1; + case 1: + return view == layout[0] ? 0 : -1; + default: + return layout.OrderByZIndex().IndexOf(view); } - - Array.Sort(indexedViews, s_comparer); - - var ordered = new IView[count]; - - for (int n = 0; n < count; n++) - { - ordered[n] = indexedViews[n].View; - } - - return ordered; - } - - public static int GetLayoutHandlerIndex(this ILayout layout, IView view) - { - return layout.OrderByZIndex().IndexOf(view); } } } diff --git a/src/Core/tests/UnitTests/Layouts/ZIndexTests.cs b/src/Core/tests/UnitTests/Layouts/ZIndexTests.cs index 5127df109de6..0395470f4062 100644 --- a/src/Core/tests/UnitTests/Layouts/ZIndexTests.cs +++ b/src/Core/tests/UnitTests/Layouts/ZIndexTests.cs @@ -1,5 +1,6 @@ using System.Collections; using System.Collections.Generic; +using System.Linq; using Microsoft.Maui.Graphics; using Microsoft.Maui.Handlers; using Microsoft.Maui.Primitives; @@ -209,7 +210,7 @@ public void ItemsOrderByZIndex() layout.Add(view0); layout.Add(view1); - var zordered = layout.OrderByZIndex(); + var zordered = layout.OrderByZIndex().ToArray(); Assert.Equal(view1, zordered[0]); Assert.Equal(view0, zordered[1]); } @@ -228,7 +229,7 @@ public void ZIndexUpdatePreservesAddOrderForEqualZIndexes() layout.Add(view2); layout.Add(view3); - var zordered = layout.OrderByZIndex(); + var zordered = layout.OrderByZIndex().ToArray(); Assert.Equal(view0, zordered[0]); Assert.Equal(view1, zordered[1]); Assert.Equal(view2, zordered[2]); @@ -237,7 +238,7 @@ public void ZIndexUpdatePreservesAddOrderForEqualZIndexes() // Fake an update view3.ZIndex.Returns(5); - zordered = layout.OrderByZIndex(); + zordered = layout.OrderByZIndex().ToArray(); Assert.Equal(view0, zordered[0]); Assert.Equal(view1, zordered[1]); Assert.Equal(view2, zordered[2]); @@ -267,7 +268,7 @@ public void ZIndexUpdatePreservesAddOrderLotsOfEqualZIndexes() for (int i = 0; i < iterations; i++) { - var zordered = layout.OrderByZIndex(); + var zordered = layout.OrderByZIndex().ToArray(); for (int n = 0; n < zordered.Length; n++) {