Proper way to deal with cell reuse with background threads?

Question

I have a UICollectionView, but the same methods should apply to UITableViews. Each of my cells contains an image I load from disk, which is a slow operation. To mitigate this, I use an async dispatch queue. This works fine, but quickly scrolling results in these operations stacking up, such that the cell will change its image from one to another in sequence, until finally stopping at the last call.

In the past, I've done a check to see if the cell was still visible, and if not I don't continue. However, this is not working with UICollectionView, and anyway it is not efficient. I am considering migrating to use NSOperations, which can be cancelled, so that only the last call to modify the cell will go through. I could possibly do this by checking if the operation had finished in the prepareForReuse method, and cancelling it if not. I'm hoping someone has dealt with this issue in the past and can provide some tips or a solution.

Answer 1

Session 211 - Building Concurrent User Interfaces on iOS, from WWDC 2012, discusses the problem of the cell's image changing as the background tasks catch up (starting at 38m15s).

Here's how you solve that problem. After you've loaded the image on the background queue, use the index path to find the current cell, if any, that's displaying the item containing that image. If you get a cell, set the cell's image. If you get nil, there's no cell currently displaying that item so just discard the image.

- (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath {
    MyCell *cell = [collectionView dequeueReusableCellWithReuseIdentifier:@"Cell" forIndexPath:indexPath];
    cell.imageView.image = nil;
    dispatch_async(myQueue, ^{
        [self bg_loadImageForRowAtIndexPath:indexPath];
    });
    return cell;
}

- (void)bg_loadImageForRowAtIndexPath:(NSIndexPath *)indexPath {
    // I assume you have a method that returns the path of the image file.  That
    // method must be safe to run on the background queue.
    NSString *path = [self bg_pathOfImageForItemAtIndexPath:indexPath];
    UIImage *image = [UIImage imageWithContentsOfFile:path];
    // Make sure the image actually loads its data now.
    [image CGImage];

    dispatch_async(dispatch_get_main_queue(), ^{
        [self setImage:image forItemAtIndexPath:indexPath];
    });
}

- (void)setImage:(UIImage *)image forItemAtIndexPath:(NSIndexPath *)indexPath {
    MyCell *cell = (MyCell *)[collectionView cellForItemAtIndexPath:indexPath];
    // If cell is nil, the following line has no effect.
    cell.imageView.image = image;
}

Here's an easy way to reduce the "stacking up" of background tasks loading images that the view no longer needs. Give yourself an NSMutableSet instance variable:

@implementation MyViewController {
    dispatch_queue_t myQueue;
    NSMutableSet *indexPathsNeedingImages;
}

When you need to load an image, put the index path of the cell in the set and dispatch a task that takes one index path out of the set and loads its image:

- (UICollectionViewCell *)collectionView:(UICollectionView *)collectionView cellForItemAtIndexPath:(NSIndexPath *)indexPath {
    MyCell *cell = [collectionView dequeueReusableCellWithReuseIdentifier:@"Cell" forIndexPath:indexPath];
    cell.imageView.image  = nil;
    [self addIndexPathToImageLoadingQueue:indexPath];
    return cell;
}

- (void)addIndexPathToImageLoadingQueue:(NSIndexPath *)indexPath {
    if (!indexPathsNeedingImages) {
        indexPathsNeedingImages = [NSMutableSet set];
    }
    [indexPathsNeedingImages addObject:indexPath];
    dispatch_async(myQueue, ^{ [self bg_loadOneImage]; });
}

If a cell stops being displayed, remove the cell's index path from the set:

- (void)collectionView:(UICollectionView *)collectionView didEndDisplayingCell:(UICollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath {
    [indexPathsNeedingImages removeObject:indexPath];
}

To load an image, get an index path from the set. We have to dispatch back to the main queue for this, to avoid a race condition on the set:

- (void)bg_loadOneImage {
    __block NSIndexPath *indexPath;
    dispatch_sync(dispatch_get_main_queue(), ^{
        indexPath = [indexPathsNeedingImages anyObject];
        if (indexPath) {
            [indexPathsNeedingImages removeObject:indexPath];
        }
    }

    if (indexPath) {
        [self bg_loadImageForRowAtIndexPath:indexPath];
    }
}

The bg_loadImageForRowAtIndexPath: method is the same as above. But when we actually set the image in the cell, we also want to remove the cell's index path from the set:

- (void)setImage:(UIImage *)image forItemAtIndexPath:(NSIndexPath *)indexPath {
    MyCell *cell = (MyCell *)[collectionView cellForItemAtIndexPath:indexPath];
    // If cell is nil, the following line has no effect.
    cell.imageView.image = image;

    [indexPathsNeedingImages removeObject:indexPath];
}

Answer 2

Have you tried using SDWebImage open source framework that works with downloading images from the web and caching them asynchronously? It works absolutely great with UITableViews and should be as good with UICollectionViews. You just use the setImage:withURL: method of the UIImageView extension and it does magic.

Answer 3

I had the same problem with an UITableView of about 400 elements and find a brutal but working solution: don't reuse cells! Depends on how much and how big the images are, but actually iOS is quite efficient in deallocating unused cells... Again: is not what purist will suggest, but couple with async load is lighting quick, error free and don't have to deal with complex sync logic.