Merge pull request #3182 from dreampiggy/performance_progressive_reuse_when_download_finished

Increase progressive decoding performance by using the progressive decoder's result instead of re-decoding the full image data

SDWebImage/Core/SDImageLoader.h

@@ -9,6 +9,7 @@
 #import "SDWebImageCompat.h"
 #import "SDWebImageDefine.h"
 #import "SDWebImageOperation.h"
+#import "SDImageCoder.h"
 
 typedef void(^SDImageLoaderProgressBlock)(NSInteger receivedSize, NSInteger expectedSize, NSURL * _Nullable targetURL);
 typedef void(^SDImageLoaderCompletedBlock)(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished);
@@ -50,6 +51,18 @@ FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeImageData(NSData * _Non
  */
 FOUNDATION_EXPORT UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull imageData, NSURL * _Nonnull imageURL, BOOL finished,  id<SDWebImageOperation> _Nonnull operation, SDWebImageOptions options, SDWebImageContext * _Nullable context);
 
+/**
+ This function get the progressive decoder for current loading operation. If no progressive decoding is happended or decoder is not able to construct, return nil.
+ @return The progressive decoder associated with the loading operation.
+ */
+FOUNDATION_EXPORT id<SDProgressiveImageCoder> _Nullable SDImageLoaderGetProgressiveCoder(id<SDWebImageOperation> _Nonnull operation);
+
+/**
+ This function set the progressive decoder for current loading operation. If no progressive decoding is happended, pass nil.
+ @param operation The loading operation to associate the progerssive decoder.
+ */
+FOUNDATION_EXPORT void SDImageLoaderSetProgressiveCoder(id<SDWebImageOperation> _Nonnull operation, id<SDProgressiveImageCoder> _Nullable progressiveCoder);
+
 #pragma mark - SDImageLoader
 
 /**

SDWebImage/Core/SDImageLoader.m

@@ -15,8 +15,20 @@
 #import "SDInternalMacros.h"
 #import "objc/runtime.h"
 
+SDWebImageContextOption const SDWebImageContextLoaderCachedImage = @"loaderCachedImage";
+
 static void * SDImageLoaderProgressiveCoderKey = &SDImageLoaderProgressiveCoderKey;
 
+id<SDProgressiveImageCoder> SDImageLoaderGetProgressiveCoder(id<SDWebImageOperation> operation) {
+    NSCParameterAssert(operation);
+    return objc_getAssociatedObject(operation, SDImageLoaderProgressiveCoderKey);
+}
+
+void SDImageLoaderSetProgressiveCoder(id<SDWebImageOperation> operation, id<SDProgressiveImageCoder> progressiveCoder) {
+    NSCParameterAssert(operation);
+    objc_setAssociatedObject(operation, SDImageLoaderProgressiveCoderKey, progressiveCoder, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+}
+
 UIImage * _Nullable SDImageLoaderDecodeImageData(NSData * _Nonnull imageData, NSURL * _Nonnull imageURL, SDWebImageOptions options, SDWebImageContext * _Nullable context) {
     NSCParameterAssert(imageData);
     NSCParameterAssert(imageURL);
@@ -136,7 +148,7 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im
     SDImageCoderOptions *coderOptions = [mutableCoderOptions copy];
     
     // Grab the progressive image coder
-    id<SDProgressiveImageCoder> progressiveCoder = objc_getAssociatedObject(operation, SDImageLoaderProgressiveCoderKey);
+    id<SDProgressiveImageCoder> progressiveCoder = SDImageLoaderGetProgressiveCoder(operation);
     if (!progressiveCoder) {
         id<SDProgressiveImageCoder> imageCoder = context[SDWebImageContextImageCoder];
         // Check the progressive coder if provided
@@ -152,7 +164,7 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im
                 }
             }
         }
-        objc_setAssociatedObject(operation, SDImageLoaderProgressiveCoderKey, progressiveCoder, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+        SDImageLoaderSetProgressiveCoder(operation, progressiveCoder);
     }
     // If we can't find any progressive coder, disable progressive download
     if (!progressiveCoder) {
@@ -190,11 +202,9 @@ UIImage * _Nullable SDImageLoaderDecodeProgressiveImageData(NSData * _Nonnull im
         if (shouldDecode) {
             image = [SDImageCoderHelper decodedImageWithImage:image];
         }
-        // mark the image as progressive (completionBlock one are not mark as progressive)
+        // mark the image as progressive (completed one are not mark as progressive)
-        image.sd_isIncremental = YES;
+        image.sd_isIncremental = !finished;
     }
     
     return image;
 }
-
-SDWebImageContextOption const SDWebImageContextLoaderCachedImage = @"loaderCachedImage";

SDWebImage/Core/SDWebImageDownloaderOperation.m

@@ -11,6 +11,7 @@
 #import "SDInternalMacros.h"
 #import "SDWebImageDownloaderResponseModifier.h"
 #import "SDWebImageDownloaderDecryptor.h"
+#import "SDAnimatedImage.h"
 
 static NSString *const kProgressCallbackKey = @"progress";
 static NSString *const kCompletedCallbackKey = @"completed";
@@ -386,7 +387,8 @@ didReceiveResponse:(NSURLResponse *)response
     
     // Using data decryptor will disable the progressive decoding, since there are no support for progressive decrypt
     BOOL supportProgressive = (self.options & SDWebImageDownloaderProgressiveLoad) && !self.decryptor;
-    if (supportProgressive) {
+    // Progressive decoding Only decode partial image, full image in `URLSession:task:didCompleteWithError:`
+    if (supportProgressive && !finished) {
         // Get the image data
         NSData *imageData = [self.imageData copy];
         
@@ -394,7 +396,7 @@ didReceiveResponse:(NSURLResponse *)response
         if (self.coderQueue.operationCount == 0) {
             // NSOperation have autoreleasepool, don't need to create extra one
             [self.coderQueue addOperationWithBlock:^{
-                UIImage *image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, finished, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
+                UIImage *image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, NO, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
                 if (image) {
                     // We do not keep the progressive decoding image even when `finished`=YES. Because they are for view rendering but not take full function from downloader options. And some coders implementation may not keep consistent between progressive decoding and normal decoding.
                     
@@ -471,7 +473,14 @@ didReceiveResponse:(NSURLResponse *)response
                     // decode the image in coder queue, cancel all previous decoding process
                     [self.coderQueue cancelAllOperations];
                     [self.coderQueue addOperationWithBlock:^{
-                        UIImage *image = SDImageLoaderDecodeImageData(imageData, self.request.URL, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
+                        // check if we already use progressive decoding, use that to produce faster decoding
+                        id<SDProgressiveImageCoder> progressiveCoder = SDImageLoaderGetProgressiveCoder(self);
+                        UIImage *image;
+                        if (progressiveCoder) {
+                            image = SDImageLoaderDecodeProgressiveImageData(imageData, self.request.URL, YES, self, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
+                        } else {
+                            image = SDImageLoaderDecodeImageData(imageData, self.request.URL, [[self class] imageOptionsFromDownloaderOptions:self.options], self.context);
+                        }
                         CGSize imageSize = image.size;
                         if (imageSize.width == 0 || imageSize.height == 0) {
                             NSString *description = image == nil ? @"Downloaded image decode failed" : @"Downloaded image has 0 pixels";

Tests/Tests/SDWebImageDownloaderTests.m

@@ -707,6 +707,32 @@
     [self waitForExpectationsWithCommonTimeout];
 }
 
+- (void)test28ProgressiveDownloadShouldUseSameCoder  {
+    XCTestExpectation *expectation = [self expectationWithDescription:@"Progressive download should use the same coder for each animated image"];
+    SDWebImageDownloader *downloader = [[SDWebImageDownloader alloc] init];
+    
+    __block SDWebImageDownloadToken *token;
+    __block id<SDImageCoder> progressiveCoder;
+    token = [downloader downloadImageWithURL:[NSURL URLWithString:kTestGIFURL] options:SDWebImageDownloaderProgressiveLoad context:@{SDWebImageContextAnimatedImageClass : SDAnimatedImage.class} progress:nil completed:^(UIImage * _Nullable image, NSData * _Nullable data, NSError * _Nullable error, BOOL finished) {
+        expect(error).beNil();
+        expect([image isKindOfClass:SDAnimatedImage.class]).beTruthy();
+        id<SDImageCoder> coder = ((SDAnimatedImage *)image).animatedCoder;
+        if (!progressiveCoder) {
+            progressiveCoder = coder;
+        }
+        expect(progressiveCoder).equal(coder);
+        if (!finished) {
+            progressiveCoder = coder;
+        } else {
+            [expectation fulfill];
+        }
+    }];
+    
+    [self waitForExpectationsWithCommonTimeoutUsingHandler:^(NSError * _Nullable error) {
+        [downloader invalidateSessionAndCancel:YES];
+    }];
+}
+
 #pragma mark - SDWebImageLoader
 - (void)test30CustomImageLoaderWorks {
     XCTestExpectation *expectation = [self expectationWithDescription:@"Custom image not works"];

Tests/Tests/SDWebImageManagerTests.m

@@ -269,6 +269,7 @@
         expect(image).notTo.beNil();
         expect(image.size).equal(CGSizeMake(1000, 1000));
         if (finished) {
+            expect(image.sd_isIncremental).beFalsy();
             [expectation fulfill];
         } else {
             expect(image.sd_isIncremental).beTruthy();
@@ -290,6 +291,7 @@
         expect(image.imageOrientation).equal(orientation);
 #endif
         if (finished) {
+            expect(image.sd_isIncremental).beFalsy();
             [expectation fulfill];
         } else {
             expect(image.sd_isIncremental).beTruthy();