Optimization breaks app #65

Closed
scompt opened this Issue Mar 30, 2012 · 8 comments

Projects

None yet

5 participants

@scompt
scompt commented Mar 30, 2012

The optimization from issue #62 is breaking my app.

My app is designed so that I load the data for the cells in a background thread and then call reloadData on the main thread to update the contents of the cells. With this optimization in place, I get a page of empty cells.

When I call reloadData, it looks like the optimization is correctly exiting loadRequiredItems early, but then loadRequiredItems is being called again from layoutSubviewsWithAnimation and not exiting early, causing the incorrect loading.

Let me know if there's any additional information I can provide.

@mwyman
mwyman commented Apr 5, 2012

The point of the optimization looks to be preventing unnecessary code running during scrolling. Rather than revert, it looks to me like the optimization code should check whether self.itemsSubviewsCacheIsValid as well. -reloadData and some of the insert/remove methods will invalidate the subview cache, whereas scrolling doesn't.

@scompt
scompt commented Apr 10, 2012

Thanks for the idea, @mwyman . Unfortunately, it looks like every time the optimization is taken, the subviews cache is valid as well.

@gmoledina
Owner

Wouldn't you want to load the data of each cell separately ? What happens when you scroll otherwise ? You fetch the new data and then reload the grid ?

If you really need to do it this way, we could have a way to force-reloadData. But it seems that each cell data should be handled separately and so as soon as the data of a specific cell is available, only that cell gets reloaded.

@scompt
scompt commented Apr 11, 2012

The API I'm using returns data in pages with 24 items per page. In my
app, I display 12 cells per page. The items are basically image and
description.

When my grid delegate fetches a cell that hasn't yet been downloaded,
it triggers an async load of the data and returns an empty cell. I
have some synchronization which prevents this from happening multiple
times per page of data. When the API call returns, I call reloadData
so that all of the cells are created again.

I suppose the alternative you're suggesting is to go through and see
which of the new items have cells that are currently displayed and
reload those. Because that will be at least 1 page of cells and
potentially more depending on how fast the user scrolls, I've chosen
to reload the entire dataset.

Based on this use case, do you still recommend your suggestion or is
the "reload everything" approach worth supporting?

On Wed, Apr 11, 2012 at 4:39 AM, Gulam Moledina
reply@reply.github.com
wrote:

Wouldn't you want to load the data of each cell separately ? What happens when you scroll otherwise ? You fetch the new data and then reload the grid ?

If you really need to do it this way, we could have a way to force-reloadData. But it seems that each cell data should be handled separately and so as soon as the data of a specific cell is available, only that cell gets reloaded.


Reply to this email directly or view it on GitHub:
#65 (comment)

@mwyman
mwyman commented Apr 11, 2012

My app does a similar thing. The way I handle the async loading is to have the cell views associated with the asynchronous load operation, and the operation completes it sets whatever properties are necessary on the cell view, then calls -setNeedsDisplay on it. This way the views update, but don't require recalculating the grid layout.

If you have the cell views reference the asynchronous operation, you can also cancel the operation when the views get recycled in -GMGridView:cellForItemAtIndex:. This way you don't need to worry much about which views are onscreen, as that largely takes care of itself.

I have paged-loading grids with 24+ items onscreen at once, all loaded asynchronously (although often from cached data). The loading works fine, but without something preventing re-layout on every scroll, I get pretty laggy scrolling (particularly on iPad 1's).

My biggest concern when paging is the lack of a method similar to UITableView's -insertRowsAtIndexPaths:withRowAnimation:. Multiple -insertObjectAtIndex:... calls cause multiple re-layouts, or I have to call -reloadData, and I don't feel a clear sense of the expected behavior of the scroll content offset that results.

@gmoledina
Owner

As explained by @mwyman, you can keep the reference to the cell.

Another approach would be to just keep the index of the cell and then call the cellForItemAtIndex: method (of the grid class, not talking about the delegate method) that will only return the cell if it hasn't been recycled.

I will add an example in the demo app eventually. You can create a new issue for the performance related concerns.

@gmoledina gmoledina closed this Apr 17, 2012
@joaufeld

When you talk about datasource and loading, does this mean that you load the cells from a file?
If yes, would it be possible to include a sample of how to do that in the example project?
For a beginner like me it would be a great help.

@roshankularatna

Check whether use casting mechanism for view.Some time this is one of issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment