From 39af2be6ee8071ab8828e07c7ae7823d61bd0c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A9=AC=E9=81=A5?= Date: Wed, 26 Jul 2023 15:23:59 +0800 Subject: [PATCH 1/3] fix redundant url requests --- SDWebImage/Core/SDWebImageDownloader.m | 24 +-- .../Core/SDWebImageDownloaderOperation.m | 155 +++++++++++------- 2 files changed, 95 insertions(+), 84 deletions(-) diff --git a/SDWebImage/Core/SDWebImageDownloader.m b/SDWebImage/Core/SDWebImageDownloader.m index cfd26461..628057fc 100644 --- a/SDWebImage/Core/SDWebImageDownloader.m +++ b/SDWebImage/Core/SDWebImageDownloader.m @@ -21,22 +21,6 @@ NSNotificationName const SDWebImageDownloadStopNotification = @"SDWebImageDownlo NSNotificationName const SDWebImageDownloadFinishNotification = @"SDWebImageDownloadFinishNotification"; static void * SDWebImageDownloaderContext = &SDWebImageDownloaderContext; -static void * SDWebImageDownloaderOperationKey = &SDWebImageDownloaderOperationKey; - -BOOL SDWebImageDownloaderOperationGetCompleted(id operation) { - NSCParameterAssert(operation); - NSNumber *value = objc_getAssociatedObject(operation, SDWebImageDownloaderOperationKey); - if (value != nil) { - return value.boolValue; - } else { - return NO; - } -} - -void SDWebImageDownloaderOperationSetCompleted(id operation, BOOL isCompleted) { - NSCParameterAssert(operation); - objc_setAssociatedObject(operation, SDWebImageDownloaderOperationKey, @(isCompleted), OBJC_ASSOCIATION_RETAIN); -} @interface SDWebImageDownloadToken () @@ -239,7 +223,7 @@ void SDWebImageDownloaderOperationSetCompleted(id BOOL shouldNotReuseOperation; if (operation) { @synchronized (operation) { - shouldNotReuseOperation = operation.isFinished || operation.isCancelled || SDWebImageDownloaderOperationGetCompleted(operation); + shouldNotReuseOperation = operation.isFinished || operation.isCancelled; } } else { shouldNotReuseOperation = YES; @@ -518,12 +502,6 @@ didReceiveResponse:(NSURLResponse *)response // Identify the operation that runs this task and pass it the delegate method NSOperation *dataOperation = [self operationWithTask:task]; - if (dataOperation) { - @synchronized (dataOperation) { - // Mark the downloader operation `isCompleted = YES`, no longer re-use this operation when new request comes in - SDWebImageDownloaderOperationSetCompleted(dataOperation, YES); - } - } if ([dataOperation respondsToSelector:@selector(URLSession:task:didCompleteWithError:)]) { [dataOperation URLSession:session task:task didCompleteWithError:error]; } diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index 0ec65281..d887d7f1 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -14,8 +14,6 @@ #import "SDImageCacheDefine.h" #import "SDCallbackQueue.h" -BOOL SDWebImageDownloaderOperationGetCompleted(id operation); // Private currently, mark open if needed - // A handler to represent individual request @interface SDWebImageDownloaderOperationToken : NSObject @@ -62,6 +60,8 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id @property (strong, nonatomic, nullable) NSError *responseError; @property (assign, nonatomic) double previousProgress; // previous progress percent +@property (assign, nonatomic, getter = isDownloadCompleted) BOOL downloadCompleted; + @property (strong, nonatomic, nullable) id responseModifier; // modify original URLResponse @property (strong, nonatomic, nullable) id decryptor; // decrypt image data @@ -112,6 +112,7 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id _finished = NO; _expectedSize = 0; _unownedSession = session; + _downloadCompleted = NO; _coderQueue = [[NSOperationQueue alloc] init]; _coderQueue.maxConcurrentOperationCount = 1; _coderQueue.name = @"com.hackemist.SDWebImageDownloaderOperation.coderQueue"; @@ -338,6 +339,91 @@ BOOL SDWebImageDownloaderOperationGetCompleted(id return YES; } +// Check for unprocessed tokens. +// if all tokens have been processed call [self done]. +- (void)checkDoneWithImageData:(NSData *)imageData + finisdTokens:(NSArray *)finisdTokens { + NSMutableArray *tokens; + @synchronized (self) { + tokens = [self.callbackTokens mutableCopy]; + } + [finisdTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) { + [tokens removeObjectIdenticalTo:obj]; + }]; + if (tokens.count == 0) { + [self done]; + } else { + // If there are new tokens added during the decoding operation, the decoding operation is supplemented with these new tokens. + [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finisdTokens]; + } +} + +- (void)startCoderOperationWithImageData:(NSData *)imageData + pendingTokens:(NSArray *)pendingTokens + finishedTokens:(NSArray *)finishedTokens { + @weakify(self); + for (SDWebImageDownloaderOperationToken *token in pendingTokens) { + [self.coderQueue addOperationWithBlock:^{ + @strongify(self); + if (!self) { + return; + } + UIImage *image; + // check if we already decode this variant of image for current callback + if (token.decodeOptions) { + image = [self.imageMap objectForKey:token.decodeOptions]; + } + if (!image) { + // check if we already use progressive decoding, use that to produce faster decoding + id progressiveCoder = SDImageLoaderGetProgressiveCoder(self); + SDWebImageOptions options = [[self class] imageOptionsFromDownloaderOptions:self.options]; + SDWebImageContext *context; + if (token.decodeOptions) { + SDWebImageMutableContext *mutableContext = [NSMutableDictionary dictionaryWithDictionary:self.context]; + SDSetDecodeOptionsToContext(mutableContext, &options, token.decodeOptions); + context = [mutableContext copy]; + } else { + context = self.context; + } + if (progressiveCoder) { + image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, options, context); + } else { + image = SDImageLoaderDecodeImageData(imageData, self.request.URL, options, context); + } + if (image && token.decodeOptions) { + [self.imageMap setObject:image forKey:token.decodeOptions]; + } + } + CGSize imageSize = image.size; + if (imageSize.width == 0 || imageSize.height == 0) { + NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels"; + NSError *error = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : description}]; + [self callCompletionBlockWithToken:token image:nil imageData:nil error:error finished:YES]; + } else { + [self callCompletionBlockWithToken:token image:image imageData:imageData error:nil finished:YES]; + } + }]; + } + // call [self done] after all completed block was dispatched + dispatch_block_t doneBlock = ^{ + @strongify(self); + if (!self) { + return; + } + // Check for new tokens added during the decode operation. + [self checkDoneWithImageData:imageData + finisdTokens:[finishedTokens arrayByAddingObjectsFromArray:pendingTokens]]; + }; + if (@available(iOS 13, tvOS 13, macOS 10.15, watchOS 6, *)) { + // seems faster than `addOperationWithBlock` + [self.coderQueue addBarrierBlock:doneBlock]; + } else { + // serial queue, this does the same effect in semantics + [self.coderQueue addOperationWithBlock:doneBlock]; + } + +} + #pragma mark NSURLSessionDataDelegate - (void)URLSession:(NSURLSession *)session @@ -478,7 +564,7 @@ didReceiveResponse:(NSURLResponse *)response } // When cancelled or transfer finished (`didCompleteWithError`), cancel the progress callback, only completed block is called and enough @synchronized (self) { - if (self.isCancelled || SDWebImageDownloaderOperationGetCompleted(self)) { + if (self.isCancelled || self.isDownloadCompleted) { return; } } @@ -521,6 +607,8 @@ didReceiveResponse:(NSURLResponse *)response // If we already cancel the operation or anything mark the operation finished, don't callback twice if (self.isFinished) return; + self.downloadCompleted = YES; + NSArray *tokens; @synchronized (self) { tokens = [self.callbackTokens copy]; @@ -565,64 +653,9 @@ didReceiveResponse:(NSURLResponse *)response } else { // decode the image in coder queue, cancel all previous decoding process [self.coderQueue cancelAllOperations]; - @weakify(self); - for (SDWebImageDownloaderOperationToken *token in tokens) { - [self.coderQueue addOperationWithBlock:^{ - @strongify(self); - if (!self) { - return; - } - UIImage *image; - // check if we already decode this variant of image for current callback - if (token.decodeOptions) { - image = [self.imageMap objectForKey:token.decodeOptions]; - } - if (!image) { - // check if we already use progressive decoding, use that to produce faster decoding - id progressiveCoder = SDImageLoaderGetProgressiveCoder(self); - SDWebImageOptions options = [[self class] imageOptionsFromDownloaderOptions:self.options]; - SDWebImageContext *context; - if (token.decodeOptions) { - SDWebImageMutableContext *mutableContext = [NSMutableDictionary dictionaryWithDictionary:self.context]; - SDSetDecodeOptionsToContext(mutableContext, &options, token.decodeOptions); - context = [mutableContext copy]; - } else { - context = self.context; - } - if (progressiveCoder) { - image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, options, context); - } else { - image = SDImageLoaderDecodeImageData(imageData, self.request.URL, options, context); - } - if (image && token.decodeOptions) { - [self.imageMap setObject:image forKey:token.decodeOptions]; - } - } - CGSize imageSize = image.size; - if (imageSize.width == 0 || imageSize.height == 0) { - NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels"; - NSError *error = [NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : description}]; - [self callCompletionBlockWithToken:token image:nil imageData:nil error:error finished:YES]; - } else { - [self callCompletionBlockWithToken:token image:image imageData:imageData error:nil finished:YES]; - } - }]; - } - // call [self done] after all completed block was dispatched - dispatch_block_t doneBlock = ^{ - @strongify(self); - if (!self) { - return; - } - [self done]; - }; - if (@available(iOS 13, tvOS 13, macOS 10.15, watchOS 6, *)) { - // seems faster than `addOperationWithBlock` - [self.coderQueue addBarrierBlock:doneBlock]; - } else { - // serial queue, this does the same effect in semantics - [self.coderQueue addOperationWithBlock:doneBlock]; - } + [self startCoderOperationWithImageData:imageData + pendingTokens:tokens + finishedTokens:@[]]; } } else { [self callCompletionBlocksWithError:[NSError errorWithDomain:SDWebImageErrorDomain code:SDWebImageErrorBadImageData userInfo:@{NSLocalizedDescriptionKey : @"Image data is nil"}]]; From 1ee0dbacea54cf8821edb5f1521e50b840f7e5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A9=AC=E9=81=A5?= Date: Fri, 28 Jul 2023 11:16:38 +0800 Subject: [PATCH 2/3] fix typo --- SDWebImage/Core/SDWebImageDownloaderOperation.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index d887d7f1..cc6b1120 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -342,19 +342,19 @@ // Check for unprocessed tokens. // if all tokens have been processed call [self done]. - (void)checkDoneWithImageData:(NSData *)imageData - finisdTokens:(NSArray *)finisdTokens { + finishedTokens:(NSArray *)finishedTokens { NSMutableArray *tokens; @synchronized (self) { tokens = [self.callbackTokens mutableCopy]; } - [finisdTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) { + [finishedTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) { [tokens removeObjectIdenticalTo:obj]; }]; if (tokens.count == 0) { [self done]; } else { // If there are new tokens added during the decoding operation, the decoding operation is supplemented with these new tokens. - [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finisdTokens]; + [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finishedTokens]; } } @@ -412,7 +412,7 @@ } // Check for new tokens added during the decode operation. [self checkDoneWithImageData:imageData - finisdTokens:[finishedTokens arrayByAddingObjectsFromArray:pendingTokens]]; + finishedTokens:[finishedTokens arrayByAddingObjectsFromArray:pendingTokens]]; }; if (@available(iOS 13, tvOS 13, macOS 10.15, watchOS 6, *)) { // seems faster than `addOperationWithBlock` From 2d73e96fdf762c2679d71356e6a767ff561bf37a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A9=AC=E9=81=A5?= Date: Fri, 28 Jul 2023 14:56:28 +0800 Subject: [PATCH 3/3] use @syncrhonized (self) in the whole checkDone method, no longer allows something that add new handlers during checkDone --- .../Core/SDWebImageDownloaderOperation.m | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/SDWebImage/Core/SDWebImageDownloaderOperation.m b/SDWebImage/Core/SDWebImageDownloaderOperation.m index cc6b1120..18a3e1c2 100644 --- a/SDWebImage/Core/SDWebImageDownloaderOperation.m +++ b/SDWebImage/Core/SDWebImageDownloaderOperation.m @@ -343,18 +343,17 @@ // if all tokens have been processed call [self done]. - (void)checkDoneWithImageData:(NSData *)imageData finishedTokens:(NSArray *)finishedTokens { - NSMutableArray *tokens; @synchronized (self) { - tokens = [self.callbackTokens mutableCopy]; - } - [finishedTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) { - [tokens removeObjectIdenticalTo:obj]; - }]; - if (tokens.count == 0) { - [self done]; - } else { - // If there are new tokens added during the decoding operation, the decoding operation is supplemented with these new tokens. - [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finishedTokens]; + NSMutableArray *tokens = [self.callbackTokens mutableCopy]; + [finishedTokens enumerateObjectsUsingBlock:^(SDWebImageDownloaderOperationToken * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) { + [tokens removeObjectIdenticalTo:obj]; + }]; + if (tokens.count == 0) { + [self done]; + } else { + // If there are new tokens added during the decoding operation, the decoding operation is supplemented with these new tokens. + [self startCoderOperationWithImageData:imageData pendingTokens:tokens finishedTokens:finishedTokens]; + } } }