Objective-c threading and cpu performance

Question

I have an app which downloads some files from the server in few threads. The problems is that it is giving a heavy load to the CPU (hitting to 80%). What can be done to make it better? I made similar app on Windows with C#, and the cpu usage never goes above 5%.

EDIT: This code has been changed after getting some suggestions below. The problem now is, that the download never reaches 100% when I set [queue setMaxConcurrentOperationCount:6]. If I change the asynchronous NSURLConnection back to sendSynchronous call it works, when I change the above OperationCount to 1, also works.

This is how I add NSOperations to the queue (may be large, like 800).

int chunkId = 0;
for (DownloadFile *downloadFile in [download filesInTheDownload])
{
    chunkId = 0;
    for (DownloadChunk *downloadChunk in [downloadFile chunksInTheFile])
    {
        DownloadChunkOperation *operation = [[DownloadChunkOperation alloc]  initWithDownloadObject:download
      downloadFile:downloadFile                                                                                                    downloadChunk:downloadChunk                                                                                                           andChunkId:chunkId];
    [queue addOperation:operation];
    chunkId++;
    }
}


#import "DownloadChunkOperation.h"
#import "Download.h"
#import "DownloadFile.h"
#import "DownloadChunk.h"

@interface DownloadChunkOperation()
@property(assign) BOOL isExecuting;
@property(assign) BOOL isFinished;
@end

@implementation DownloadChunkOperation

@synthesize download = _download;
@synthesize downloadFile = _downloadFile;
@synthesize downloadChunk = _downloadChunk;

@synthesize isFinished = _isFinished;
@synthesize isExecuting = _isExecuting;

- (id) initWithDownloadObject:(Download *)download downloadFile:(DownloadFile *)downloadFile downloadChunk:(DownloadChunk *)downloadChunk andChunkId:(uint32_t)chunkId
{
    self = [super init];

    if (self) {
        self.download = download;
        self.downloadFile = downloadFile;
        self.downloadChunk = downloadChunk;
        self.chunkId = chunkId;
    }

    return self;
}

- (void) start
{
    if ([self isCancelled]) {
        [self setIsFinished:YES];
        [self setIsExecuting:NO];
        return;
    }

    [self setIsExecuting:YES];
    [self setIsFinished:NO];
    [self.downloadChunk setChunkState:cDownloading];

    downloadPath = [[NSString stringWithFormat:@"%@/%@", [self.download downloadFolder], [self.download escapedTitle]] stringByExpandingTildeInPath];

    NSURL *fileURL = [[NSURL alloc] initWithString:[self.downloadFile filePath]];
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:fileURL];
    NSString *range = [NSString stringWithFormat:@"bytes=%lli-%lli", [self.downloadChunk startingByte], [self.downloadChunk endingByte]];
    [request setValue:range forHTTPHeaderField:@"Range"];
    connection = [[NSURLConnection alloc] initWithRequest:request delegate:self startImmediately:NO];
    // IMPORTANT! The next line is what keeps the NSOperation alive for the during of the NSURLConnection!
    [connection scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSDefaultRunLoopMode];
    [connection start];

    if (connection) {
        NSLog(@"connection established!");
        do {
            [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantFuture]];
        } while (!self.isFinished);
    } else {
        NSLog(@"couldn't establish connection for: %@", fileURL);
    }
}

- (BOOL) isConcurrent
{
    return YES;
}

- (void) connection:(NSURLConnection *)_connection didReceiveResponse:(NSURLResponse *)response
{
    receivedData = [[NSMutableData alloc] init];
}

- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data
{
    // Not cancelled, receive data.
    if (![self isCancelled]) {
        [receivedData appendData:data];
        self.download.downloadedBytes += [data length];
        return;
    }

    // Cancelled, tear down connection.
    [self setIsExecuting:NO];
    [self setIsFinished:YES];
    [self.downloadChunk setChunkState:cConnecting];
    [self->connection cancel];
}

- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
{
    [self setIsExecuting:NO];
    [self setIsFinished:YES];
    NSLog(@"Connection failed! Error - %@ %@",
          [error localizedDescription],
          [[error userInfo] objectForKey:NSURLErrorFailingURLStringErrorKey]);
}

- (void)connectionDidFinishLoading:(NSURLConnection *)connection
{
    NSString *chunkPath = [downloadPath stringByAppendingFormat:@"/%@.%i", [self.downloadFile fileName], self.chunkId];
    NSError *saveError = nil;
    [receivedData writeToFile:chunkPath options:NSDataWritingAtomic error:&saveError];
    if (saveError != nil) {
        NSLog(@"Download save failed! Error: %@", [saveError description]);
    }
    else {
        NSLog(@"file has been saved!: %@", chunkPath);
    }
    [self setIsExecuting:NO];
    [self setIsFinished:YES];
    [self.downloadChunk setChunkState:cFinished];

    if ([self.download downloadedBytes] == [self.download size])
        [[NSNotificationCenter defaultCenter] postNotificationName:@"downloadFinished" object:self.download];
}

@end

Answer 1

You should not create threads yourself. Use dedicated API like NSOperationQueue or even GCD directly for this purpose. They know better about hardware limits, virtual cores, etc. and support priority settings.

You shouldn't use +sendSynchronousRequest: either. Wrapping your -downloadChunk method in a dispatch call as suggested by charith won't help you improve performance, as +sendSynchronousRequest: blocks the thread until new data comes in and forces GCD to spawn new threads.

Use the asynchronous API of NSURLConnection using delegate callbacks. You can also wrap your NSURLConnection code inside a NSOperation subclass and use NSOperationQueue to manage the downloads: Using NSURLConnections

If you don't want to write the NSOperation subclass yourself, you can also use a 3rd party framework like AFNetworking.

Answer 2

Try with GCD blocks and global queues. This is the apple recommended way now for concurrency ex:

dispatch_queue_t globalQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    dispatch_async(globalQueue, ^{

        [self downloadChunk:objDownload];
    });