xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.63k stars 1.87k forks source link

[Bug] Xamarin.Forms 5.0.0.x ObservableCollection.Add performance decrease on iOS #13429

Open neckerblue opened 3 years ago

neckerblue commented 3 years ago

Description

Using Mobile App (Xamarin.Forms) template, all defaults, nothing touched

[iOS] ObservableCollection.Add() severe performance hit after updating from Xamarin.Forms 4.8.0.1451 to Xamarin.Forms 5.0.0.1874 & Xamarin.Essentials 1.5.3.2 to Xamarin.Essentials 1.6.0

[Android] No change in performance.

Steps to Reproduce

  1. Create New Project (VS2019 16.8.4 stable, .Net Framework 4.8.04084)
  2. Choose "Mobile App (Xamarin.Forms)"
  3. Update Xamarin.Forms, Xamarin.Essentials to the newest (5.0.0.1874 & 1.6.0 respectively)
  4. Go to Models -> MockDataStore.cs,
  5. Add some Items: new Item { Id = Guid.NewGuid().ToString(), Text = "anything", Description = "something" },
  6. I added 884 Items
  7. Deploy App on iPhone, I used iPhone 6s (version: 14.3 (18C66)) via USB & HotRestart
  8. Open hamburger menu, top left, navigate to "Browse"
  9. Pull down to on the list to reload RefreshView
  10. Watch it struggle (~1min)

Expected Behavior

Pre-Xamarin.Forms 5.0.0.x nuget update a reload of 884 Items of ObservableCollection was sub-second.

Actual Behavior

Reload of 884 Items of ObservableCollection takes over a minute, have to keep the device awake or it crashes.

Basic Information

Environment

Show/Hide Visual Studio info ``` Microsoft Visual Studio Community 2019 Version 16.8.4 VisualStudio.16.Release/16.8.4+30907.101 Microsoft .NET Framework Version 4.8.04084 Installed Version: Community ASP.NET and Web Tools 2019 16.8.557.25636 ASP.NET and Web Tools 2019 ASP.NET Core Razor Language Services 16.1.0.2052803+84e121f1403378489b842e1797df2f3f5a49ac3c Provides languages services for ASP.NET Core Razor. ASP.NET Web Frameworks and Tools 2019 16.8.557.25636 For additional information, visit https://www.asp.net/ Azure App Service Tools v3.0.0 16.8.557.25636 Azure App Service Tools v3.0.0 Azure Functions and Web Jobs Tools 16.8.557.25636 Azure Functions and Web Jobs Tools C# Tools 3.8.0-5.20604.10+9ed4b774d20940880de8df1ca8b07508aa01c8cd C# components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used. Common Azure Tools 1.10 Provides common services for use by Azure Mobile Services and Microsoft Azure Tools. Extensibility Message Bus 1.2.6 (master@34d6af2) Provides common messaging-based MEF services for loosely coupled Visual Studio extension components communication and integration. IntelliCode Extension 1.0 IntelliCode Visual Studio Extension Detailed Info Microsoft Azure Tools 2.9 Microsoft Azure Tools for Microsoft Visual Studio 2019 - v2.9.30924.1 Microsoft Continuous Delivery Tools for Visual Studio 0.4 Simplifying the configuration of Azure DevOps pipelines from within the Visual Studio IDE. Microsoft JVM Debugger 1.0 Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines Microsoft Library Manager 2.1.113+g422d40002e.RR Install client-side libraries easily to any web project Microsoft MI-Based Debugger 1.0 Provides support for connecting Visual Studio to MI compatible debuggers Microsoft Visual Studio Tools for Containers 1.1 Develop, run, validate your ASP.NET Core applications in the target environment. F5 your application directly into a container with debugging, or CTRL + F5 to edit & refresh your app without having to rebuild the container. Mono Debugging for Visual Studio 16.8.43 (00471f8) Support for debugging Mono processes with Visual Studio. NuGet Package Manager 5.8.1 NuGet Package Manager in Visual Studio. For more information about NuGet, visit https://docs.nuget.org/ ProjectServicesPackage Extension 1.0 ProjectServicesPackage Visual Studio Extension Detailed Info SQL Server Data Tools 16.0.62012.31170 Microsoft SQL Server Data Tools TypeScript Tools 16.0.21016.2001 TypeScript Tools for Microsoft Visual Studio Visual Basic Tools 3.8.0-5.20604.10+9ed4b774d20940880de8df1ca8b07508aa01c8cd Visual Basic components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used. Visual F# Tools 16.8.0-beta.20507.4+da6be68280c89131cdba2045525b80890401defd Microsoft Visual F# Tools Visual Studio Code Debug Adapter Host Package 1.0 Interop layer for hosting Visual Studio Code debug adapters in Visual Studio Visual Studio Container Tools Extensions 1.0 View, manage, and diagnose containers within Visual Studio. Visual Studio Tools for Containers 1.0 Visual Studio Tools for Containers VisualStudio.DeviceLog 1.0 Information about my package VisualStudio.Foo 1.0 Information about my package VisualStudio.Mac 1.0 Mac Extension for Visual Studio Xamarin 16.8.000.261 (d16-8@bb99248) Visual Studio extension to enable development for Xamarin.iOS and Xamarin.Android. Xamarin Designer 16.8.0.507 (remotes/origin/d16-8@e87b24884) Visual Studio extension to enable Xamarin Designer tools in Visual Studio. Xamarin Templates 16.8.112 (86385a3) Templates for building iOS, Android, and Windows apps with Xamarin and Xamarin.Forms. Xamarin.Android SDK 11.1.0.26 (d16-8/a36ce73) Xamarin.Android Reference Assemblies and MSBuild support. Mono: 5e9cb6d Java.Interop: xamarin/java.interop/d16-8@79d9533 ProGuard: Guardsquare/proguard/proguard6.2.2@ebe9000 SQLite: xamarin/sqlite/3.32.1@1a3276b Xamarin.Android Tools: xamarin/xamarin-android-tools/d16-8@2fb1cbc Xamarin.iOS and Xamarin.Mac SDK 14.8.0.3 (c51fabee8) Xamarin.iOS and Xamarin.Mac Reference Assemblies and MSBuild support. ```

Build Logs

none

Screenshots

none

Reproduction Link

none

Workaround

It seems Notifications are related, using custom class inheriting from ObservableCollection but disabling notifications fixes the extreme refresh times (AddNoNotification(T item)):

public class RangeObservableCollection<T> : ObservableCollection<T>
    {
        private bool _suppressNotification = false;
        protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
        {
            if (!_suppressNotification)
                base.OnCollectionChanged(e);
        }
        public void AddRange(IEnumerable<T> list)
        {
            if (list == null)
                throw new ArgumentNullException("list given to RangeObservableCollection was null");
            _suppressNotification = true;
            foreach (T item in list)
            {
                Add(item);
            }
            _suppressNotification = false;
            OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
        }
        public void AddNoNotification(T item)
        {
            if (item == null)
                throw new ArgumentNullException("item was null");

            _suppressNotification = true;

             Add(item);

            _suppressNotification = false;
            OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
        }
    }
PureWeen commented 3 years ago

@neckerblue can you attach a repro?

Are you seeing the performance difference on the same version of VS? If you down grade to 4.8 vs being on 5.0?

neckerblue commented 3 years ago

@PureWeen It's all showing up right from the templates, were my instructions incorrect or ...?

Are you seeing the performance difference on the same version of VS? If you down grade to 4.8 vs being on 5.0?

Same VS version for both. If 4.8 performance = ok, if 5.0 performance = dead. Downgrading from 5.0.x -> 4.8 fixes ObservableCollection's performance problems.

I am unsure what repro referrs to, so I made 2 repositories:

rmarinho commented 3 years ago

I changed a little bit the sample to include this code bellow so the text of the cells is a little diff every time we refresh , we can see old versions on 4.8 have some issues rendering correction

  int count = 1;

        async Task ExecuteLoadItemsCommand()
        {
            IsBusy = true;

            try
            {
                count++;
                Items.Clear();
                var items = await DataStore.GetItemsAsync(true);
                foreach (var item in items)
                {
                    item.Description = $"{item.Description} + {count}";
                    Items.Add(item);
                }
            }
            catch (Exception ex)
            {
                Debug.WriteLine(ex);
            }
            finally
            {
                IsBusy = false;
            }
        }

Simulator Screen Shot - iPod touch (7th generation) - 2021-01-27 at 16 39 16

ChristopherStephan commented 3 years ago

I am in doubt of the last known good version. I think it is 4.8.0.1687.

anton-yashin commented 3 years ago

If i change the sample to remove Items.Clear() then i get same performance issue on 4.8.1 and previous versions. Same issue if i want to remove or replace items.

        async Task ExecuteLoadItemsCommand()
        {
            IsBusy = true;

            try
            {
                var items = await DataStore.GetItemsAsync(true);
                foreach (var item in items)
                {
                    Items.Add(item);
                }
            }
            catch (Exception ex)
            {
                Debug.WriteLine(ex);
            }
            finally
            {
                IsBusy = false;
            }
        }

Workaround: use a custom ObservableCollection that send Reset message on any action.

jfversluis commented 2 years ago

Hey everyone! I think we merged something in the meantime that would make the performance better. Would anyone still be willing and able to test it out with the latest release (5.0.0.224) and let me know if this is still an issue? :)

anton-yashin commented 2 years ago

CollectionView still have poor performance when adding, removing or replacing items via ObservableCollection.

jfversluis commented 2 years ago

Thanks for letting me know! Just to confirm, this is with version 2244?

anton-yashin commented 2 years ago

Yes 5.0.0.2244+819-sha.02c4532c8-azdo.5446418. Same issue on MAUI.

anton-yashin commented 2 years ago

UI test for XF Control Issues: https://gist.github.com/anton-yashin/e5b70a2b4ce3c6f39e51f29ee1f07eb3

There too many requests to offscreen elements at each addition. For instance: if elements [0..7] is visible then it will request elements with number 10, number 9 and number 11 at each addition to set up a binding.

anton-yashin commented 2 years ago

There something strange at ItemsViewDelegator<TItemsView, TViewController>.GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout, NSIndexPath indexPath). It's calls lot a reflection stuff. When i comment it out then addition works a lot faster. It's marshaling issue maybe? Here apple instruments trace: 13429.trace.zip

anton-yashin commented 2 years ago

https://github.com/xamarin/xamarin-macios/issues/4923 may be related to this