Replace the NSOperation based downloader by a simple async NSURLConne…

…ction (read-on to understand why) I finally found the reason behind the download not started while UITableView is manipulated: the default NSURLConnection runloop mode. Its default mode is NSEventTrackingRunLoopMode which is interrupted by UI events. Changing default NSURLConnection runloop mode to NSRunLoopCommonModes just fix this good old responsiveness issue. I thus decided to replace the current NSOperation based implementation by this finding, as NSOperation is far more expensive than simple async connections. Additionally, moving aways from NSOperation here fix an odd lagging issue with iOS 4, an issue I can't explain at the moment. Note that `SDWebImageDownloader`'s `setMaxConcurrentDownloads:` method is now a no-op as I didn't implemented the NSOperation queuing system with async connections. I don't think it still necessary as thread-less async connectaions are very lightweight. If you think there is a real need of this, I may reconsider and implement it in the future. In the meantime, this method does nothing and its usage is declared as deprecated.
siuying · Jun 9, 2010 · e0e3696 · e0e3696
1 parent 467be16
commit e0e3696
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 94 deletions.
diff --git a/README.md b/README.md
@@ -6,7 +6,7 @@ This library provides a category for UIImageVIew with support for remote images
 It provides:
 
 - An UIImageView category adding web image and cache management to the Cocoa Touch framework
-- An asynchronous image downloader using threads (NSOperation)
+- An asynchronous image downloader
 - An asynchronous memory + disk image caching with automatic cache expiration handling
 - A garantie that the same URL won't be downloaded several times
 - A garantie that bogus URLs won't be retried again and again
@@ -39,23 +39,21 @@ time faster than my own servers... WTF??
 
 In fact, my servers were well but a lot of latency was added to the requests, certainly because my
 application wasn't responsive enough to handle the requests at full speed. At this moment, I
-understood something important, asynchronous NSURLConnections are tied to the main runloop (I
-guess). It's certainly based on the poll multiplexer system call, which allows a single thread to
-handle quite a huge number of simultaneous connections. It works well while nothing blocks in the
-loop, but in this loop, there is also the events handling. A simple test to recognize an application
-using NSURLConnection to load there remote images is to scroll the UITableView with your finger to
-disclose an unloaded image, and to keep your finger pressed on the screen. If the image doesn't load
-until you release you finger, the application is certainly using NSURLConnection (try with the
-Facebook app for instance). I'm not completely clear about the reason of this blocking, I thought
-the iPhone was running a dedicated run-loop for connections, but the fact is, NSURLConnection is
-affected by the application events and/or UI handling (or something else I'm not aware of).
-
-Thus I explored another path and found this marvelous NSOperation class to handle concurrency with
-love. I ran some quick tests with this tool and I instantly got enhanced responsiveness of the image
-loading in my UITableView by... a lot. Thus I rewrote the [Fraggle][]'s implementation using the
-same concept of drop-in remplacement for UIImageView but with this new technic. I thought the result
-could benefits to a lot of other applications, thus we decided, with [Fraggle][], to open-sourced
-it, et voila!
+understood something important, asynchronous NSURLConnections are tied to the main runloop in the
+NSEventTrackingRunLoopMode. As explained in the documentation, this runloop mode is affected by
+UI events:
+
+> Cocoa uses this mode to restrict incoming events during mouse-dragging loops and other sorts of
+> user interface tracking loops.
+
+A simple test to recognize an application using NSURLConnection in its default mode to load there
+remote images is to scroll the UITableView with your finger to disclose an unloaded image, and to
+keep your finger pressed on the screen. If the image doesn't load until you release you finger,
+you've got one (try with the Facebook app for instance). It took me quite some time to understand
+the reason for this lagging issue. Actually I first used NSOperation to workaround this issue.
+
+This technic combined with an image cache instantly gave a lot of responsiveness to my app.
+I thought this lib could benefits to a lot of other Cocoa Touch application so I open-sourced it.
 
 How To Use It
 -------------
@@ -64,7 +62,7 @@ How To Use It
 
 Just #import the UIImageView+WebCache.h header, and call the setImageWithURL:placeholderImage:
 method from the tableView:cellForRowAtIndexPath: UITableViewDataSource method. Everything will be
-handled for you, from parallel downloads to caching management.
+handled for you, from async downloads to caching management.
 
     #import "UIImageView+WebCache.h"
 
@@ -122,14 +120,13 @@ imageHelper:didFinishWithImage: method from this protocol:
 
 ### Using Asynchronous Image Downloader Independently
 
-It is possible to use the NSOperation based image downloader independently. Just create an instance
-of SDWebImageDownloader using its convenience constructor downloaderWithURL:target:action:.
+It is possible to use the async image downloader independently. You just have to create an instance
+of SDWebImageDownloader using its convenience constructor downloaderWithURL:delegate:.
 
     downloader = [SDWebImageDownloader downloaderWithURL:url delegate:self];
 
-The download will by queued immediately and the imageDownloader:didFinishWithImage: method from the
-SDWebImageDownloaderDelegate protocol will be called as soon as the download of image is completed
-(prepare not to be called from the main thread).
+The download will start immediately and the imageDownloader:didFinishWithImage: method from the
+SDWebImageDownloaderDelegate protocol will be called as soon as the download of image is completed.
 
 ### Using Asynchronous Image Caching Independently
 
@@ -168,4 +165,4 @@ Future Enhancements
 [Fraggle]: http://fraggle.squarespace.com
 [Urban Rivals]: http://fraggle.squarespace.com/blog/2009/9/15/almost-done-here-is-urban-rivals-iphone-trailer.html
 [Three20]: http://groups.google.com/group/three20
-[Joe Hewitt]: http://www.joehewitt.com
+[Joe Hewitt]: http://www.joehewitt.com
diff --git a/SDWebImageDownloader.h b/SDWebImageDownloader.h
@@ -9,16 +9,23 @@
 #import <Foundation/Foundation.h>
 #import "SDWebImageDownloaderDelegate.h"
 
-@interface SDWebImageDownloader : NSOperation
+@interface SDWebImageDownloader : NSObject
 {
+    @private
     NSURL *url;
     id<SDWebImageDownloaderDelegate> delegate;
+    NSURLConnection *connection;
+    NSMutableData *imageData;
 }
 
-@property (retain) NSURL *url;
-@property (assign) id<SDWebImageDownloaderDelegate> delegate;
+@property (nonatomic, retain) NSURL *url;
+@property (nonatomic, assign) id<SDWebImageDownloaderDelegate> delegate;
 
 + (id)downloaderWithURL:(NSURL *)url delegate:(id<SDWebImageDownloaderDelegate>)delegate;
-+ (void)setMaxConcurrentDownloads:(NSUInteger)max;
+- (void)start;
+- (void)cancel;
+
+// This method is now no-op and is deprecated
++ (void)setMaxConcurrentDownloads:(NSUInteger)max __attribute__((deprecated));
 
 @end
diff --git a/SDWebImageDownloader.m b/SDWebImageDownloader.m
@@ -8,59 +8,103 @@
 
 #import "SDWebImageDownloader.h"
 
-static NSOperationQueue *downloadQueue;
+@interface SDWebImageDownloader ()
+@property (nonatomic, retain) NSURLConnection *connection;
+@property (nonatomic, retain) NSMutableData *imageData;
+@end
 
 @implementation SDWebImageDownloader
+@synthesize url, delegate, connection, imageData;
 
-@synthesize url, delegate;
-
-- (void)dealloc
-{
-    [url release], url = nil;
-    [super dealloc];
-}
+#pragma mark Public Methods
 
 + (id)downloaderWithURL:(NSURL *)url delegate:(id<SDWebImageDownloaderDelegate>)delegate
 {
     SDWebImageDownloader *downloader = [[[SDWebImageDownloader alloc] init] autorelease];
     downloader.url = url;
     downloader.delegate = delegate;
-
-    if (downloadQueue == nil)
-    {
-        downloadQueue = [[NSOperationQueue alloc] init];
-        downloadQueue.maxConcurrentOperationCount = 8;
-    }
-
-    [downloadQueue addOperation:downloader];
-
+    [downloader start];
     return downloader;
 }
 
 + (void)setMaxConcurrentDownloads:(NSUInteger)max
 {
-    if (downloadQueue == nil)
+    // NOOP
+}
+
+- (void)start
+{
+    // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests
+    NSURLRequest *request = [[NSURLRequest alloc] initWithURL:url cachePolicy:NSURLRequestReloadIgnoringLocalCacheData timeoutInterval:15];
+    self.connection = [[[NSURLConnection alloc] initWithRequest:request delegate:self startImmediately:NO] autorelease];
+    // Ensure we aren't blocked by UI manipulations (default runloop mode for NSURLConnection is NSEventTrackingRunLoopMode)
+    [connection scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:NSRunLoopCommonModes];
+    [connection start];
+    [request release];
+
+    if (connection)
+    {
+        self.imageData = [NSMutableData data];
+    }
+    else
     {
-        downloadQueue = [[NSOperationQueue alloc] init];
+        if ([delegate respondsToSelector:@selector(imageDownloader:didFailWithError:)])
+        {
+            [delegate performSelector:@selector(imageDownloader:didFailWithError:) withObject:self withObject:nil];
+        }
     }
+}
 
-    downloadQueue.maxConcurrentOperationCount = max;
+- (void)cancel
+{
+    if (connection)
+    {
+        [connection cancel];
+        self.connection = nil;
+    }
 }
 
-- (void)main
+#pragma mark NSURLConnection (delegate)
+
+- (void)connection:(NSURLConnection *)aConnection didReceiveData:(NSData *)data
 {
-    NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
+    [imageData appendData:data];
+}
 
-    // In order to prevent from potential duplicate caching (NSURLCache + SDImageCache) we disable the cache for image requests
-    NSURLRequest *request = [NSURLRequest requestWithURL:url cachePolicy:NSURLRequestReloadIgnoringLocalCacheData timeoutInterval:5];
-    UIImage *image = [UIImage imageWithData:[NSURLConnection sendSynchronousRequest:request returningResponse:nil error:NULL]];
+- (void)connectionDidFinishLoading:(NSURLConnection *)aConnection
+{
+    UIImage *image = [[UIImage alloc] initWithData:imageData];
+    self.imageData = nil;
+    self.connection = nil;
 
-    if (!self.isCancelled && [delegate respondsToSelector:@selector(imageDownloader:didFinishWithImage:)])
+    if ([delegate respondsToSelector:@selector(imageDownloader:didFinishWithImage:)])
     {
         [delegate performSelector:@selector(imageDownloader:didFinishWithImage:) withObject:self withObject:image];
     }
 
-    [pool release];
+    [image release];
+}
+
+- (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)error
+{
+    if ([delegate respondsToSelector:@selector(imageDownloader:didFailWithError:)])
+    {
+        [delegate performSelector:@selector(imageDownloader:didFailWithError:) withObject:self withObject:error];
+    }
+
+    self.connection = nil;
+    self.imageData = nil;
+}
+
+#pragma mark NSObject
+
+- (void)dealloc
+{
+    [url release], url = nil;
+    [connection release], connection = nil;
+    [imageData release], imageData = nil;
+    [super dealloc];
 }
 
+
 @end
diff --git a/SDWebImageDownloaderDelegate.h b/SDWebImageDownloaderDelegate.h
@@ -13,5 +13,6 @@
 @optional
 
 - (void)imageDownloader:(SDWebImageDownloader *)downloader didFinishWithImage:(UIImage *)image;
+- (void)imageDownloader:(SDWebImageDownloader *)downloader didFailWithError:(NSError *)error;
 
-@end
+@end
diff --git a/SDWebImageManager.m b/SDWebImageManager.m
@@ -67,62 +67,53 @@ - (void)downloadWithURL:(NSURL *)url delegate:(id<SDWebImageManagerDelegate>)del
         [downloaderForURL setObject:downloader forKey:url];
     }
 
-    @synchronized(self)
-    {
-        [delegates addObject:delegate];
-        [downloaders addObject:downloader];
-    }
+    [delegates addObject:delegate];
+    [downloaders addObject:downloader];
 }
 
 - (void)cancelForDelegate:(id<SDWebImageManagerDelegate>)delegate
 {
-    @synchronized(self)
-    {
-        NSUInteger idx = [delegates indexOfObjectIdenticalTo:delegate];
+    NSUInteger idx = [delegates indexOfObjectIdenticalTo:delegate];
 
-        if (idx == NSNotFound)
-        {
-            return;
-        }
-
-        SDWebImageDownloader *downloader = [[downloaders objectAtIndex:idx] retain];
+    if (idx == NSNotFound)
+    {
+        return;
+    }
 
-        [delegates removeObjectAtIndex:idx];
-        [downloaders removeObjectAtIndex:idx];
+    SDWebImageDownloader *downloader = [[downloaders objectAtIndex:idx] retain];
 
-        if (![downloaders containsObject:downloader])
-        {
-            // No more delegate are waiting for this download, cancel it
-            [downloader cancel];
-            [downloaderForURL removeObjectForKey:downloader.url];
-        }
+    [delegates removeObjectAtIndex:idx];
+    [downloaders removeObjectAtIndex:idx];
 
-        [downloader release];
+    if (![downloaders containsObject:downloader])
+    {
+        // No more delegate are waiting for this download, cancel it
+        [downloader cancel];
+        [downloaderForURL removeObjectForKey:downloader.url];
     }
+
+    [downloader release];
 }
 
 - (void)imageDownloader:(SDWebImageDownloader *)downloader didFinishWithImage:(UIImage *)image
 {
     [downloader retain];
 
-    @synchronized(self)
+    // Notify all the delegates with this downloader
+    for (NSInteger idx = [downloaders count] - 1; idx >= 0; idx--)
     {
-        // Notify all the delegates with this downloader
-        for (NSInteger idx = [downloaders count] - 1; idx >= 0; idx--)
+        SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:idx];
+        if (aDownloader == downloader)
         {
-            SDWebImageDownloader *aDownloader = [downloaders objectAtIndex:idx];
-            if (aDownloader == downloader)
-            {
-                id<SDWebImageManagerDelegate> delegate = [delegates objectAtIndex:idx];
-
-                if (image && [delegate respondsToSelector:@selector(webImageManager:didFinishWithImage:)])
-                {
-                    [delegate performSelector:@selector(webImageManager:didFinishWithImage:) withObject:self withObject:image];
-                }
+            id<SDWebImageManagerDelegate> delegate = [delegates objectAtIndex:idx];
 
-                [downloaders removeObjectAtIndex:idx];
-                [delegates removeObjectAtIndex:idx];
+            if (image && [delegate respondsToSelector:@selector(webImageManager:didFinishWithImage:)])
+            {
+                [delegate performSelector:@selector(webImageManager:didFinishWithImage:) withObject:self withObject:image];
             }
+
+            [downloaders removeObjectAtIndex:idx];
+            [delegates removeObjectAtIndex:idx];
         }
     }