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

[WinUI] Perf when creating non-trivial grids (and other visual controls) (take 2) #22108

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml
@@ -1,6 +1,18 @@
<ContentPage
xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:Maui.Controls.Sample"
x:Class="Maui.Controls.Sample.MainPage"
xmlns:local="clr-namespace:Maui.Controls.Sample">
x:DataType="local:MainPage">
<VerticalStackLayout x:Name="myLayout">
<Label x:Name="info" Text="Click a button" VerticalOptions="Center"/>
<Button Text="Clear Grid" Clicked="ClearGrid_Clicked"/>
<Button Text="Generate Grid" Clicked="Button_Clicked"/>
<Entry x:Name="BatchSize" Text="15"/>
<Button Text="Batch Generate Grid" Clicked="BatchGenerate_Clicked"/>

<HorizontalStackLayout x:Name="myGridWrapper">
<Grid x:Name="contentGrid"/>
</HorizontalStackLayout>
</VerticalStackLayout>
</ContentPage>
68 changes: 60 additions & 8 deletions src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml.cs
@@ -1,18 +1,70 @@
using System;
using System.Collections.ObjectModel;
using System.Diagnostics;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Graphics;

namespace Maui.Controls.Sample
namespace Maui.Controls.Sample;

public partial class MainPage : ContentPage
{
public partial class MainPage : ContentPage
const int rowCount = 30;
const int columnCount = 30;

public MainPage()
{
InitializeComponent();
}

private void ClearGrid_Clicked(object sender, EventArgs e)
{
Stopwatch sw = Stopwatch.StartNew();

contentGrid.Clear();

sw.Stop();

info.Text = $"Clearing grid took: {sw.ElapsedMilliseconds} ms";
}

private void Button_Clicked(object sender, EventArgs e)
{
public MainPage()
Stopwatch sw = Stopwatch.StartNew();
contentGrid.Clear();

for (int rowIndex = 0; rowIndex < rowCount; rowIndex++)
{
InitializeComponent();
for (int columnIndex = 0; columnIndex < columnCount; columnIndex++)
{
Label label = new Label() { Text = $"[{columnIndex}x{rowIndex}]" };
contentGrid.Add(label, column: columnIndex, row: rowIndex);
}
}

sw.Stop();
info.Text = $"Clearing grid took: {sw.ElapsedMilliseconds} ms";
}

private void BatchGenerate_Clicked(object sender, EventArgs e)
{
Stopwatch sw = Stopwatch.StartNew();

int batchSize = int.Parse(BatchSize.Text);

for (int i = 0; i < batchSize; i++)
{
contentGrid.Clear();

for (int rowIndex = 0; rowIndex < rowCount; rowIndex++)
{
for (int columnIndex = 0; columnIndex < columnCount; columnIndex++)
{
Label label = new Label() { Text = $"[{columnIndex}x{rowIndex}]" };
contentGrid.Add(label, column: columnIndex, row: rowIndex);
}
}
}

sw.Stop();
info.Text = $"Grid was created {batchSize} times and it took {sw.ElapsedMilliseconds} ms in total. Avg run took {Math.Round(sw.ElapsedMilliseconds / (double)batchSize, 2)} ms";
}

}
Expand Up @@ -5,4 +5,7 @@
xmlns:maui="using:Microsoft.Maui"
xmlns:local="using:Maui.Controls.Sample.Platform">

<maui:MauiWinUIApplication.Resources>
<x:Double x:Key="ControlContentThemeFontSize">14</x:Double>
</maui:MauiWinUIApplication.Resources>
</maui:MauiWinUIApplication>
2 changes: 1 addition & 1 deletion src/Controls/src/Core/Controls.Core.csproj
Expand Up @@ -10,7 +10,7 @@
<GitInfoReportImportance>high</GitInfoReportImportance>
<MauiGenerateResourceDesigner>true</MauiGenerateResourceDesigner>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);CS1591;RS0041;RS0026;RS0027;RS0022</NoWarn>
<NoWarn>$(NoWarn);CS1591;RS0041;RS0026;RS0027;RS0016;RS0017;RS0022</NoWarn>
<NoWarn Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'tizen' or $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">$(NoWarn);CA1420</NoWarn>
</PropertyGroup>

Expand Down
2 changes: 2 additions & 0 deletions src/Controls/src/Core/Element/Element.cs
Expand Up @@ -91,6 +91,8 @@ public string AutomationId
}
}

public bool IsPlatformViewNew { get; set; }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is terrible. It should be internal if possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a way to make this internal if it is a property on IElement.

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

I don't think I can simulate it that way because I need to set IsPlatformViewNew to true and then back to false:

an alternative which I would consider the best (in an ideal world) would be to pass a flag to:

_mapper.UpdateProperties(this, VirtualView);

but unfortunately then I would need to propagate that flag here:

action?.Invoke(viewHandler, virtualView);

and that would mean changing the way mappers are defined:

protected readonly Dictionary<string, Action<IElementHandler, IElement>> _mapper = new(StringComparer.Ordinal);

which seems like a very invasive thing to do (with many and many changes required). That's why I went with the IView.IsPlatformViewNew flag to avoid making changes to mappers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a way to make this internal if it is a property on IElement.

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

I was thinking about this a bit more and one alternative idea would be: Do not add Element.IsPlatformViewNew but rather add two new mapper calls:

  • InitializingNewElement
  • NoLongerInitializingNewElement

but I would still need to store the state somewhere and mappers are "state-less" AFAIK.


/// <summary>Gets or sets a value used to identify a collection of semantically similar elements.</summary>
/// <value>A string that represents the collection the element belongs to.</value>
/// <remarks>Use the class id property to collect together elements into semantically similar groups for identification in UI testing and in theme engines.</remarks>
Expand Down
Expand Up @@ -49,7 +49,10 @@ public static string SetAutomationPropertiesName(this FrameworkElement Control,
else if (elemValue == false)
newValue = AccessibilityView.Raw;

Control.SetValue(NativeAutomationProperties.AccessibilityViewProperty, newValue);
if (!Element.IsPlatformViewNew || newValue != AccessibilityView.Content)
{
Control.SetValue(NativeAutomationProperties.AccessibilityViewProperty, newValue);
}

return _defaultAutomationPropertiesAccessibilityView;

Expand Down
1 change: 1 addition & 0 deletions src/Controls/src/Core/Toolbar/Toolbar.cs
Expand Up @@ -32,6 +32,7 @@ public Toolbar(Maui.IElement parent)
_parent = parent;
}

public bool IsPlatformViewNew { get; set; }
public IEnumerable<ToolbarItem> ToolbarItems { get => _toolbarItems; set => SetProperty(ref _toolbarItems, value); }
public double? BarHeight { get => _barHeight; set => SetProperty(ref _barHeight, value); }
public string BackButtonTitle { get => _backButtonTitle; set => SetProperty(ref _backButtonTitle, value); }
Expand Down
Expand Up @@ -5,6 +5,7 @@ namespace Microsoft.Maui.Controls.Core.UnitTests
{
class ApplicationStub : IApplication
{
bool IElement.IsPlatformViewNew { get; set; }
readonly List<IWindow> _windows = new List<IWindow>();

public IElementHandler Handler { get; set; }
Expand Down
Expand Up @@ -7,6 +7,7 @@ namespace Microsoft.Maui.DeviceTests.Stubs
{
class MauiAppNewWindowStub : IApplication
{
bool IElement.IsPlatformViewNew { get; set; }
readonly IWindow _window;
Window Window => _window as Window;

Expand Down
4 changes: 2 additions & 2 deletions src/Core/src/Core.csproj
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netstandard2.1;netstandard2.0;$(_MauiDotNetTfm);$(MauiPlatforms)</TargetFrameworks>
Expand All @@ -8,7 +8,7 @@
<IsTrimmable>false</IsTrimmable>
<MauiGenerateResourceDesigner>true</MauiGenerateResourceDesigner>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<NoWarn>$(NoWarn);CS1591;RS0041;RS0026;RS0027</NoWarn>
<NoWarn>$(NoWarn);CS1591;RS0041;RS0026;RS0016;RS0017;RS0027</NoWarn>
</PropertyGroup>

<PropertyGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Core/src/Core/IElement.cs
Expand Up @@ -11,5 +11,7 @@ public interface IElement
/// Gets the Parent of the Element.
/// </summary>
IElement? Parent { get; }

bool IsPlatformViewNew { get; set; }
}
}
15 changes: 14 additions & 1 deletion src/Core/src/Handlers/Element/ElementHandler.cs
Expand Up @@ -47,10 +47,21 @@ public virtual void SetVirtualView(IElement view)
bool setupPlatformView = oldVirtualView == null;

VirtualView = view;
PlatformView ??= CreatePlatformElement();

bool isNew = false;

if (PlatformView is null)
{
PlatformView = CreatePlatformElement();
isNew = true;
}

VirtualView.IsPlatformViewNew = isNew;

if (VirtualView.Handler != this)
{
VirtualView.Handler = this;
}

// We set the previous virtual view to null after setting it on the incoming virtual view.
// This makes it easier for the incoming virtual view to have influence
Expand All @@ -77,6 +88,8 @@ public virtual void SetVirtualView(IElement view)
}

_mapper.UpdateProperties(this, VirtualView);

VirtualView.IsPlatformViewNew = false;
}

public virtual void UpdateValue(string property)
Expand Down
7 changes: 6 additions & 1 deletion src/Core/src/Handlers/View/ViewHandler.cs
Expand Up @@ -517,7 +517,12 @@ public static void MapToolTip(IViewHandler handler, IView view)
{
#if PLATFORM
if (view is IToolTipElement tooltipContainer)
handler.ToPlatform().UpdateToolTip(tooltipContainer.ToolTip);
{
if (!view.IsPlatformViewNew || tooltipContainer.ToolTip is not null)
{
handler.ToPlatform().UpdateToolTip(tooltipContainer.ToolTip);
}
}
#endif
}
}
Expand Down