Skip to content

Commit

Permalink
Fixes Issue:538 - Memory problem/leak on iOS 7
Browse files Browse the repository at this point in the history
This was happening as the decompressed images take up lot of memory on older iOS devices
such as iPad 2 with a collection view and crashes the app without a memory warning most of the times.
These options don't break existing features but adds:
    * Adds Queue options to SDWebImagePrefetcher.
    * Adds option to decompress images in cache and post download.
  • Loading branch information
Harish Krishnamurthy committed Dec 30, 2014
1 parent 21656fa commit ed64e00
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 8 deletions.
6 changes: 6 additions & 0 deletions SDWebImage/SDImageCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ typedef void(^SDWebImageCalculateSizeBlock)(NSUInteger fileCount, NSUInteger tot
*/
@interface SDImageCache : NSObject

/**
* Decompressing images that are downloaded and cached can improve peformance but can consume lot of memory.
* Defaults to YES. Set this to NO if you are experiencing a crash due to excessive memory consumption.
*/
@property (assign, nonatomic) BOOL shouldDecompressImages;

/**
* The maximum "total cost" of the in-memory image cache. The cost function is the number of pixels held in memory.
*/
Expand Down
7 changes: 6 additions & 1 deletion SDWebImage/SDImageCache.m
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ - (id)initWithNamespace:(NSString *)ns {
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
_diskCachePath = [paths[0] stringByAppendingPathComponent:fullNamespace];

// Set decompression to YES
_shouldDecompressImages = YES;

dispatch_sync(_ioQueue, ^{
_fileManager = [NSFileManager new];
});
Expand Down Expand Up @@ -266,7 +269,9 @@ - (UIImage *)diskImageForKey:(NSString *)key {
if (data) {
UIImage *image = [UIImage sd_imageWithData:data];
image = [self scaledImageForKey:key image:image];
image = [UIImage decodedImageWithImage:image];
if (self.shouldDecompressImages) {
image = [UIImage decodedImageWithImage:image];
}
return image;
}
else {
Expand Down
9 changes: 6 additions & 3 deletions SDWebImage/SDWebImageDownloader.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ typedef NS_OPTIONS(NSUInteger, SDWebImageDownloaderOptions) {
* Put the image in the high priority queue.
*/
SDWebImageDownloaderHighPriority = 1 << 7,


};

typedef NS_ENUM(NSInteger, SDWebImageDownloaderExecutionOrder) {
Expand Down Expand Up @@ -79,12 +77,17 @@ typedef NSDictionary *(^SDWebImageDownloaderHeadersFilterBlock)(NSURL *url, NSDi
*/
@interface SDWebImageDownloader : NSObject

/**
* Decompressing images that are downloaded and cached can improve peformance but can consume lot of memory.
* Defaults to YES. Set this to NO if you are experiencing a crash due to excessive memory consumption.
*/
@property (assign, nonatomic) BOOL shouldDecompressImages;

@property (assign, nonatomic) NSInteger maxConcurrentDownloads;

/**
* Shows the current amount of downloads that still need to be downloaded
*/

@property (readonly, nonatomic) NSUInteger currentDownloadCount;


Expand Down
2 changes: 2 additions & 0 deletions SDWebImage/SDWebImageDownloader.m
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ + (SDWebImageDownloader *)sharedDownloader {
- (id)init {
if ((self = [super init])) {
_operationClass = [SDWebImageDownloaderOperation class];
_shouldDecompressImages = YES;
_executionOrder = SDWebImageDownloaderFIFOExecutionOrder;
_downloadQueue = [NSOperationQueue new];
_downloadQueue.maxConcurrentOperationCount = 6;
Expand Down Expand Up @@ -158,6 +159,7 @@ - (void)setOperationClass:(Class)operationClass {
if (!sself) return;
[sself removeCallbacksForURL:url];
}];
operation.shouldDecompressImages = wself.shouldDecompressImages;

if (wself.username && wself.password) {
operation.credential = [NSURLCredential credentialWithUser:wself.username password:wself.password persistence:NSURLCredentialPersistenceForSession];
Expand Down
3 changes: 3 additions & 0 deletions SDWebImage/SDWebImageDownloaderOperation.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
*/
@property (strong, nonatomic, readonly) NSURLRequest *request;


@property (assign, nonatomic) BOOL shouldDecompressImages;

/**
* Whether the URL connection should consult the credential storage for authenticating the connection. `YES` by default.
*
Expand Down
12 changes: 10 additions & 2 deletions SDWebImage/SDWebImageDownloaderOperation.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ - (id)initWithRequest:(NSURLRequest *)request
cancelled:(SDWebImageNoParamsBlock)cancelBlock {
if ((self = [super init])) {
_request = request;
_shouldDecompressImages = YES;
_shouldUseCredentialStorage = YES;
_options = options;
_progressBlock = [progressBlock copy];
Expand Down Expand Up @@ -289,7 +290,12 @@ - (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data {
UIImage *image = [UIImage imageWithCGImage:partialImageRef scale:1 orientation:orientation];
NSString *key = [[SDWebImageManager sharedManager] cacheKeyForURL:self.request.URL];
UIImage *scaledImage = [self scaledImageForKey:key image:image];
image = [UIImage decodedImageWithImage:scaledImage];
if (self.shouldDecompressImages) {
image = [UIImage decodedImageWithImage:scaledImage];
}
else {
image = scaledImage;
}
CGImageRelease(partialImageRef);
dispatch_main_sync_safe(^{
if (self.completedBlock) {
Expand Down Expand Up @@ -358,7 +364,9 @@ - (void)connectionDidFinishLoading:(NSURLConnection *)aConnection {

// Do not force decoding animated GIFs
if (!image.images) {
image = [UIImage decodedImageWithImage:image];
if (self.shouldDecompressImages) {
image = [UIImage decodedImageWithImage:image];
}
}
if (CGSizeEqualToSize(image.size, CGSizeZero)) {
completionBlock(nil, nil, [NSError errorWithDomain:@"SDWebImageErrorDomain" code:0 userInfo:@{NSLocalizedDescriptionKey : @"Downloaded image has 0 pixels"}], YES);
Expand Down
5 changes: 5 additions & 0 deletions SDWebImage/SDWebImagePrefetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ typedef void(^SDWebImagePrefetcherCompletionBlock)(NSUInteger noOfFinishedUrls,
*/
@property (nonatomic, assign) SDWebImageOptions options;

/**
* Queue options for Prefetcher. Defaults to Main Queue.
*/
@property (nonatomic, assign) dispatch_queue_t prefetcherQueue;

@property (weak, nonatomic) id <SDWebImagePrefetcherDelegate> delegate;

/**
Expand Down
4 changes: 2 additions & 2 deletions SDWebImage/SDWebImagePrefetcher.m
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ - (id)init {
if ((self = [super init])) {
_manager = [SDWebImageManager new];
_options = SDWebImageLowPriority;
_prefetcherQueue = dispatch_get_main_queue();
self.maxConcurrentDownloads = 3;
}
return self;
Expand Down Expand Up @@ -82,9 +83,8 @@ - (void)startPrefetchingAtIndex:(NSUInteger)index {
totalCount:self.prefetchURLs.count
];
}

if (self.prefetchURLs.count > self.requestedCount) {
dispatch_async(dispatch_get_main_queue(), ^{
dispatch_async(self.prefetcherQueue, ^{
[self startPrefetchingAtIndex:self.requestedCount];
});
}
Expand Down

4 comments on commit ed64e00

@rromanchuk
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harishkashyap any chance you could merge some of the other solid PRs into your fork? Going to switch over to your fork.

https://github.com/rs/SDWebImage/pull/988/files

Some other candidates
SDWebImage#997
SDWebImage#1004
SDWebImage#985

@harishkashyap
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing.

@harishkashyap
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rromanchuk I am waiting to hear from @Jur4s.

@harishkashyap
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rromanchuk tested SDWebImage#997 out. unfortunately for us, this slows the loading on collection views. I think its a multi threading problem which requires much deeper investigation than wrapping the start function with the main_sync_safe method. So I can't add this to the fork. Others are looking good.

Please sign in to comment.