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

[Android] CollectionVIew scrolling performance on MAUI lags compared to XF #18505

Closed
PureWeen opened this issue Nov 3, 2023 · 28 comments · Fixed by #21229
Closed

[Android] CollectionVIew scrolling performance on MAUI lags compared to XF #18505

PureWeen opened this issue Nov 3, 2023 · 28 comments · Fixed by #21229
Assignees
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-8.0.20 fixed-in-8.0.40 fixed-in-9.0.0-preview.3.10457 p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/android 🤖 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Milestone

Comments

@PureWeen
Copy link
Member

PureWeen commented Nov 3, 2023

Description

Given the following layout

<ContentPage>
     <CollectionView x:Name="cvGrid" Background="Yellow" HorizontalOptions="Fill"
                             BackgroundColor="Black">
		<CollectionView.ItemTemplate>
			<DataTemplate>
				<Grid BackgroundColor="Purple" ColumnDefinitions="25,25,25,25,25,25,25,25,25,25,25">
					<Label Grid.Column="0" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="1" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="2" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="3" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="4" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="5" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="6" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="7" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="8" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="9" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="10" TextColor="White" Text="{Binding Symbol}"></Label>
					<Label Grid.Column="11" TextColor="White" Text="{Binding Symbol}"></Label>
				</Grid>
			</DataTemplate>
		</CollectionView.ItemTemplate>
	</CollectionView>
</ContentPage>

the performance on XF is much better than MAUI

Steps to Reproduce

Run the following projects on Android and you'll notice that the scrolling on XF is super smooth compared to MAUI. If you perform really fast scrolling, XF remains smooth whereas MAUI starts to glitch and slow down.

MAUI:
MauiCollectionView.zip

XF:
XFCOllectionView.zip

I didn't test this on WinUI/iOS yet but it would be good to validate there as well

Link to public reproduction project repository

No response

Version with bug

8.0.0-rc.2.9373

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Android

Did you find any workaround?

No response

Relevant log output

No response

Related Issues

#20433
#19933
#20797

@PureWeen PureWeen added t/bug Something isn't working legacy-area-perf Startup / Runtime performance labels Nov 3, 2023
@PureWeen PureWeen added this to the .NET 8 SR2 milestone Nov 3, 2023
@PureWeen PureWeen added the partner/cat 😻 Client CAT Team label Nov 3, 2023
@Amir000001
Copy link

this problem also exists in net 7

@PureWeen
Copy link
Member Author

PureWeen commented Nov 3, 2023

Related #17326

@mattleibow
Copy link
Member

Just tested iOS, wow, much slow.

@mjsb212
Copy link

mjsb212 commented Nov 10, 2023

I'm having a similar problem with GridItemsLayout and described what I've tried/found in: #17326 -- Does anyone know if the issues are related??

@Alex-Dobrynin
Copy link

Alex-Dobrynin commented Nov 16, 2023

@PureWeen I have noticed that CreateContent() of datatemplate is much slower than in XF, may be here is the problem. because the same template in XF takes 30 ms to create and 200 ms in MAUI.
I supposed that MAUI should be much performant than XF but it is not

@mikebikerider
Copy link

I observed CollectionView sluggishness in MAUI .NET 7, and even more with .NET 8 iOS compared to Xamarin forms. Multiple issues with grids not resizing properly. Star columns and rows do not behave as expected. I moved a big Xamarin iOS project to MAUI .NET 7, I will not publish it. Will keep updating Xamarin.Forms version. With Android it is even worse. Resizing issues worse than with iOS, CollectionView scrolling slower and jerkier. My Android Xamarin.Forms apps run faster than iOS counterparts. MAU Android apps significantly slower than MAUI iOS. Waiting for .NET 8 was a big disappointment. It brought further performance degradation instead of improvement. I like MAUI coding, but MAUI GUI performance is not acceptable.

@BaY1251
Copy link

BaY1251 commented Nov 28, 2023

any plan for it?

@Redth
Copy link
Member

Redth commented Dec 5, 2023

It's worth noting that if anyone is looking at the speed of scrolling in CollectionView or ListView while debugging their app through Visual Studio, it is definitely going to be slower than on an actual release build. The reason for this is there's a lot more visual tree interaction happening with the live visual tree tooling which slows things down significantly.

Having said that, it does seem like it might be slower than it could be on Android even in a release build. I'm working on collecting some traces so we can look at the details a bit more and see where time is being spent.

@Redth
Copy link
Member

Redth commented Dec 5, 2023

Here's a speedscope trace from this app running on a Pixel 4 in release mode...
dotnet-dsrouter.exe_20231205_163422.speedscope.json

Here's a video of the app running:
https://github.com/dotnet/maui/assets/271950/f97cfdf8-631b-4a26-8386-1cc4f40c33bf

I think this is running reasonably fast looking at the video, and nothing stands out too much from the speedscope trace.

@jonathanpeppers thoughts?

@Alex-Dobrynin
Copy link

@Redth as i said before, CreateContent() method of DataTemplate is much slower in MAUI than in XF on same templates both in debug mode. in XF it takes 20 -30ms to construct template, in MAUI takes 200 ms. Maybe this is the case

@jonathanpeppers
Copy link
Member

The CreateContent() method is showing a small % in the .speedscope file above:

image

When looking for low-hanging fruit while scrolling, I tended to look for things in the 2% range. But there still could be something done here that would help.

I added this section, as this seems to be coming up a lot:

@jonathanpeppers
Copy link
Member

One thing I do see, I think we could completely eliminate this call:

image

Where it only calls into C# from Java if the TextView.Text is a "formatted string". So plain strings could avoid the interop.

@mikebikerider
Copy link

@Redth, is it worth comparing scrolling in CollectionView or ListView in release mode to scrolling of CollectoinView or ListView in Xamarin.Forms in any mode?
I moved a 5 years old Xamarin.Forms project to MAUI .NET 7. Xamarin.Forms ListView and CollectionView scrolling on a physical device (Samsung Galaxy S20) is smooth and fast even when run in Visual Studio debugger with Hot Reload enabled. The MAUI app scrolling is slower, and it is jerky when running on the same device in release mode directly. Pretty much same on iOS, only Android is worse. I think there is more than one cause for poor performance. MAUI Grid.

From Microsoft online documentation https://learn.microsoft.com/en-us/dotnet/maui/user-interface/layouts/grid?view=net-maui-8.0

Caution
Try to ensure that as few rows and columns as possible are set to Auto size. Each auto-sized row or column will cause the layout engine to perform additional layout calculations. Instead, use fixed size rows and columns if possible. Alternatively, set rows and columns to occupy a proportional amount of space with the GridUnitType.Star enumeration value.

Somehow in Xamarin.Forms using 'auto' rows and columns did not cause problems I could notice, and Xamarin.Forms Star rows and columns resize as documented. Not in MAUI .NET 7 or .NET8. Star columns and rows behave like 'auto' and that affects controls they are hosting, and controls that are hosting grid cells. ListView and CollectionView in my apps use one-row multiple columns Grid in the DataTemplate. One of the columns is always Star. Other columns width is bound to datasource (List<>). In Xamarin.Forms it works like a charm. In MAUI works too but does not resize properly when device is rotated.
The app has 7 pages that have either ListView or CollectionView. In theXamarin.Forms app none of the pages has OnSizeAllocated(). I can rotate the device and everything including CollectionView/ListView resizes correctly. In MAUI the same XAML and code behind produce wrong sizing on load and when device is rotated and not just for CollectionView. In MAUI I had to write OnSizeAllocated() for each page to make it look more or less right, like I did in early Xamarin days. I tried .NET 8 when it was released in November. It is so much worse I went back to .NET 7.
The added MAUI code is different between iOS and Android projects. Xamarin.Forms beats MAUI in the 'write code once' area too.
If I would not have the Xamarin.Forms app in respective app stores since 2018 and vastly outperforming the MAUI version I might even publish the iOS version. Android still requires some work. But it makes no sense.

@jonathanpeppers
Copy link
Member

is it worth comparing scrolling in CollectionView or ListView in release mode to scrolling of CollectoinView or ListView in Xamarin.Forms in any mode?

That's why we added the section at the top here: https://aka.ms/profile-maui#feature-xyz-was-faster-in-xamarinforms

Compare Release vs Release.

@mikebikerider
Copy link

Jonathan, in my experience the Release against Release comparison is hugely in favor of Xamarin.Forms.
Microsoft MAUI .NET 7 and .NET 8 update notes always refer to improvements (in tests) against previous MAUI versions,
The sad thing is I like MAUI coding. I don't like the result, and the ugly tweaks I do to keep layouts intact, more or less, when the screen orientation changes.

@jonathanpeppers
Copy link
Member

@mikebikerider can you share a sample app, .nettrace, .speedscope where we can take a look? (and file a new issue)

It sounds like we need to just profile your MAUI app and see what it's doing.

@mikebikerider
Copy link

Jonathan, I have to write a sample app.
If I do something terribly wrong that should affect both iOS and Android, right? Below is a flyout page with a CollectionView that my code loads from another flyout page.

Screenshot_20231207_131110

I issue a call from the previous page:
await Shell.Current.GoToAsync("//assets",false);
assetsPage.LoadAssets(listedassets, w, Width);

In MAUI iOS the page loads almost immediately. Under Android the calling page freezes for about 30 seconds, before the next page appears. Apparently, under Android await does not wait long enough for the page to load to the point it can start processing GUI code. Everything in MAUI Android seems be slower than in MAUI iOS. So, I do a trick, I call the loading code from a Timer. It kind of works, but first the user sees a mostly blank page:
Dispatcher.StartTimer(TimeSpan.FromMilliseconds(500), () =>
{
assetsPage.LoadAssets(listedassets, w, Width);
return false;
});
I don't need that trick next time I get to the page during the program run, but it is still ugly.

My Xamarin Android apps run faster than their iOS equivalents and code behind is identical except for the platform-specific calls. Not so with MAUI. MAUI iOS GUI updates are faster compared to Android, and I can't use the same common code. So far Xamarin renders do much better job.

@jonathanpeppers
Copy link
Member

Did you profile this app? File an issue with some .nettrace or .speedscope files, thanks!

@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Dec 7, 2023
@anpin
Copy link

anpin commented Mar 12, 2024

@mikebikerider the link you posted seems to be private

@mikebikerider
Copy link

I changed the repository to public. Thank you for pointing it out.

StephaneDelcroix added a commit that referenced this issue Mar 15, 2024
instead of subscribing to ThemeChanged event, use the ResourcesChanged
mechanism of DynamicResource to propagate the change

- related to #8713
- alternative to #19931
- fixes #18505
StephaneDelcroix added a commit that referenced this issue Mar 15, 2024
instead of subscribing to ThemeChanged event, use the ResourcesChanged
mechanism of DynamicResource to propagate the change

- related to #8713
- alternative to #19931
- fixes #18505
StephaneDelcroix added a commit that referenced this issue Mar 18, 2024
instead of subscribing to ThemeChanged event, use the ResourcesChanged
mechanism of DynamicResource to propagate the change

- related to #8713
- alternative to #19931
- fixes #18505
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 18, 2024
Context: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: dotnet#21229 (review)

In profiling scrolling of an app with a `<CollectionView/>` and 12
`<Label/>`s, we see time spent in:

    1.9% Microsoft.Maui!Microsoft.Maui.Platform.MauiTextView.OnLayout(bool,int,int,int,int)

This is a callback from Java to C#, which has a performance cost.
Reviewing the code, we would only need to make this callback *at all*
if `Label.TextType` is not `TextType.Text`. The bulk of all `Label`'s
can avoid this call?

To do this:

* Write a new `PlatformAppCompatTextView.java` that override `onLayout()`

* It only calls `onLayoutFormatted()` if a `isFormatted` `boolean`
  field is `true`

* We can set `isFormatted` when `Label.TextType` changes to something
  other than `TextType.Text`.

With this change in place, the above `MauiTextView.OnLayout()` call is
completely gone from `dotnet-trace` output. Scrolling the sample also
"feels" a bit snappier.

This should improve the performance of all `Label`s on Android.

This is the mininum amount of API changes possible -- which seems like
what we should go for if we ship this change in .NET 8 servicing.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 19, 2024
Context: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: dotnet#21229 (review)

In profiling scrolling of an app with a `<CollectionView/>` and 12
`<Label/>`s, we see time spent in:

    1.9% Microsoft.Maui!Microsoft.Maui.Platform.MauiTextView.OnLayout(bool,int,int,int,int)

This is a callback from Java to C#, which has a performance cost.
Reviewing the code, we would only need to make this callback *at all*
if `Label.TextType` is not `TextType.Text`. The bulk of all `Label`'s
can avoid this call?

To do this:

* Write a new `PlatformAppCompatTextView.java` that override `onLayout()`

* It only calls `onLayoutFormatted()` if a `isFormatted` `boolean`
  field is `true`

* We can set `isFormatted` when `Label.TextType` changes to something
  other than `TextType.Text`.

With this change in place, the above `MauiTextView.OnLayout()` call is
completely gone from `dotnet-trace` output. Scrolling the sample also
"feels" a bit snappier.

This should improve the performance of all `Label`s on Android.

This is the mininum amount of API changes possible -- which seems like
what we should go for if we ship this change in .NET 8 servicing.
StephaneDelcroix added a commit that referenced this issue Mar 20, 2024
* [C] use ResourcesChanged to propagate Theme

instead of subscribing to ThemeChanged event, use the ResourcesChanged
mechanism of DynamicResource to propagate the change

- related to #8713
- alternative to #19931
- fixes #18505

* fix test

* use a const string
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 20, 2024
Applies to: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip

I profiled the above sample with `dotnet-trace` with the following PRs
applied locally:

* dotnet#21229
* dotnet#21291

While scrolling, a lot of time is spent in `ResourceDictionary`
lookups on an Android Pixel 5 device:

    2.0% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&)

Drilling in, I can see System.Linq's `Reverse()` method:

    0.56% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>.MoveNext()
    0.14% System.Linq!System.Linq.Enumerable.Reverse(System.Collections.Generic.IEnumerable`1<TSource_REF>)
    0.04% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>..ctor(System.Collections.Generic.IEnumerable`1<TSource_REF>)
    0.04% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>.Dispose()

`Reverse()` can be problematic as it can sometimes create a copy of
the entire collection, in order to sort in reverse. We can juse use a
reverse `for`-loop instead.

The indexer, we can also avoid a double-lookup:

    if (dict.ContainsKey(index))
        return dict[index];

And instead do:

    if (dict.TryGetValue(index, out var value))
        return value;

The MAUI project template seems to setup a few "merged"
`ResourceDictionary` as it contains `Styles.xaml`, so this is why this
code path is being hit.

I wrote a BenchmarkDotNet benchmark, and it indicates the collection
is being copied, as the 872 bytes of allocation occur:

| Method      | key         | Mean      | Error    | StdDev   | Gen0   | Allocated |
|------------ |------------ |----------:|---------:|---------:|-------:|----------:|
| TryGetValue | key0        |  11.45 ns | 0.026 ns | 0.023 ns |      - |         - |
| Indexer     | key0        |  24.72 ns | 0.133 ns | 0.118 ns |      - |         - |
| TryGetValue | merged99,99 | 117.06 ns | 2.334 ns | 2.497 ns | 0.1042 |     872 B |
| Indexer     | merged99,99 | 145.60 ns | 2.737 ns | 2.286 ns | 0.1042 |     872 B |

With these changes in place, I see less time spent inside:

    0.91% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&)

The benchmark no longer allocates either:

| Method      | key         | Mean      | Error     | StdDev    | Allocated |
|------------ |------------ |----------:|----------:|----------:|----------:|
| TryGetValue | key0        |  11.92 ns |  0.094 ns |  0.084 ns |         - |
| Indexer     | merged99,99 |  23.12 ns |  0.418 ns |  0.391 ns |         - |
| Indexer     | key0        |  24.20 ns |  0.485 ns |  0.453 ns |         - |
| TryGetValue | merged99,99 |  29.09 ns |  0.296 ns |  0.262 ns |         - |

This should improve the performance "parenting" of any MAUI view on
all platforms -- as well as scrolling `CollectionView`.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 26, 2024
Context: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: dotnet#21229 (review)

In profiling scrolling of an app with a `<CollectionView/>` and 12
`<Label/>`s, we see time spent in:

    1.9% Microsoft.Maui!Microsoft.Maui.Platform.MauiTextView.OnLayout(bool,int,int,int,int)

This is a callback from Java to C#, which has a performance cost.
Reviewing the code, we would only need to make this callback *at all*
if `Label.FormattedText` is not `null`. The bulk of all `Label`'s can
avoid this call?

To do this:

* Write a new `PlatformAppCompatTextView.java` that override `onLayout()`

* It only calls `onLayoutFormatted()` if a `isFormatted` `boolean`
  field is `true`

* We can set `isFormatted` if a formatted string is such as:
  `isFormatted = !(text instanceof String)`

With this change in place, the above `MauiTextView.OnLayout()` call is
completely gone from `dotnet-trace` output. Scrolling the sample also
"feels" a bit snappier.

This should improve the performance of all non-formatted `Label`s on
Android.

This is the mininum amount of API changes possible -- which seems like
what we should go for if we ship this change in .NET 8 servicing.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Mar 26, 2024
Applies to: dotnet#18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip

In reviewing various issues about `CollectionView`, there is a pattern of:

* My app is slow (in `Debug` mode)

* Oh, but it seems completely OK in `Release`!

This is due to several features enabled in `Debug` mode, such as:

* On mobile, the Mono interpreter is used (enables C# hot reload)

* Xaml compilation is disabled (enables XAML hot reload)

* Other debug features are enabled (such as the `$(Optimize)` flag)

I attached `dotnet-trace` to the app above on a Pixel 5 while
debugging, just to see what I could find. I also had XAML & C# hot
reload enabled.

One thing that appears when parsing XAML is:

      2.11s microsoft.maui.controls.xaml!Microsoft.Maui.Controls.Xaml.ApplyPropertiesVisitor.GetBindableProperty(System.Type,string
    257.40ms System.Private.CoreLib!System.String.Format(string,object,object)
    172.25ms microsoft.maui.controls!Microsoft.Maui.Controls.Xaml.XamlParseException.FormatMessage(string,System.Xml.IXmlLineInfo)

So about ~2 seconds of this app's startup was in
`ApplyPropertiesVisitor.GetBindableProperty()`. Looking at what is
happening:

    if (exception == null && bindableFieldInfo == null)
    {
        exception =
            new XamlParseException(
                Format("BindableProperty {0} not found on {1}", localName + "Property", elementType.Name), lineInfo);
    }
    //...
    if (throwOnError)
        throw exception; // only use of `exception`

But then `throwOnError` is actually `false` in this case, so a
`XamlParseException` is created and not used. I could just reorder the
logic to avoid creating a `XamlParseException` and calling `Format()`.

Next, I noticed the System.Reflection usage:

    var bindableFieldInfo = elementType.GetFields(supportedFlags)
        .FirstOrDefault(fi => (fi.IsAssembly || fi.IsPublic) && fi.Name == localName + "Property");

If this were called on many bindable properties on `Label`, `Button`,
etc., this code would create lots of `FieldInfo[]` arrays and iterate
until a matching property (field) is found.

I think we can change this to avoid creating `FieldInfo[]` arrays,
such as:

    var bindableFieldInfo = elementType.GetField(localName + "Property", supportedFlags);
    if (bindableFieldInfo is not null && (bindableFieldInfo.IsAssembly || bindableFieldInfo.IsPublic))
    {
        //...

With these changes in place, it significantly improves the call:

    816.35ms microsoft.maui.controls.xaml!Microsoft.Maui.Controls.Xaml.ApplyPropertiesVisitor.GetBindableProperty(System.Type,string

Saving over about ~1.3 seconds of startup time in debug mode.

This should improve the performance of XAML parsing on all platforms,
which is most likely being used while debugging. You *can* disable
Xaml compilation in `Release` mode, but it is not recommended.
jonathanpeppers added a commit that referenced this issue Mar 27, 2024
Context: #18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip
Context: #21229 (review)

In profiling scrolling of an app with a `<CollectionView/>` and 12
`<Label/>`s, we see time spent in:

    1.9% Microsoft.Maui!Microsoft.Maui.Platform.MauiTextView.OnLayout(bool,int,int,int,int)

This is a callback from Java to C#, which has a performance cost.
Reviewing the code, we would only need to make this callback *at all*
if `Label.FormattedText` is not `null`. The bulk of all `Label`'s can
avoid this call?

To do this:

* Write a new `PlatformAppCompatTextView.java` that override `onLayout()`

* It only calls `onLayoutFormatted()` if a `isFormatted` `boolean`
  field is `true`

* We can set `isFormatted` if a formatted string is such as:
  `isFormatted = !(text instanceof String)`

With this change in place, the above `MauiTextView.OnLayout()` call is
completely gone from `dotnet-trace` output. Scrolling the sample also
"feels" a bit snappier.

This should improve the performance of all non-formatted `Label`s on
Android.

This is the mininum amount of API changes possible -- which seems like
what we should go for if we ship this change in .NET 8 servicing.
StephaneDelcroix pushed a commit that referenced this issue Mar 27, 2024
Applies to: #18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip

I profiled the above sample with `dotnet-trace` with the following PRs
applied locally:

* #21229
* #21291

While scrolling, a lot of time is spent in `ResourceDictionary`
lookups on an Android Pixel 5 device:

    2.0% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&)

Drilling in, I can see System.Linq's `Reverse()` method:

    0.56% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>.MoveNext()
    0.14% System.Linq!System.Linq.Enumerable.Reverse(System.Collections.Generic.IEnumerable`1<TSource_REF>)
    0.04% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>..ctor(System.Collections.Generic.IEnumerable`1<TSource_REF>)
    0.04% System.Linq!System.Linq.Enumerable.ReverseIterator<TSource_REF>.Dispose()

`Reverse()` can be problematic as it can sometimes create a copy of
the entire collection, in order to sort in reverse. We can juse use a
reverse `for`-loop instead.

The indexer, we can also avoid a double-lookup:

    if (dict.ContainsKey(index))
        return dict[index];

And instead do:

    if (dict.TryGetValue(index, out var value))
        return value;

The MAUI project template seems to setup a few "merged"
`ResourceDictionary` as it contains `Styles.xaml`, so this is why this
code path is being hit.

I wrote a BenchmarkDotNet benchmark, and it indicates the collection
is being copied, as the 872 bytes of allocation occur:

| Method      | key         | Mean      | Error    | StdDev   | Gen0   | Allocated |
|------------ |------------ |----------:|---------:|---------:|-------:|----------:|
| TryGetValue | key0        |  11.45 ns | 0.026 ns | 0.023 ns |      - |         - |
| Indexer     | key0        |  24.72 ns | 0.133 ns | 0.118 ns |      - |         - |
| TryGetValue | merged99,99 | 117.06 ns | 2.334 ns | 2.497 ns | 0.1042 |     872 B |
| Indexer     | merged99,99 | 145.60 ns | 2.737 ns | 2.286 ns | 0.1042 |     872 B |

With these changes in place, I see less time spent inside:

    0.91% Microsoft.Maui.Controls!Microsoft.Maui.Controls.ResourceDictionary.TryGetValue(string,object&)

The benchmark no longer allocates either:

| Method      | key         | Mean      | Error     | StdDev    | Allocated |
|------------ |------------ |----------:|----------:|----------:|----------:|
| TryGetValue | key0        |  11.92 ns |  0.094 ns |  0.084 ns |         - |
| Indexer     | merged99,99 |  23.12 ns |  0.418 ns |  0.391 ns |         - |
| Indexer     | key0        |  24.20 ns |  0.485 ns |  0.453 ns |         - |
| TryGetValue | merged99,99 |  29.09 ns |  0.296 ns |  0.262 ns |         - |

This should improve the performance "parenting" of any MAUI view on
all platforms -- as well as scrolling `CollectionView`.
StephaneDelcroix pushed a commit that referenced this issue Mar 29, 2024
* [xaml] improve performance in debug-mode

Applies to: #18505
Context: https://github.com/dotnet/maui/files/13251041/MauiCollectionView.zip

In reviewing various issues about `CollectionView`, there is a pattern of:

* My app is slow (in `Debug` mode)

* Oh, but it seems completely OK in `Release`!

This is due to several features enabled in `Debug` mode, such as:

* On mobile, the Mono interpreter is used (enables C# hot reload)

* Xaml compilation is disabled (enables XAML hot reload)

* Other debug features are enabled (such as the `$(Optimize)` flag)

I attached `dotnet-trace` to the app above on a Pixel 5 while
debugging, just to see what I could find. I also had XAML & C# hot
reload enabled.

One thing that appears when parsing XAML is:

      2.11s microsoft.maui.controls.xaml!Microsoft.Maui.Controls.Xaml.ApplyPropertiesVisitor.GetBindableProperty(System.Type,string
    257.40ms System.Private.CoreLib!System.String.Format(string,object,object)
    172.25ms microsoft.maui.controls!Microsoft.Maui.Controls.Xaml.XamlParseException.FormatMessage(string,System.Xml.IXmlLineInfo)

So about ~2 seconds of this app's startup was in
`ApplyPropertiesVisitor.GetBindableProperty()`. Looking at what is
happening:

    if (exception == null && bindableFieldInfo == null)
    {
        exception =
            new XamlParseException(
                Format("BindableProperty {0} not found on {1}", localName + "Property", elementType.Name), lineInfo);
    }
    //...
    if (throwOnError)
        throw exception; // only use of `exception`

But then `throwOnError` is actually `false` in this case, so a
`XamlParseException` is created and not used. I could just reorder the
logic to avoid creating a `XamlParseException` and calling `Format()`.

Next, I noticed the System.Reflection usage:

    var bindableFieldInfo = elementType.GetFields(supportedFlags)
        .FirstOrDefault(fi => (fi.IsAssembly || fi.IsPublic) && fi.Name == localName + "Property");

If this were called on many bindable properties on `Label`, `Button`,
etc., this code would create lots of `FieldInfo[]` arrays and iterate
until a matching property (field) is found.

I think we can change this to avoid creating `FieldInfo[]` arrays,
such as:

    var bindableFieldInfo = elementType.GetField(localName + "Property", supportedFlags);
    if (bindableFieldInfo is not null && (bindableFieldInfo.IsAssembly || bindableFieldInfo.IsPublic))
    {
        //...

With these changes in place, it significantly improves the call:

    816.35ms microsoft.maui.controls.xaml!Microsoft.Maui.Controls.Xaml.ApplyPropertiesVisitor.GetBindableProperty(System.Type,string

Saving over about ~1.3 seconds of startup time in debug mode.

This should improve the performance of XAML parsing on all platforms,
which is most likely being used while debugging. You *can* disable
Xaml compilation in `Release` mode, but it is not recommended.

* Update BuildWarningsUtilities.cs

* Remove `bool throwOnError = false` parameter
@github-actions github-actions bot locked and limited conversation to collaborators Apr 20, 2024
@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) and removed legacy-area-perf Startup / Runtime performance labels May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-8.0.20 fixed-in-8.0.40 fixed-in-9.0.0-preview.3.10457 p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/android 🤖 t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
Status: Done