Bug 52487 - ListView with Recycle + HasUnevenRows generates lots (and lots!) of content view instances and leaks memory
Summary: ListView with Recycle + HasUnevenRows generates lots (and lots!) of content v...
Status: VERIFIED FIXED
Alias: None
Product: Forms
Classification: Xamarin
Component: iOS ()
Version: 2.3.3
Hardware: PC Windows
: Highest major
Target Milestone: 15.5
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-02-13 22:56 UTC by Johan
Modified: 2017-11-08 14:54 UTC (History)
13 users (show)

Tags: ac, ios, performance, memory, listview, viewcell, unevenrows fr
Is this bug a regression?: ---
Last known good build:


Attachments
Sample solution (with a demo menu and all!) that reproduces the bugs (161.91 KB, application/x-zip-compressed)
2017-02-13 23:01 UTC, Johan
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
VERIFIED FIXED

Description Johan 2017-02-13 22:56:53 UTC
When creating a ListView that uses CachingStrategy="RecycleElement" and HasUnevenRows="True" the control will always create new instances of the content views used for rendering the rows when scrolling.

The content view instances used in the list are recycled properly (inside the listview) but for some reason lots of lots of unused instances of the content views are also generated when scrolling up and down (my guess is they are used for off-screen rendering for height measuring reasons or similair?).

This defeats the purpose of recycling elements and creates a lot of overhead with possible stuttering and memory problems. New instances will never stop generating and will eventually reach hundreds or thousands of items. A memory leak also seams to exist as these instances will never be allocated by the garbage collector when closing the page and doing a GC.Collect().

I've attached a sample solution to this bug report. In this sample you can test using a listview with recycle, with no recycle and recycle + hasunevenrows set to true. When scrolling in the list you can see how items are recycled properly BUT you can also see in the console that a lot of unused content views are generated when scrolling.

You can also try going back to the start page and pressing the garbage collect button. You will see that it only works for the regular non recycled elements (retain) and never collects any items when the recycle option is used (regardless of hasuneven rows). This indicates a memory leak for all cases where we recycle elements. This also has the side effect that the page the listview lives in does not collect.

We can see if an item is collected by a console message being printed when it is deconstructed. Maybe the Xamarin team can create a separate memory bug if nescessary.

Thank you for looking into this!
Comment 1 Johan 2017-02-13 23:01:03 UTC
Created attachment 19807 [details]
Sample solution (with a demo menu and all!) that reproduces the bugs
Comment 2 Johan 2017-02-15 21:51:38 UTC
I wanted to add that I found the code that causes the generation of cells (see commented row). In file ListViewRenderer.cs about line 700 something (UnevenListViewDataSource):

 public override nfloat GetHeightForRow(UITableView tableView, NSIndexPath indexPath)
            {

                // *** This row cases a cell to be instanced! ***
                var cell = GetCellForPath(indexPath);

                if (List.RowHeight == -1 && cell.Height == -1 && cell is ViewCell)
                {
                    return UITableView.AutomaticDimension;
                }

                var renderHeight = cell.RenderHeight;
                return renderHeight > 0 ? (nfloat)renderHeight : DefaultRowHeight;

            }
Comment 3 adrianknight89 2017-02-17 23:49:14 UTC
I have a PR for this already: https://github.com/xamarin/Xamarin.Forms/pull/482
Comment 4 Jimmy [MSFT] 2017-03-08 17:51:02 UTC
Since I was able to reproduce this issue with the attached project I am confirming this report so the team can investigate further.


### Repro steps
1. Run the attached project
2. Click "Recycle Uneven Rows Test"
3. Scroll through the list


### Expected Results
New cells on the screen will be re-used instead of created. 
You can see this behavior with the "Recycle Test" and "No Recycle Test" pages.


### Actual Results
When new cells rendered on screen, the debug output indicates that new cell instances are being created though they do not appear on the screen because the previous cells are still being re-used.


### Version Tests
- Forms 2.3.3.180       BAD
- Forms 2.3.4-pre2      BAD
Comment 5 Jimmy [MSFT] 2017-06-01 20:29:54 UTC
This appears to still be an issue in recent versions of Xamarin.Forms. Using the repro steps from comment 4, the debug output still indicates that new cells are being created when using RecycleElement combined with HasUnevenRows = true.

- 2.3.6.105   BAD
- 2.3.5-pre3  BAD
Comment 6 Frank Schwieterman 2017-07-18 16:41:52 UTC
We are seeing this issue on iOS and Android.  I put together a repro project:

https://github.com/trainerroad/ListviewBugRepro

It counts the instances of views and models that are in memory at any time.  The list has 500 entries.  On Android, if I scroll down through the list and back the memory reference tracing shows there are 500 models but only 21 instances of the view classes are needed:

Memory: Total 542|500 ListItemModels|21 InstrumentedGrids|21 InstrumentedViewCells


However on iOS as I scroll through the list the number of instances only go up.  Eventually the view has been instantiated more times than there are actually models to show views for.  This is a memory leak:

Memory: Total 2280|890 InstrumentedGrids|890 InstrumentedViewCells|500 ListItemModels


I repro'd this on Xamarin.Forms.2.3.5.256-pre6.
Comment 7 Frank Schwieterman 2017-07-18 16:47:32 UTC
Correction to my comment above: We are seeing this issue on iOS but NOT Android
Comment 8 Frank Schwieterman 2017-07-20 20:45:04 UTC
Just in case the master branch changes in the future, the repro at is in branch bugzilla-52487 at https://github.com/trainerroad/ListviewBugRepro.
Comment 9 Chris King 2017-08-04 03:33:20 UTC
Making progress... https://github.com/xamarin/Xamarin.Forms/pull/1085. Team lead is on vaca for a few days. Gonna need his sign-off for this change.
Comment 10 Samantha Houts [MSFT] 2017-09-15 20:11:52 UTC
https://github.com/xamarin/Xamarin.Forms/pull/1143
Comment 11 Rui Marinho 2017-09-16 14:16:16 UTC
Should be fixed on 2.4.0-pre3
Comment 12 Umesh Kamble 2017-11-08 14:54:56 UTC
ListView with Recycle + HasUnevenRows generates lots (and lots!) of content view instances and leaks memory is not reproduced on latest sample hence mark as verified.    

Test Build Info:
Microsoft Visual Studio Enterprise 2017 d15rel
Version 15.5.0 Preview 4.0 [27107.1.d15rel]
VisualStudio.15.int.d15rel/15.5.0-pre.4.0+27107.1.d15rel
Microsoft .NET Framework
Version 4.7.02046
Installed Version: Enterprise
Xamarin   4.8.0.730 (ac2f35928)
Xamarin Designer   4.8.143 (14e3edba0)
Xamarin.Android SDK   8.1.0.22 (HEAD/0baf02a75)
Xamarin.iOS and Xamarin.Mac SDK   11.4.0.208 (f288a51)



Detailed info :
https://gist.github.com/umesh-kamble/e7e97473cdafaf9db2c29ac35c85526f

Screencast link :
https://www.screencast.com/t/p9FYPIgL

As issue is not reproducible marking as "verified".