From b831eff3160e165985a6327f9bea29392ab5ba59 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 25 Jan 2018 22:30:05 +0800 Subject: [PATCH 1/2] Refactor the implementation of SDWebImagePrefetcher. Now prefetcher do not cancel previous request and it separate different prefetching process. When you call prefetchURLs for different url lists, you can get callback for different completion block --- SDWebImage/SDWebImagePrefetcher.h | 50 +++--- SDWebImage/SDWebImagePrefetcher.m | 257 ++++++++++++++++++++---------- 2 files changed, 205 insertions(+), 102 deletions(-) diff --git a/SDWebImage/SDWebImagePrefetcher.h b/SDWebImage/SDWebImagePrefetcher.h index 93357f4e..a8eb583c 100644 --- a/SDWebImage/SDWebImagePrefetcher.h +++ b/SDWebImage/SDWebImagePrefetcher.h @@ -11,12 +11,18 @@ @class SDWebImagePrefetcher; +@interface SDWebImagePrefetchToken : NSObject + +@property (nonatomic, copy, readonly, nullable) NSArray *urls; + +@end + @protocol SDWebImagePrefetcherDelegate @optional /** - * Called when an image was prefetched. + * Called when an image was prefetched. Which means it's called when one URL from any of prefetching finished. * * @param imagePrefetcher The current image prefetcher * @param imageURL The image url that was prefetched @@ -26,7 +32,7 @@ - (void)imagePrefetcher:(nonnull SDWebImagePrefetcher *)imagePrefetcher didPrefetchURL:(nullable NSURL *)imageURL finishedCount:(NSUInteger)finishedCount totalCount:(NSUInteger)totalCount; /** - * Called when all images are prefetched. + * Called when all images are prefetched. Which means it's called when all URLs from all of prefetching finished. * @param imagePrefetcher The current image prefetcher * @param totalCount The total number of images that were prefetched (whether successful or not) * @param skippedCount The total number of images that were skipped @@ -54,15 +60,23 @@ typedef void(^SDWebImagePrefetcherCompletionBlock)(NSUInteger noOfFinishedUrls, @property (nonatomic, assign) NSUInteger maxConcurrentDownloads; /** - * SDWebImageOptions for prefetcher. Defaults to SDWebImageLowPriority. + * The options for prefetcher. Defaults to SDWebImageLowPriority. */ @property (nonatomic, assign) SDWebImageOptions options; /** - * Queue options for Prefetcher. Defaults to Main Queue. + * The context for prefetcher. Defaults to nil. + */ +@property (nonatomic, copy, nullable) SDWebImageContext *context; + +/** + * Queue options for prefetcher when call the progressBlock, completionBlock and delegate methods. Defaults to Main Queue. */ @property (strong, nonatomic, nonnull) dispatch_queue_t prefetcherQueue; +/** + * The delegate for the prefetcher. + */ @property (weak, nonatomic, nullable) id delegate; /** @@ -76,35 +90,35 @@ typedef void(^SDWebImagePrefetcherCompletionBlock)(NSUInteger noOfFinishedUrls, - (nonnull instancetype)initWithImageManager:(nonnull SDWebImageManager *)manager NS_DESIGNATED_INITIALIZER; /** - * Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching, - * currently one image is downloaded at a time, - * and skips images for failed downloads and proceed to the next image in the list. - * Any previously-running prefetch operations are canceled. + * Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching. It based on the image manager so the image may from the cache and network according to the `options` property. + * Prefetching is seperate to each other, which means the progressBlock and completionBlock you provide is bind to the prefetching for the list of urls. + * Attention that call this will not cancel previous fetched urls. You should keep the token return by this to cancel or cancel all the prefetch. * * @param urls list of URLs to prefetch + * @return the token to cancel the current prefetching. */ -- (void)prefetchURLs:(nullable NSArray *)urls; +- (nullable SDWebImagePrefetchToken *)prefetchURLs:(nullable NSArray *)urls; /** - * Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching, - * currently one image is downloaded at a time, - * and skips images for failed downloads and proceed to the next image in the list. - * Any previously-running prefetch operations are canceled. + * Assign list of URLs to let SDWebImagePrefetcher to queue the prefetching. It based on the image manager so the image may from the cache and network according to the `options` property. + * Prefetching is seperate to each other, which means the progressBlock and completionBlock you provide is bind to the prefetching for the list of urls. + * Attention that call this will not cancel previous fetched urls. You should keep the token return by this to cancel or cancel all the prefetch. * * @param urls list of URLs to prefetch * @param progressBlock block to be called when progress updates; * first parameter is the number of completed (successful or not) requests, * second parameter is the total number of images originally requested to be prefetched - * @param completionBlock block to be called when prefetching is completed + * @param completionBlock block to be called when the current prefetching is completed * first param is the number of completed (successful or not) requests, * second parameter is the number of skipped requests + * @return the token to cancel the current prefetching. */ -- (void)prefetchURLs:(nullable NSArray *)urls - progress:(nullable SDWebImagePrefetcherProgressBlock)progressBlock - completed:(nullable SDWebImagePrefetcherCompletionBlock)completionBlock; +- (nullable SDWebImagePrefetchToken *)prefetchURLs:(nullable NSArray *)urls + progress:(nullable SDWebImagePrefetcherProgressBlock)progressBlock + completed:(nullable SDWebImagePrefetcherCompletionBlock)completionBlock; /** - * Remove and cancel queued list + * Remove and cancel all the prefeching for the prefetcher. */ - (void)cancelPrefetching; diff --git a/SDWebImage/SDWebImagePrefetcher.m b/SDWebImage/SDWebImagePrefetcher.m index 18c433e7..d8133403 100644 --- a/SDWebImage/SDWebImagePrefetcher.m +++ b/SDWebImage/SDWebImagePrefetcher.m @@ -7,17 +7,27 @@ */ #import "SDWebImagePrefetcher.h" +#import + +@interface SDWebImagePrefetchToken () { + @public + int64_t _skippedCount; + int64_t _finishedCount; +} + +@property (nonatomic, copy, readwrite) NSArray *urls; +@property (nonatomic, assign) int64_t totalCount; +@property (nonatomic, strong) NSMutableArray> *operations; +@property (nonatomic, weak) SDWebImagePrefetcher *prefetcher; +@property (nonatomic, copy, nullable) SDWebImagePrefetcherCompletionBlock completionBlock; +@property (nonatomic, copy, nullable) SDWebImagePrefetcherProgressBlock progressBlock; + +@end @interface SDWebImagePrefetcher () @property (strong, nonatomic, nonnull) SDWebImageManager *manager; -@property (strong, atomic, nullable) NSArray *prefetchURLs; // may be accessed from different queue -@property (assign, nonatomic) NSUInteger requestedCount; -@property (assign, nonatomic) NSUInteger skippedCount; -@property (assign, nonatomic) NSUInteger finishedCount; -@property (assign, nonatomic) NSTimeInterval startedTime; -@property (copy, nonatomic, nullable) SDWebImagePrefetcherCompletionBlock completionBlock; -@property (copy, nonatomic, nullable) SDWebImagePrefetcherProgressBlock progressBlock; +@property (strong, atomic, nonnull) NSMutableSet *runningTokens; @end @@ -39,6 +49,7 @@ - (nonnull instancetype)initWithImageManager:(SDWebImageManager *)manager { if ((self = [super init])) { _manager = manager; + _runningTokens = [NSMutableSet set]; _options = SDWebImageLowPriority; _prefetcherQueue = dispatch_get_main_queue(); self.maxConcurrentDownloads = 3; @@ -54,91 +65,169 @@ return self.manager.imageDownloader.maxConcurrentDownloads; } -- (void)startPrefetchingAtIndex:(NSUInteger)index { - NSURL *currentURL; - @synchronized(self) { - if (index >= self.prefetchURLs.count) return; - currentURL = self.prefetchURLs[index]; - self.requestedCount++; - } - [self.manager loadImageWithURL:currentURL options:self.options progress:nil completed:^(UIImage *image, NSData *data, NSError *error, SDImageCacheType cacheType, BOOL finished, NSURL *imageURL) { - if (!finished) return; - self.finishedCount++; - - if (self.progressBlock) { - self.progressBlock(self.finishedCount,(self.prefetchURLs).count); - } - if (!image) { - // Add last failed - self.skippedCount++; - } - if ([self.delegate respondsToSelector:@selector(imagePrefetcher:didPrefetchURL:finishedCount:totalCount:)]) { - [self.delegate imagePrefetcher:self - didPrefetchURL:currentURL - finishedCount:self.finishedCount - totalCount:self.prefetchURLs.count - ]; - } - if (self.prefetchURLs.count > self.requestedCount) { - dispatch_async(self.prefetcherQueue, ^{ - // we need dispatch to avoid function recursion call. This can prevent stack overflow even for huge urls list - [self startPrefetchingAtIndex:self.requestedCount]; - }); - } else if (self.finishedCount == self.requestedCount) { - [self reportStatus]; - if (self.completionBlock) { - self.completionBlock(self.finishedCount, self.skippedCount); - self.completionBlock = nil; - } - self.progressBlock = nil; - } - }]; +#pragma mark - Prefetch +- (nullable SDWebImagePrefetchToken *)prefetchURLs:(nullable NSArray *)urls { + return [self prefetchURLs:urls progress:nil completed:nil]; } -- (void)reportStatus { - NSUInteger total = (self.prefetchURLs).count; - if ([self.delegate respondsToSelector:@selector(imagePrefetcher:didFinishWithTotalCount:skippedCount:)]) { - [self.delegate imagePrefetcher:self - didFinishWithTotalCount:(total - self.skippedCount) - skippedCount:self.skippedCount - ]; - } -} - -- (void)prefetchURLs:(nullable NSArray *)urls { - [self prefetchURLs:urls progress:nil completed:nil]; -} - -- (void)prefetchURLs:(nullable NSArray *)urls - progress:(nullable SDWebImagePrefetcherProgressBlock)progressBlock - completed:(nullable SDWebImagePrefetcherCompletionBlock)completionBlock { - [self cancelPrefetching]; // Prevent duplicate prefetch request - self.startedTime = CFAbsoluteTimeGetCurrent(); - self.prefetchURLs = urls; - self.completionBlock = completionBlock; - self.progressBlock = progressBlock; - - if (urls.count == 0) { +- (nullable SDWebImagePrefetchToken *)prefetchURLs:(nullable NSArray *)urls + progress:(nullable SDWebImagePrefetcherProgressBlock)progressBlock + completed:(nullable SDWebImagePrefetcherCompletionBlock)completionBlock { + if (!urls || urls.count == 0) { if (completionBlock) { - completionBlock(0,0); - } - } else { - // Starts prefetching from the very first image on the list with the max allowed concurrency - NSUInteger listCount = self.prefetchURLs.count; - for (NSUInteger i = 0; i < self.maxConcurrentDownloads && self.requestedCount < listCount; i++) { - [self startPrefetchingAtIndex:i]; + completionBlock(0, 0); } + return nil; + } + SDWebImagePrefetchToken *token = [SDWebImagePrefetchToken new]; + token.prefetcher = self; + token->_skippedCount = 0; + token->_finishedCount = 0; + token.urls = urls; + token.totalCount = urls.count; + token.operations = [NSMutableArray arrayWithCapacity:urls.count]; + token.progressBlock = progressBlock; + token.completionBlock = completionBlock; + [self addRunningToken:token]; + + for (NSURL *url in urls) { + __weak typeof(self) wself = self; + id operation = [self.manager loadImageWithURL:url options:self.options progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, SDImageCacheType cacheType, BOOL finished, NSURL * _Nullable imageURL) { + __strong typeof(wself) sself = wself; + if (!sself) { + return; + } + if (!finished) { + return; + } + OSAtomicIncrement64(&(token->_finishedCount)); + if (error) { + // Add last failed + OSAtomicIncrement64(&(token->_skippedCount)); + } + + // Current operation finished + [sself callProgressBlockForToken:token imageURL:imageURL]; + + if (token->_finishedCount + token->_skippedCount == token.totalCount) { + // All finished + [sself callCompletionBlockForToken:token]; + [sself removeRunningToken:token]; + } + } context:self.context]; + [token.operations addObject:operation]; + } + + return token; +} + +#pragma mark - Cancel +- (void)cancelPrefetching { + @synchronized(self.runningTokens) { + NSSet *copiedTokens = [self.runningTokens copy]; + [copiedTokens makeObjectsPerformSelector:@selector(cancel)]; + [self.runningTokens removeAllObjects]; } } -- (void)cancelPrefetching { - @synchronized(self) { - self.prefetchURLs = nil; - self.skippedCount = 0; - self.requestedCount = 0; - self.finishedCount = 0; +- (void)callProgressBlockForToken:(SDWebImagePrefetchToken *)token imageURL:(NSURL *)url { + if (!token) { + return; } - [self.manager cancelAll]; + BOOL shouldCallDelegate = [self.delegate respondsToSelector:@selector(imagePrefetcher:didPrefetchURL:finishedCount:totalCount:)]; + dispatch_queue_async_safe(self.prefetcherQueue, ^{ + if (shouldCallDelegate) { + [self.delegate imagePrefetcher:self didPrefetchURL:url finishedCount:[self tokenFinishedCount] totalCount:[self tokenTotalCount]]; + } + if (token.progressBlock) { + token.progressBlock((NSUInteger)token->_finishedCount, (NSUInteger)token.totalCount); + } + }); +} + +- (void)callCompletionBlockForToken:(SDWebImagePrefetchToken *)token { + if (!token) { + return; + } + BOOL shoulCallDelegate = [self.delegate respondsToSelector:@selector(imagePrefetcher:didFinishWithTotalCount:skippedCount:)] && ([self countOfRunningTokens] == 1); // last one + dispatch_queue_async_safe(self.prefetcherQueue, ^{ + if (shoulCallDelegate) { + [self.delegate imagePrefetcher:self didFinishWithTotalCount:[self tokenTotalCount] skippedCount:[self tokenSkippedCount]]; + } + if (token.completionBlock) { + token.completionBlock((NSUInteger)token->_finishedCount, (NSUInteger)token->_skippedCount); + } + }); +} + +#pragma mark - Helper +- (NSUInteger)tokenTotalCount { + NSUInteger tokenTotalCount = 0; + @synchronized (self.runningTokens) { + for (SDWebImagePrefetchToken *token in self.runningTokens) { + tokenTotalCount += token.totalCount; + } + } + return tokenTotalCount; +} + +- (NSUInteger)tokenSkippedCount { + NSUInteger tokenSkippedCount = 0; + @synchronized (self.runningTokens) { + for (SDWebImagePrefetchToken *token in self.runningTokens) { + tokenSkippedCount += token->_skippedCount; + } + } + return tokenSkippedCount; +} + +- (NSUInteger)tokenFinishedCount { + NSUInteger tokenFinishedCount = 0; + @synchronized (self.runningTokens) { + for (SDWebImagePrefetchToken *token in self.runningTokens) { + tokenFinishedCount += token->_finishedCount; + } + } + return tokenFinishedCount; +} + +- (void)addRunningToken:(SDWebImagePrefetchToken *)token { + if (!token) { + return; + } + @synchronized (self.runningTokens) { + [self.runningTokens addObject:token]; + } +} + +- (void)removeRunningToken:(SDWebImagePrefetchToken *)token { + if (!token) { + return; + } + @synchronized (self.runningTokens) { + [self.runningTokens removeObject:token]; + } +} + +- (NSUInteger)countOfRunningTokens { + NSUInteger count = 0; + @synchronized (self.runningTokens) { + count = self.runningTokens.count; + } + return count; +} + +@end + +@implementation SDWebImagePrefetchToken + +- (void)cancel { + @synchronized (self) { + for (id operation in self.operations) { + [operation cancel]; + } + } + [self.prefetcher removeRunningToken:self]; } @end From 69bc9cbd2c9d157d10e8d6bf3ee4ba745cf17c15 Mon Sep 17 00:00:00 2001 From: DreamPiggy Date: Thu, 25 Jan 2018 23:50:40 +0800 Subject: [PATCH 2/2] Update the prefetcher test to ensure that prefetch different urls works and the delegate methods work --- Tests/Tests/SDWebImagePrefetcherTests.m | 85 ++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/Tests/Tests/SDWebImagePrefetcherTests.m b/Tests/Tests/SDWebImagePrefetcherTests.m index 4079547a..0a2bc795 100644 --- a/Tests/Tests/SDWebImagePrefetcherTests.m +++ b/Tests/Tests/SDWebImagePrefetcherTests.m @@ -10,7 +10,12 @@ #import "SDTestCase.h" #import -@interface SDWebImagePrefetcherTests : SDTestCase +@interface SDWebImagePrefetcherTests : SDTestCase + +@property (nonatomic, strong) SDWebImagePrefetcher *prefetcher; +@property (nonatomic, assign) NSUInteger finishedCount; +@property (nonatomic, assign) NSUInteger skippedCount; +@property (nonatomic, assign) NSUInteger totalCount; @end @@ -59,6 +64,82 @@ [self waitForExpectationsWithCommonTimeout]; } -// TODO: test the prefetcher delegate works +- (void)test04PrefetchWithMultipleArrayDifferentQueueWorks { + XCTestExpectation *expectation = [self expectationWithDescription:@"Prefetch with multiple array at different queue failed"]; + + NSArray *imageURLs1 = @[@"http://via.placeholder.com/20x20.jpg", + @"http://via.placeholder.com/30x30.jpg"]; + NSArray *imageURLs2 = @[@"http://via.placeholder.com/30x30.jpg", + @"http://via.placeholder.com/40x40.jpg"]; + dispatch_queue_t queue1 = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0); + dispatch_queue_t queue2 = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0); + __block int numberOfPrefetched1 = 0; + __block int numberOfPrefetched2 = 0; + __block BOOL prefetchFinished1 = NO; + __block BOOL prefetchFinished2 = NO; + + // Clear the disk cache to make it more realistic for multi-thread environment + [[SDImageCache sharedImageCache] clearDiskOnCompletion:^{ + dispatch_async(queue1, ^{ + [[SDWebImagePrefetcher sharedImagePrefetcher] prefetchURLs:imageURLs1 progress:^(NSUInteger noOfFinishedUrls, NSUInteger noOfTotalUrls) { + numberOfPrefetched1 += 1; + } completed:^(NSUInteger noOfFinishedUrls, NSUInteger noOfSkippedUrls) { + expect(numberOfPrefetched1).to.equal(noOfFinishedUrls); + prefetchFinished1 = YES; + // both completion called + if (prefetchFinished1 && prefetchFinished2) { + [expectation fulfill]; + } + }]; + }); + dispatch_async(queue2, ^{ + [[SDWebImagePrefetcher sharedImagePrefetcher] prefetchURLs:imageURLs2 progress:^(NSUInteger noOfFinishedUrls, NSUInteger noOfTotalUrls) { + numberOfPrefetched2 += 1; + } completed:^(NSUInteger noOfFinishedUrls, NSUInteger noOfSkippedUrls) { + expect(numberOfPrefetched2).to.equal(noOfFinishedUrls); + prefetchFinished2 = YES; + // both completion called + if (prefetchFinished1 && prefetchFinished2) { + [expectation fulfill]; + } + }]; + }); + }]; + + [self waitForExpectationsWithCommonTimeout]; +} + +- (void)test05PrefecherDelegateWorks { + XCTestExpectation *expectation = [self expectationWithDescription:@"Prefetcher delegate failed"]; + + NSArray *imageURLs = @[@"http://via.placeholder.com/20x20.jpg", + @"http://via.placeholder.com/30x30.jpg", + @"http://via.placeholder.com/40x40.jpg"]; + self.prefetcher = [SDWebImagePrefetcher new]; + self.prefetcher.delegate = self; + // Current implementation, the delegate method called before the progressBlock and completionBlock + [self.prefetcher prefetchURLs:imageURLs progress:^(NSUInteger noOfFinishedUrls, NSUInteger noOfTotalUrls) { + expect(self.finishedCount).to.equal(noOfFinishedUrls); + expect(self.totalCount).to.equal(noOfTotalUrls); + } completed:^(NSUInteger noOfFinishedUrls, NSUInteger noOfSkippedUrls) { + expect(self.finishedCount).to.equal(noOfFinishedUrls); + expect(self.skippedCount).to.equal(noOfSkippedUrls); + [expectation fulfill]; + }]; + + [self waitForExpectationsWithCommonTimeout]; +} + +- (void)imagePrefetcher:(SDWebImagePrefetcher *)imagePrefetcher didFinishWithTotalCount:(NSUInteger)totalCount skippedCount:(NSUInteger)skippedCount { + expect(imagePrefetcher).to.equal(self.prefetcher); + self.skippedCount = skippedCount; + self.totalCount = totalCount; +} + +- (void)imagePrefetcher:(SDWebImagePrefetcher *)imagePrefetcher didPrefetchURL:(NSURL *)imageURL finishedCount:(NSUInteger)finishedCount totalCount:(NSUInteger)totalCount { + expect(imagePrefetcher).to.equal(self.prefetcher); + self.finishedCount = finishedCount; + self.totalCount = totalCount; +} @end