Why does my thread seem to fail after multiple threads started on iOS?
I have this code in a delegate call - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
:
dispatch_async( dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
AVPlayerItem *playerItem = [AVPlayerItem playerItemWithURL:[webUrls objectAtIndex:indexPath.row]];
CMTime timeduration = playerItem.duration;
float seconds = CMTimeGetSeconds(timeduration);
NSString *duration = [NSString stringWithFormat:@"%f", seconds];
dispatch_async( dispatch_get_main_queue(), ^{
UITableViewCell *updatecell = [tblView cellForRowAtIndexPath:indexPath];
updatecell.detailTextLabel.text = duration;
[updatecell setNeedsLayout];
});
});
Each cell slowly, in the background, loads seconds
into updatecell.detailTextLabel.text
the cell. The problem is that after I scroll, after 3 or 4 cells are loaded, the rest just show 0 in detailTextLabel quickly and won't load.
Any ideas why this is so? Am I not executing my threads correctly?
source to share
A few thoughts:
-
Many servers impose a limit on the number of concurrent requests they accept from a given client. I suggest you use
NSOperationQueue
to limit your server's concurrent requests to 4 or 5, instead of using send queues. -
You are potentially making the problem worse than it should be, because if you scroll down the table view and then do a backup, when you re-render the first few cells, you re-load
AVPlayerItem
and try to execute additional concurrent requests from your server. You really should keep the results of your previous downloads to eliminate the need for redundant re-requests for the same data. -
Currently, you don't check if the cell you just loaded is saved before trying to update the interface. You should really check it out.
So, I might suggest the following:
-
In your controller,
viewDidLoad
createNSOperationQueue
which we will use to load. Also specify how many concurrent operations your server will allow:downloadQueue = [[NSOperationQueue alloc] init]; downloadQueue.maxConcurrentOperationCount = 4; // replace this with an appropriate value for your server
-
Before that, you had an array
webUrls
which was an array of objectsNSURL
. In point # 4 below we will look at the issue of exiting this array and creating a new array of string objects. But before we can do that, we must create this new objectRowData
.Each string object will have not only
webURL
but also other things likedurationText
and maybe evenAVPlayerItem
. (By keeping these other properties of the object as the cell scrolls back, we don't need to reload the data.) So the public interface for this new class might look like this:// // RowData.h // #import <Foundation/Foundation.h> @class AVPlayerItem; @interface RowData : NSObject @property (nonatomic, strong) NSURL *webURL; @property (nonatomic, strong) NSString *durationText; @property (nonatomic, strong) AVPlayerItem *playerItem; @property (nonatomic, getter = isDownloaded, readonly) BOOL downloaded; @property (nonatomic, getter = isDownloading, readonly) BOOL downloading; - (void)downloadInQueue:(NSOperationQueue *)queue completion:(void (^)(BOOL success))block; - (void)cancelDownload; @end
By the way, I'm not crazy about the class name
RowData
. This is a little too ambiguous. But I don't know enough about the nature of your model data to suggest a better name. Feel free to name this class as you see fit. -
Your new class
RowData
might have an instance method nameddownloadInQueue
that does the loading, setsdurationText
appropriately, etc. By moving the loading logic here, we successfully isolatecellForRowAtIndexPath
from some of the loading related details. It is also important that this methoddownloadInQueue
doesnโt update the UI itself, but rather has the blockcompletion
providedcellForRowAtIndexPath
(demonstrated in # 5 below), so this methoddownloadInQueue
doesnโt have to worry about UI considerations. In any case, the implementationdownloadInQueue
might look like this:// // RowData.m // #import "RowData.h" #import <AVFoundation/AVFoundation.h> @interface RowData () @property (nonatomic, getter = isDownloaded) BOOL downloaded; @property (nonatomic, getter = isDownloading) BOOL downloading; @property (nonatomic, weak) NSOperation *operation; @end @implementation RowData - (void)downloadInQueue:(NSOperationQueue *)queue completion:(void (^)(BOOL success))completion { if (!self.isDownloading) { self.downloading = YES; NSOperation *currentOperation = [NSBlockOperation blockOperationWithBlock:^{ BOOL success = NO; self.playerItem = [AVPlayerItem playerItemWithURL:self.webURL]; if (self.playerItem) { success = YES; CMTime timeduration = self.playerItem.duration; float seconds = CMTimeGetSeconds(timeduration); self.durationText = [NSString stringWithFormat:@"%f", seconds]; } self.downloading = NO; self.downloaded = YES; [[NSOperationQueue mainQueue] addOperationWithBlock:^{ completion(success); }]; }]; [queue addOperation:currentOperation]; self.operation = currentOperation; } } - (void)cancelDownload { if ([self isDownloading] && self.operation) { self.downloading = NO; [self.operation cancel]; } } @end
-
In the main view controller, instead of creating an old array,
webUrls
create a new array from these objectsRowData
called for exampleobjects
. Set a propertywebURL
for each of these objects , of courseRowData
. (Again, I'm not crazy about the ambiguous nameobjects
, but I don't know enough about your application to make a more specific suggestion. Call it what you want. But my code below will use itobjects
.) -
Finally, change yours
cellForRowAtIndexPath
to use this new objectRowData
and methoddownloadInQueue
. Also, note that the blockcompletion
checks that the cell is still visible:RowData *rowData = self.objects[indexPath.row]; if ([rowData isDownloaded]) { cell.detailTextLabel.text = rowData.durationText; } else { cell.detailTextLabel.text = @"..."; // you really should initialize this so we show something during download or remove anything previously there [rowData downloadInQueue:self.downloadQueue completion:^(BOOL success) { // note, if you wanted to change your behavior based upon whether the // download was successful or not, just use the `success` variable UITableViewCell *updateCell = [tblView cellForRowAtIndexPath:indexPath]; // by the way, make sure the cell is still on screen if (updateCell) { updateCell.detailTextLabel.text = rowData.durationText; [updateCell setNeedsLayout]; } }]; }
-
If you are using iOS 6, if you want to cancel pending downloads when a cell is scrolled off the screen, you can use the
didEndDisplayingCell
protocol methodUITableViewDelegate
.- (void)tableView:(UITableView *)tableView didEndDisplayingCell:(UITableViewCell *)cell forRowAtIndexPath:(NSIndexPath *)indexPath { RowData *rowData = self.objects[indexPath.row]; if ([rowData isDownloading]) [rowData cancelDownload]; }
If you support earlier versions of iOS, you will have to use protocol methods
UIScrollViewDelegate
such asscrollViewDidScroll
determining which cells (for example, not included inindexPathsForVisibleRows
) manually, but the idea is the same.
By the way, in my example RowData
above, I am saving AVPlayerItem
. You should only do this if you need it AVPlayerItem
later. We've saved duration
which achieves everything we need for UITableViewCell
, but I'm guessing you might want to do something with AVPlayerItem
, so I'll save it too. But if you don't need this one AVPlayerItem
later, don't save it to an object RowData
. Also, I don't know how big they are, but you can write didReceiveMemoryWarning
that will loop through yours objects
and set an playerItem
object for each element nil
.
source to share