[TIMOB-5429] iOS: TableViewRows are not being reused even though className is set
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | High |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2012-07-18T19:35:20.000+0000 |
Affected Version/s | Release 1.7.2 |
Fix Version/s | Sprint 2012-14 Core, Release 3.0.0 |
Components | iOS |
Labels | module_tableviewrow, qe-testadded |
Reporter | Anthony Decena |
Assignee | Max Stepanov |
Created | 2011-10-04T14:56:26.000+0000 |
Updated | 2013-10-07T09:57:05.000+0000 |
Description
While profiling our app I noticed that scrolling through the stream continues to generate new views rather than reusing existing ones. These rows do in fact have classNames set, and when the table is cleared, all the views are indeed garbage collected, so I do not believe it is an issue of external references being kept. Also of interest, the # of transitory instances also continues to rise, suggesting that the rows are being continuously garbage collected and recreated (potentially as a result of continuous setData calls).
Video of the issue: http://cl.ly/1M3L2T2y2k3k1q1y3g2Y
Factory method used to generate these rows: https://gist.github.com/7bd7d37b056562eb9962
Not that to page in new rows, they are added to the main tableViewSection, and then setData is called on the table with the entire array of sections again
To create similar results:
Note that in the app in the original video, paging is being done through setData instead of appendRow
setData must start with a fresh set of classNames and views. Also why wasn't the TC-269 just moved to this ticket? Most of the required fields don't have appropriate information in them.
re: "setData must start with a fresh set of classNames and views". Are you saying that setData can not reliably be used for paging? Appending rows one at a time is extremely slow.
Has any progress been made on this? Anything else I can provide that would help?
The biggest issue is going to be that even with reusing className, there will be major performance degradation when setting any properties which affect layout (the exact thing that className is supposed to prevent) and so we will need to filter those properties which affect layout extensively and impose strict rules on what constitutes two rows with the same className.
The following tickets must all be checked for possible regressions when resolving this issue (both for tables using className and those without, where appropriate): TIMOB-5380 TIMOB-5441 TIMOB-6134 TIMOB-3600 TIMOB-4687 TIMOB-3007 TIMOB-2225 TIMOB-1037 TIMOB-2346 TIMOB-2304 TIMOB-5279 TIMOB-6082 TIMOB-1664
This bug may need to be marked INVAID. From Apple's documentation: bq. The table view's delegate in tableView:cellForRowAtIndexPath: should always reset all content when reusing a cell. If the cell object does not have an associated reuse identifier, this method is not called. If you override this method, you must be sure to invoke the superclass implementation. This indicates that any subviews need to be re-added because they may be cleaned up by the system (they are no longer drawn, and therefore are fair game for removal). In this case there is absolutely no performance improvement from className.
The point of the reuse identifiers is precisely so that the expensive operation of creating views does not need to occur for each row. I believe the word "reset" here just means that all subviews must be returned to a blank state, for example UILabels must have their text cleared. If you take a look at how any open source iOS app is built, you will see this is the case.
There is an apparent bug in iOS 5 preventing this fix. Apparently subviews of a cell now track the row index in some manner, since hit detection on them sends the wrong row index to the table after a row has been reproxied. As a result this fix may be delayed.
Pull merged.
GetGlue : I'd love to hear results of you re-testing with a fresh CI build!
Verified fix with: Titanium Studio, build: 2.1.1.201207271312 Titanium SDK: 2.2.0.v20120816212512 Devices: iPhone 4s 5.0.1 iPad1 ios5 5.1.1 Simulator 5.1 Mac osx 10.8 Mountain Lion