[Xamarin.iOS] UICollectionViewDelegate -> GetSizeForItem() will freeze app

[projectgoav]

Steps to Reproduce

We have a music application that allows users to browse their own collection or the contents from a number of streaming services. On many screens we display a grid of albums or playlists with an alphamap down the right-hand side. Some of these may have 10s or even 100s of thousands of items.

We virtualise the data we get from external services. We tell the collection view the total number of items in the collection but only load into memory the actual content as the user scrolls (about every 100-200 items). We only reload the cells whose data we've fetched.

Recently we wanted to add the ability for the user to adjust the size of the cells in the grid. To do this we used the GetSizeForItem() method as part of UICollectionViewDelegateFlowLayout. This is the recommended way in Xamarin and Apple docs.

What we've found is on the larger collection sizes the application grinds to a halt, both when first displayed and then when scrolling. GetSizeForItem() is called for every item when the layout has updated. A quick profile in Instruments suggests that most of the time is the internals of mono/Xamarin trying to figure out, via reflection(?), if the method has been implemented(?). I believe this is what's causing the application to freeze.

If this is indeed the case, I would have maybe expected Xamarin to check when the delegate is first assigned and keep a reference to the method or something similiar after the first call to it.

I've created a small sample app with the following steps:

1. Create a plain Xamarin.iOS application with a single collection view
2. Override GetSizeForItem() in UICollectionViewDelegateFlowLayout and set as delegate for collection view
3. Attach a UICollectionViewSource with an item count of:
   • 1000 -> No noticable issues apart from slight flicker when we reload changed rows (expected)
   • 10,000 -> Noticable pause while scrolling when we reload changed rows
   • 100,000 -> Long pause (multiple seconds) before cells are first displayed and app will freeze for multiple seconds when we reload changed rows.
4. Run the app and scroll

Expected Behavior

Application will scroll without freezing the entire app with large number of cells in a collection.

Actual Behavior

Anything over 1000 items will start to impact the performance when scrolling. At around 10,000 items the app will freeze for multiple seconds everytime you alter a cell or group of cells.

I've seen our app running on a device freeze for 20seconds or more.

Environment

=== Visual Studio Community 2017 for Mac ===

Version 7.6.8 (build 38)
Installation UUID: 15d318c1-0022-470b-8742-ac625e930509
Runtime:
	Mono 5.12.0.309 (2018-02/39d89a335c8) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)
	Xamarin.Mac 4.4.1.178 (master / eeaeb7e6)

	Package version: 512000309

=== NuGet ===

Version: 4.3.1.4445

=== .NET Core ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	2.1.2
	2.1.1
	2.0.5
	2.0.0
SDK: /usr/local/share/dotnet/sdk/2.1.302/Sdks
SDK Versions:
	2.1.302
	2.1.301
	2.1.4
	2.0.0
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/5.12.0/lib/mono/msbuild/15.0/bin/Sdks

=== Xamarin.Profiler ===

Version: 1.6.3
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Apple Developer Tools ===

Xcode 10.0 (14320.25)
Build 10A255

=== Xamarin.Mac ===

Version: 5.0.0.0 (Visual Studio Community)
Hash: b40230c0
Branch: 
Build date: 2018-09-27 11:41:37-0400

=== Xamarin.iOS ===

Version: 12.0.0.15 (Visual Studio Community)
Hash: 84552a46
Branch: xcode10
Build date: 2018-09-17 21:54:33-0400

=== Xamarin.Android ===

Not Installed

=== Xamarin Inspector ===

Version: 1.4.3
Hash: db27525
Branch: 1.4-release
Build date: Mon, 09 Jul 2018 21:20:18 GMT
Client compatibility: 1

=== Build Information ===

Release ID: 706080038
Git revision: f4178f550a4de7c03aa23678041abe4fc388cf72
Build date: 2018-09-28 14:21:41+00
Build branch: release-7.6
Xamarin extensions: 051b653186a95ced1c127dfd8c358df1b9315d0d

=== Operating System ===

Mac OS X 10.14.0
Darwin 18.0.0 Darwin Kernel Version 18.0.0
    Wed Aug 22 20:13:40 PDT 2018
    root:xnu-4903.201.2~1/RELEASE_X86_64 x86_64

Build Logs

Example Project (If Possible)

I've attached a small sample app with a simulated virtualised collection.
GSFIPerformance_Minimal.zip

[VincentDondain]

Wow thanks for the test case this is great I could reproduce the issue right away with this environment: https://gist.github.com/VincentDondain/56e267066c8896b310a683219e292814

It's very precise kItemCount = 100; no performance impact, kItemCount = 10000; slightly choppy scroll performance, kItemCount = 100000; long pauses.

Will report back after more investigation.

[spouliot]

There should not be reflection in the hot path. @VincentDondain will profile it to double check this.

The sample is a bit hard to measure. Try changing ViewDidLoad for

        public override void ViewDidLoad()
        {
			base.ViewDidLoad();
			// Perform any additional setup after loading the view, typically from a nib.

            var source = new Source(CollectionView);
            CollectionView.Source = source;
            CollectionView.Delegate = new Delegate();

			DateTime start = DateTime.UtcNow;
			CollectionView.ReloadData ();
			CollectionView.PerformBatchUpdates (() => {}, (bool finished) => {
				if (finished)
					Console.WriteLine ((DateTime.UtcNow - start).TotalSeconds);
			});
        }

when comparing times.

[VincentDondain]

@projectgoav did you get something similar to:

when profiling?

Can you point me to what made you think the issue was due to reflexion?

[VincentDondain]

Mmh your performance issues seem to have as much or more to do with: dataRefreshTask in GetCell than: GetSizeForItem. Commenting GetSizeForItem doesn't remove the performance issues as it says in the comment.

Commenting the random delay when getting the cells does fix the performance issues though.

[VincentDondain]

Here is a more accurate time profiling view.

Now the questions about what you think are our reflexion issues remain but I can give you a clearer overview of what's happening with GetSizeForItem and how to make it faster.

However please note that no matter how fast we make GetSizeForItem it won't make a big difference in this test case as there are other performance issues. GetSizeForItem "only" has a 25% weight.

The GetSizeForItem override triggers a situation where native code has to call into managed code. To deal with that we have a native_to_managed_trampoline which is called proportionally to the number of cells you need.

While this operation is "relatively fast", it adds up quickly on a very large number of calls (huge number of cells needing to be refreshed).

In particular we, in some cases, need reflexion to convert arguments from native to managed and that's a variable cost we can address.

One trick you can use is the following:

[Export ("collectionView:layout:sizeForItemAtIndexPath:")]
CGSize FastGetSizeForItem (IntPtr collectionView, IntPtr layout, IntPtr indexPath)
{
	callCount++;

	// Uncomment line below to track call count (WILL IMPACT PERFORMANCE EVEN MORE)
	//Console.WriteLine("GetSizeForItem() called: {0}time(s)", callCount);

	return new CGSize (150, 150);
}

Here because we're using arguments with IntPtr (no need to change the return type as CGSize has the same representation in native/managed) we cut some of the time spent in the trampoline and have a performance gain.

Here's the same profiling view with the hack in place:

15.12s without the hack VS 184ms with the hack (:

Let me know if this helps and answers your specific GetSizeForItem performance question.

[projectgoav]

Edit: FastGetSizeForItem was never called in my first set of runs. I've updated this comment to reflect that.

Thanks for looking into this!

Firstly, I must appologise about the reflection comment. Looking back I realised I was profiling our own application and not my test app... Ooops!

@projectgoav did you get something similar to:

I re-ran my test app with instruments and I got a different stack-trace. I couldn't find anything related to 'GetSizeForItem()' but it was obvious that something was taking a long time. Due to this I then ran my test app in the following conditions with instruments to see the effects. I've attached the file incase it's any use to you?

GetSizeForItem2.trace.zip

In all cases it was run on my Mac via an iPad Pro (9.7 inch) simulator running iOS 10.3.1 in Debug configuration. For the first 3 runs I disabled that dataRefreshTask. The second 3, I re-enabled it and gave the app a small scroll. kItemCount was 100,000

Run 1

• GetSizeForItem implemented using override in delegate.

It took about 5-6seconds of 100% CPU usage before anything was to show in the collection and the app become responsive. As expected the scroll performance was near perfect as nothing else was happening after the initial load.

Run 2

• No GetSizeForItem (Delegate not attached)

It took around 1second for data to appear and the app to become responsive. There is a really noticable difference in the CPU usage from the graph in instruments.

Run 3

• FastGetSizeForItem

Takes around 2seconds for the data to appear and the app to become responsive.

Run 4

• GetSizeForItem + dataRefreshTask

Again, a large amount of CPU usuage for 4-5seconds during initial load and again when the dataRefreshTask ReloadItems() kicks in.

Run 5

• NoGetSizeForItem + dataRefreshTask

Largely similiar to Run 2

Run 6

• FastGetSizeForItem + dataRefreshTask

Largely similiar to run 3 with a much shorter freeze when ReloadItems() from the dataRefreshTask kicks in.

It certainly looks like it was this the 'trampolining' that was causing the slowdown.

[projectgoav]

I had to update my previous comment as FastGetSizeForItem wasn't actually being called.

While freezing is less than ideal I can appreciate that calling native to/from Xamarin does incurr a cost and when multiplied by 100s of thousands of times it certainly does add up. However I don't believe I'm seeing the same about of speedup that you have seen with the FastGetSizeForItem

[rolfbjarne]

The call to Class.LookupClass is slower than it need to be, so I've filed #4936 to see if we can fix that.

[spouliot]

However I don't believe I'm seeing the same about of speedup that you have seen with the FastGetSizeForItem

@projectgoav you should run/profile on device using release builds for performance profiling.

The code generated for simulator + JIT + debug is, for various reasons (different architecture, less optimization by the JIT, less optimization from mtouch and the extra overhead for soft debugging), making the numbers less useful, i.e. you could end up trying to optimize the wrong hot path.

[rolfbjarne]

I had a look at your test project, and I have a few comments:

• The reason for the freezes is that that's when the GC runs and stops the app. This is easy to see if you add --setenv:MONO_LOG_LEVEL=debug --setenv:MONO_LOG_MASK=gc to the additional mtouch arguments in the project's iOS Build options: you'll see debug output from the GC at the same time as the app pauses.

• The reason GC runs is that the app creates many managed objects. There are two main solutions for the freezes that occurs when the GC runs:

  1. Allocate fewer managed objects.
  2. Run the GC manually (by executing GC.Collect ();) when the user isn't doing anything (such as when not scrolling in this case). This doesn't prevent the freezes, it just makes them much less noticeable. If this is feasible or not depends on the app, so I'll focus on the first solution here.

• Every time native code calls managed code, Xamarin.iOS may have to allocate managed objects for input parameters. For the GetSizeForItem method three managed objects are created for every call (since there are three parameters). If the method implementation doesn't need some or all of these managed objects, it's possible to change the signature so that Xamarin.iOS just passes the pointer to the native object. This is done by:

  1. Removing the override keyword.
  2. Adding the [Export ("...")] attribute with the corresponding selector.
  3. Changing the signature as desired (use IntPtr instead of the actual class).

  This is an example of how the GetSizeForItem signature would look if none of the three parameters are used:

  [Export ("collectionView:layout:sizeForItemAtIndexPath:")]
  public CGSize GetSizeForItem (IntPtr collectionView, IntPtr layout, IntPtr indexPath)

• This change already has a major impact on the test project, but there are still pauses, so I applied the same change to the GetCell method as well. Here things became more complicated, since some of the parameters were used in the method, so I had to apply a few other changes:

  1. If the managed object is not always used, then only create it when needed. An example of this is here: https://gist.github.com/rolfbjarne/84ee9aac2b4f6a9fdd2ad7007164b5f8#file-foo-diff-L31-L38, where I only create the indexPath instance when it's needed (when dataRefreshTask is null).
  2. LINQ creates many objects, so avoid LINQ: https://gist.github.com/rolfbjarne/84ee9aac2b4f6a9fdd2ad7007164b5f8#file-foo-diff-L47-L52
  3. Dispose any temporary NSIndexPath instances: https://gist.github.com/rolfbjarne/84ee9aac2b4f6a9fdd2ad7007164b5f8#file-foo-diff-L61-L62. This makes the GC able to collect the memory for those objects much faster, since the GC doesn't have to run the finalizer for them (the memory can be reclaimed directly).
  4. Don't create an instance for collectionView, instead call the native API directly using P/Invokes: https://gist.github.com/rolfbjarne/84ee9aac2b4f6a9fdd2ad7007164b5f8#file-foo-diff-L68-L69

  With these changes the GC still runs occasionally in the test project, but most of the time the scrolling is not affected.

Updated ViewController.cs with all my changes

Could you have a look and see if this is a viable solution for your app as well?

[projectgoav]

Thanks looking into this and your detailed response!

I looked at your changes and tried to port them into our original application. I then deployed them to an iPad Air running iOS 12.1 in Debug mode, to allow me to see some timing information in the console.

I'm running VS Mac 7.6.11 (build 9) with Xamarin iOS 12.1.0.15 and XCode 10.1

Changing the NumberOfSections and ItemsInSection was relatively simple. On it's own it didn't account for much of a speedup. Fetching a cell wasn't any faster (as expected) and the total time taken to reload a number of rows was maybe 0.5-1ms faster.

Converting to use an exported GetCell method, like in your example above, was a little more difficult. We use the constructed cell instances to do some configuration. One of the libraries we use provides some helpers for the UICollectionViewSource which I worried would interfere with my tests. I managed to strip away as much of that as possible and replicate the helpers we used within tests below.

I suspected that since we use the cell instances resolving them from their handle would negate any speedup we might see. By results below largely follow this pattern.

Method Type	ms / GetCell	ms / ReloadItems
Override	1.5 - 2	175 - 200
Export / Handles	1 - 1.5	175

I then removed a similiar LINQ query to the one you commented on in my example. Looping the indexes and manually constructing NSIndexPaths + dispose knocked about 25ms from the overall ReloadItems time.

I finally I tried a different method to update the contents in our cells. Instead of calling ReloadItems with the rows that had changed I decided to manually call 'GetCell` for each of the cells that had changed. I would then update the resulting instance with any new data. This doesn't feel like the right thing to do here but it showed the biggest improvement in speeds:

• Overriden GetCell the total ReloadItems call took on average 12ms
• Export GetCell the total ReloadItems call took on average 9ms

If you couple these changes with the FastGetSizeForItem solution in a previous reply the stuttering I was seeing is significantly reduced. It's certainly not any worse than we'd seen sometimes with a fixed size of cell.

I'm sure once the improvements you've made since my inital report are released this may become even faster!

[VincentDondain]

Hi, glad this helped (:

Rolf made some optimizations, see: #4936 (comment) and there's a request for comment issue #5025 for even more gains 🎉

I'm closing this issue since I believe we provided as much info as we could but feel free to report any improvements here or file more performance related issues and we'll be happy to take a look.