From c4cd101bd5cfb0b6624ed660e2c7f09624552127 Mon Sep 17 00:00:00 2001 From: Mike Abdullah Date: Sun, 10 Nov 2013 20:09:10 +0000 Subject: [PATCH 1/5] .maxConcurrentOperationCount property, defaulting to 1 --- ConnectionKit/CKUploader.h | 2 ++ ConnectionKit/CKUploader.m | 2 ++ 2 files changed, 4 insertions(+) diff --git a/ConnectionKit/CKUploader.h b/ConnectionKit/CKUploader.h index 7c661545c..f0eb41a7c 100644 --- a/ConnectionKit/CKUploader.h +++ b/ConnectionKit/CKUploader.h @@ -27,6 +27,7 @@ typedef NSUInteger CKUploadingOptions; @private NSURLRequest *_request; unsigned long _permissions; + NSUInteger _maxConcurrentOperationCount; CKUploadingOptions _options; CK2FileManager *_fileManager; @@ -47,6 +48,7 @@ typedef NSUInteger CKUploadingOptions; filePosixPermissions:(NSNumber *)customPermissions options:(CKUploadingOptions)options; +@property (nonatomic) NSUInteger maxConcurrentOperationCount; // defaults to 1 @property (nonatomic, assign, readonly) CKUploadingOptions options; @property (nonatomic, assign) id delegate; diff --git a/ConnectionKit/CKUploader.m b/ConnectionKit/CKUploader.m index 23fab2c26..2a7dd990e 100644 --- a/ConnectionKit/CKUploader.m +++ b/ConnectionKit/CKUploader.m @@ -19,6 +19,7 @@ - (id)initWithRequest:(NSURLRequest *)request filePosixPermissions:(unsigned lon { _request = [request copy]; _permissions = customPermissions; + _maxConcurrentOperationCount = 1; _options = options; if (!(_options & CKUploadingDryRun)) @@ -62,6 +63,7 @@ - (void)dealloc @synthesize delegate = _delegate; @synthesize options = _options; +@synthesize maxConcurrentOperationCount = _maxConcurrentOperationCount; @synthesize rootTransferRecord = _rootRecord; @synthesize baseTransferRecord = _baseRecord; From f4357f433ef292ba6d655e632230c8dc2f46aa41 Mon Sep 17 00:00:00 2001 From: Mike Abdullah Date: Sun, 10 Nov 2013 22:27:05 +0000 Subject: [PATCH 2/5] Reign in number of concurrent ops when needed --- ConnectionKit/CKUploader.m | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/ConnectionKit/CKUploader.m b/ConnectionKit/CKUploader.m index 2a7dd990e..9a46571e1 100644 --- a/ConnectionKit/CKUploader.m +++ b/ConnectionKit/CKUploader.m @@ -63,7 +63,34 @@ - (void)dealloc @synthesize delegate = _delegate; @synthesize options = _options; + @synthesize maxConcurrentOperationCount = _maxConcurrentOperationCount; +- (NSUInteger)effectiveConcurrentOperationCount; +{ + NSUInteger result = self.maxConcurrentOperationCount; + + // Reign in count when: + // * Using CURLHandle sync backend + // * Deleting files before uploading; have no dependency system to handle that + if (result > 1) + { + if (self.options & CKUploadingDeleteExistingFileFirst) + { + result = 1; + } + else + { + NSString *scheme = _request.URL.scheme; + if ([scheme caseInsensitiveCompare:@"ftp"] == NSOrderedSame || [scheme caseInsensitiveCompare:@"sftp"] == NSOrderedSame) + { + result = 1; + } + } + } + + return result; +} + @synthesize rootTransferRecord = _rootRecord; @synthesize baseTransferRecord = _baseRecord; From c1b5d3f1309fa3cad51f9f1b6ee05583a4920706 Mon Sep 17 00:00:00 2001 From: Mike Abdullah Date: Sun, 10 Nov 2013 23:30:42 +0000 Subject: [PATCH 3/5] Make a pretty good stab at parallel uploading --- ConnectionKit/CKUploader.h | 1 - ConnectionKit/CKUploader.m | 118 ++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 29 deletions(-) diff --git a/ConnectionKit/CKUploader.h b/ConnectionKit/CKUploader.h index f0eb41a7c..abbf673fc 100644 --- a/ConnectionKit/CKUploader.h +++ b/ConnectionKit/CKUploader.h @@ -31,7 +31,6 @@ typedef NSUInteger CKUploadingOptions; CKUploadingOptions _options; CK2FileManager *_fileManager; - id _currentOperation; NSMutableArray *_queue; CKTransferRecord *_rootRecord; diff --git a/ConnectionKit/CKUploader.m b/ConnectionKit/CKUploader.m index 9a46571e1..c8b7a1c9f 100644 --- a/ConnectionKit/CKUploader.m +++ b/ConnectionKit/CKUploader.m @@ -9,6 +9,20 @@ #import "CKUploader.h" +@interface CKUploaderOperation : NSOperation +{ + id (^_block)(void); + id _fileOp; +} + +- initWithBlock:(id (^)(void))block __attribute((nonnull)); + +@end + + +#pragma mark - + + @implementation CKUploader #pragma mark Lifecycle @@ -117,18 +131,21 @@ - (void)removeFileAtPath:(NSString *)path; - (void)removeFileAtPath:(NSString *)path reportError:(BOOL)reportError; { - [self addOperationWithBlock:^{ - + __block CKUploaderOperation *op = [[CKUploaderOperation alloc] initWithBlock:^id{ + return [_fileManager removeItemAtURL:[self URLForPath:path] completionHandler:^(NSError *error) { - [self operationDidFinish:(reportError ? error : nil)]; + [self operation:op didFinish:(reportError ? error : nil)]; }]; }]; + + [self addOperation:op]; + [op release]; } - (CKTransferRecord *)uploadData:(NSData *)data toPath:(NSString *)path; { - return [self uploadToPath:path size:data.length usingBlock:^id(CKTransferRecord *record) { + __block CKTransferRecord *record = [self transferRecordWithPath:path size:data.length usingBlock:^id { NSDictionary *attributes = @{ NSFilePosixPermissions : @([self posixPermissionsForPath:path isDirectory:NO]) }; @@ -144,7 +161,7 @@ - (CKTransferRecord *)uploadData:(NSData *)data toPath:(NSString *)path; [record transferDidFinish:record error:error]; }); - [self operationDidFinish:error]; + [self operation:[record propertyForKey:@"uploadOperation"] didFinish:error]; }]; NSAssert(op, @"Failed to create upload operation"); @@ -157,6 +174,9 @@ - (CKTransferRecord *)uploadData:(NSData *)data toPath:(NSString *)path; return op; }]; + + [self addOperation:[record propertyForKey:@"uploadOperation"]]; + return record; } - (CKTransferRecord *)uploadFileAtURL:(NSURL *)localURL toPath:(NSString *)path; @@ -164,7 +184,7 @@ - (CKTransferRecord *)uploadFileAtURL:(NSURL *)localURL toPath:(NSString *)path; NSNumber *size; if (![localURL getResourceValue:&size forKey:NSURLFileSizeKey error:NULL]) size = nil; - return [self uploadToPath:path size:size.unsignedLongLongValue usingBlock:^id(CKTransferRecord *record) { + __block CKTransferRecord *record = [self transferRecordWithPath:path size:size.unsignedLongLongValue usingBlock:^id { NSDictionary *attributes = @{ NSFilePosixPermissions : @([self posixPermissionsForPath:path isDirectory:NO]) }; @@ -180,7 +200,7 @@ - (CKTransferRecord *)uploadFileAtURL:(NSURL *)localURL toPath:(NSString *)path; [record transferDidFinish:record error:error]; }); - [self operationDidFinish:error]; + [self operation:[record propertyForKey:@"uploadOperation"] didFinish:error]; }]; NSAssert(op, @"Failed to create upload operation"); @@ -193,9 +213,12 @@ - (CKTransferRecord *)uploadFileAtURL:(NSURL *)localURL toPath:(NSString *)path; return op; }]; + + [self addOperation:[record propertyForKey:@"uploadOperation"]]; + return record; } -- (CKTransferRecord *)uploadToPath:(NSString *)path size:(unsigned long long)size usingBlock:(id (^)(CKTransferRecord *record))block; +- (CKTransferRecord *)transferRecordWithPath:(NSString *)path size:(unsigned long long)size usingBlock:(id (^)(void))block; { // Create transfer record if (_options & CKUploadingDeleteExistingFileFirst) @@ -208,9 +231,9 @@ - (CKTransferRecord *)uploadToPath:(NSString *)path size:(unsigned long long)siz // Enqueue upload - [self addOperationWithBlock:^id{ - return block(result); - }]; + CKUploaderOperation *op = [[CKUploaderOperation alloc] initWithBlock:block]; + [result setProperty:op forKey:@"uploadOperation"]; + [op release]; // Notify delegate @@ -239,7 +262,7 @@ - (void)finishUploading; NSAssert(!self.isCancelled, @"Shouldn't be able to finish once cancelled!"); [[self delegate] uploaderDidFinishUploading:self]; - [self operationDidFinish:nil]; + [self operation:nil didFinish:nil]; return nil; }]; @@ -251,43 +274,40 @@ - (void)finishUploading; - (void)cancel; { _isCancelled = YES; - [_fileManager cancelOperation:self.currentOperation]; [_queue makeObjectsPerformSelector:_cmd]; [_queue release]; _queue = nil; } - (BOOL)isCancelled; { return _isCancelled; } -- (id)currentOperation; { return _currentOperation; } - // The block must return the operation vended to it by CK2FileManager - (void)addOperationWithBlock:(id (^)(void))block; +{ + CKUploaderOperation *operation = [[CKUploaderOperation alloc] initWithBlock:block]; + [self addOperation:operation]; + [operation release]; +} + +- (void)addOperation:(CKUploaderOperation *)operation; { NSAssert([NSThread isMainThread], @"-addOperation: is only safe to call on the main thread"); // No more operations can go on once finishing up if (_isFinishing) return; - NSBlockOperation *operation = [NSBlockOperation blockOperationWithBlock:^{ - - NSAssert(_currentOperation == nil, @"Seems like an op is starting before another has finished"); - _currentOperation = [block() retain]; - }]; - [_queue addObject:operation]; - if ([_queue count] == 1) + + if ([_queue count] <= self.effectiveConcurrentOperationCount) { [operation start]; } } -- (void)operationDidFinish:(NSError *)error; +- (void)operation:(CKUploaderOperation *)operation didFinish:(NSError *)error; { // This method gets called on all sorts of threads, so marshall back to main queue dispatch_async(dispatch_get_main_queue(), ^{ - [_currentOperation release]; _currentOperation = nil; - if (error) { if ([self.delegate respondsToSelector:@selector(uploader:shouldProceedAfterError:)]) @@ -305,10 +325,16 @@ - (void)operationDidFinish:(NSError *)error; } } - [_queue removeObjectAtIndex:0]; - if ([_queue count]) + NSUInteger index = [_queue indexOfObject:operation]; + NSAssert(index != NSNotFound, @"Finished operation should be in the queue"); + [_queue removeObjectAtIndex:index]; + + // Dequeue next op + // TODO: Check for ops which should have been started but haven't? + NSUInteger maxOps = self.effectiveConcurrentOperationCount; + if (_queue.count >= maxOps) { - [[_queue objectAtIndex:0] start]; + [[_queue objectAtIndex:(maxOps - 1)] start]; } }); } @@ -391,3 +417,39 @@ - (void)fileManager:(CK2FileManager *)manager appendString:(NSString *)info toTr @end + +#pragma mark - + + +@implementation CKUploaderOperation + +- initWithBlock:(id (^)(void))block; +{ + NSParameterAssert(block); + if (self = [self init]) + { + _block = [block copy]; + } + return self; +} + +- (void)main; +{ + _fileOp = [_block() retain]; +} + +- (void)cancel; +{ + [super cancel]; + [_fileOp cancel]; +} + +- (void)dealloc; +{ + [_block release]; + [_fileOp release]; + + [super dealloc]; +} + +@end From e1f3b3c3f2766f0511aceabe031b96d585f292e6 Mon Sep 17 00:00:00 2001 From: Mike Abdullah Date: Mon, 11 Nov 2013 10:42:45 +0000 Subject: [PATCH 4/5] Properly finish when doing parallel uploading --- ConnectionKit/CKUploader.m | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ConnectionKit/CKUploader.m b/ConnectionKit/CKUploader.m index c8b7a1c9f..700aeb01b 100644 --- a/ConnectionKit/CKUploader.m +++ b/ConnectionKit/CKUploader.m @@ -258,15 +258,14 @@ - (NSURL *)URLForPath:(NSString *)path; - (void)finishUploading; { - [self addOperationWithBlock:^id{ - - NSAssert(!self.isCancelled, @"Shouldn't be able to finish once cancelled!"); - [[self delegate] uploaderDidFinishUploading:self]; - [self operation:nil didFinish:nil]; - return nil; - }]; + if (_isFinishing) return; _isFinishing = YES; + if (_queue.count == 0) + { + NSAssert(!self.isCancelled, @"Shouldn't be able to finish once cancelled!"); + [[self delegate] uploaderDidFinishUploading:self]; + } } #pragma mark Queue @@ -336,6 +335,11 @@ - (void)operation:(CKUploaderOperation *)operation didFinish:(NSError *)error; { [[_queue objectAtIndex:(maxOps - 1)] start]; } + else if (_isFinishing && _queue.count == 0) + { + NSAssert(!self.isCancelled, @"Shouldn't be able to finish once cancelled!"); + [[self delegate] uploaderDidFinishUploading:self]; + } }); } From 18c1d1ed6ec9098c28e458084f3298c3cf6a343c Mon Sep 17 00:00:00 2001 From: Mike Abdullah Date: Mon, 11 Nov 2013 10:48:09 +0000 Subject: [PATCH 5/5] Mostly handle cancellation --- ConnectionKit/CKUploader.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ConnectionKit/CKUploader.m b/ConnectionKit/CKUploader.m index 700aeb01b..9d0198c3a 100644 --- a/ConnectionKit/CKUploader.m +++ b/ConnectionKit/CKUploader.m @@ -258,7 +258,7 @@ - (NSURL *)URLForPath:(NSString *)path; - (void)finishUploading; { - if (_isFinishing) return; + if (_isFinishing || _isCancelled) return; _isFinishing = YES; if (_queue.count == 0) @@ -439,6 +439,7 @@ @implementation CKUploaderOperation - (void)main; { + if (self.isCancelled) return; _fileOp = [_block() retain]; }