njdehoog / NHBalancedFlowLayout

UICollectionViewLayout subclass for displaying items of different sizes in a grid without wasting any visual space. Inspired by: http://www.crispymtn.com/stories/the-algorithm-for-a-perfectly-balanced-photo-gallery
MIT License
1.34k stars 106 forks source link

Performance issue with large image galleries #4

Closed mwyman closed 10 years ago

mwyman commented 10 years ago

Thanks for this layout implementation. I found a performance problem, however, that I think is related to the Objective-C implementation of the linear partitioning algorithm.

I tried pulling this into a project displaying particularly large image galleries (say >500 images) on an iPad, aiming to have the images be ~150pt tall, giving on the order of 90 partitions.

When laying out the galley, the collection layout spent about 20 seconds each time it hit NHLinearPartition +linearPartitionForSequence:numberOfPartitions:, and Instruments particularly identified line 64:

m[x] = @{@"0": @(MAX([table[x][j-1] integerValue], [table[i][0] integerValue] - [table[x][0] integerValue])), @"1": @(x)};

nhbalancedflowlayout

This implementation of Linear Partitioning does a lot of Objective-C object-creation and method-calls in a very tight loop (the algorithm should be O(k•n^2) ). I doubt the sort is required—the Skiena algorithm this is based on doesn't perform a sort.

njdehoog commented 10 years ago

Thanks for doing the research on this. I was expecting this could be a problem with larger data sets, but I had not tested this yet.

You're probably right that using a pure C implementation for the algorithm would increase performance dramatically. I don't have a lot of experience writing C code myself, so I might not get around to doing something about this for a while. If you are able to write a better/faster implementation of the algorithm, please issue a pull request, and I'll merge it back in.

ddaddy commented 10 years ago

I'm also seeing performance issues with this :( It's looks really good in my app, but after implementing it the performance is poor when initially adding the images to the cell's.

I download images on a background thread for each cell and when finished I call 'reloadItemsAtIndexPaths' for that cell. At this point the scrolling gets really jittery.

If I change the layout to a standard UICollectionViewFlowLayout it flies.

mwyman commented 10 years ago

ddaddy, in general you should not be reloading the cells when network connections come back with the image. You should really check out something like AFNetwork's asynchronous image views, as they can have much better scrolling performance in large collections or table views.

ddaddy commented 10 years ago

I have all the necessary background operations to make it all run smoothly. Like I said above, it runs perfectly fine when I substitute this layout for a standard one. The bottleneck is with the actual refresh of the cell or collectionView

njdehoog commented 10 years ago

Every time you do a reload on the collection view it needs to recalculate the positioning information for all the cells. That's probably what's causing the lag. Do you know the sizes of the images before you download them? Because then you wouldn't have to reload the cells.

ddaddy commented 10 years ago

How can I add the image to the cell without reloading the cell? A lot of the images are on the device. I use an NSOperationQueue to check each one and if it's not available it downloads it from the net. The NSOperation returns with an image either from disc or web.

I guess the recalculation is just too complex and I may have to look at a simpler approach.

Thanks for your work.

njdehoog commented 10 years ago

When an operation completes you could ask the collection view for the corresponding cell by calling cellForItemAtIndexPath, and then set the image directly, without reloading the cell.

ddaddy commented 10 years ago

That's an idea, I hadn't thought of that. I'll try now, thanks.

njdehoog commented 10 years ago

The main performance issues should be fixed in: 817a6ab