Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set priority of download data tasks at the right time #490

Merged
merged 10 commits into from
Feb 20, 2019
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [cleanup] Use NS_ERROR_ENUM to improve Swift import. [#440](https://github.com/pinterest/PINRemoteImage/pull/440) [Adlai-Holler](https://github.com/Adlai-Holler)
- [fixed] Fixes nil session manager configuration. [#460](https://github.com/pinterest/PINRemoteImage/pull/460) [garrettmoon](https://github.com/garrettmoon)
- [fixed] Fixes deprecated -defaultImageCache not being called if overridden. [479](https://github.com/pinterest/PINRemoteImage/pull/479) [nguyenhuy](https://github.com/nguyenhuy)
- [new] Add a new API that allows a priority to be set when a new download task is scheduled. [#490](https://github.com/pinterest/PINRemoteImage/pull/490) [nguyenhuy](https://github.com/nguyenhuy)

## 3.0.0 Beta 13
- [new] Support for webp and improved support for GIFs. [#411](https://github.com/pinterest/PINRemoteImage/pull/411) [garrettmoon](https://github.com/garrettmoon)
Expand Down
2 changes: 1 addition & 1 deletion Source/Classes/PINRemoteImageDownloadQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ typedef void (^PINRemoteImageDownloadCompletion)(NSURLResponse * _Nullable respo

@interface PINRemoteImageDownloadQueue : NSObject

@property (nonatomic, assign) NSUInteger maxNumberOfConcurrentDownloads;
@property (atomic, assign) NSUInteger maxNumberOfConcurrentDownloads;
Copy link
Member Author

Choose a reason for hiding this comment

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

This property is actually thread-safe so I thought it'd be a good idea to let clients know that.


- (instancetype)init NS_UNAVAILABLE;
+ (PINRemoteImageDownloadQueue *)queueWithMaxConcurrentDownloads:(NSUInteger)maxNumberOfConcurrentDownloads;
Expand Down
27 changes: 14 additions & 13 deletions Source/Classes/PINRemoteImageDownloadQueue.m
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ - (PINRemoteImageDownloadQueue *)initWithMaxConcurrentDownloads:(NSUInteger)maxN
{
if (self = [super init]) {
_maxNumberOfConcurrentDownloads = maxNumberOfConcurrentDownloads;

_lock = [[PINRemoteLock alloc] initWithName:@"PINRemoteImageDownloadQueue Lock"];
_highPriorityQueuedOperations = [[NSMutableOrderedSet alloc] init];
_defaultPriorityQueuedOperations = [[NSMutableOrderedSet alloc] init];
Expand Down Expand Up @@ -68,19 +68,21 @@ - (NSURLSessionDataTask *)addDownloadWithSessionManager:(PINURLSessionManager *)
priority:(PINRemoteImageManagerPriority)priority
completionHandler:(PINRemoteImageDownloadCompletion)completionHandler
{
NSURLSessionDataTask *dataTask = [sessionManager dataTaskWithRequest:request completionHandler:^(NSURLSessionTask *task, NSError *error) {
completionHandler(task.response, error);
[self lock];
[self->_runningTasks removeObject:task];
[self unlock];

[self scheduleDownloadsIfNeeded];
}];

NSURLSessionDataTask *dataTask = [sessionManager dataTaskWithRequest:request
priority:priority
completionHandler:^(NSURLSessionTask *task, NSError *error) {
completionHandler(task.response, error);
[self lock];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the indentation change here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The new priority param made this line too long so I break it into multiple lines hence the new indentation.

[self->_runningTasks removeObject:task];
[self unlock];

[self scheduleDownloadsIfNeeded];
}];

[self setQueuePriority:priority forTask:dataTask addIfNecessary:YES];

[self scheduleDownloadsIfNeeded];

return dataTask;
}

Expand All @@ -105,7 +107,6 @@ - (void)scheduleDownloadsIfNeeded
[queue removeObjectAtIndex:0];
[task resume];


[_runningTasks addObject:task];
}
[self unlock];
Expand Down
11 changes: 4 additions & 7 deletions Source/Classes/PINRemoteImageDownloadTask.m
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ - (void)setPriority:(PINRemoteImageManagerPriority)priority
[super setPriority:priority];
if (@available(iOS 8.0, macOS 10.10, tvOS 9.0, watchOS 2.0, *)) {
[self.lock lockWithBlock:^{
if (self->_progressImage.dataTask) {
self->_progressImage.dataTask.priority = dataTaskPriorityWithImageManagerPriority(priority);
[self.manager.urlSessionTaskQueue setQueuePriority:priority forTask:self->_progressImage.dataTask];
NSURLSessionDataTask *dataTask = self->_progressImage.dataTask;
if (dataTask) {
dataTask.priority = dataTaskPriorityWithImageManagerPriority(priority);
[self.manager.urlSessionTaskQueue setQueuePriority:priority forTask:dataTask];
Copy link
Member Author

Choose a reason for hiding this comment

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

No behavior change here, just cache the data task in a variable for better readability.

}
}];
}
Expand Down Expand Up @@ -342,10 +343,6 @@ - (void)scheduleDownloadWithRequest:(NSURLRequest *)request
}
}];
}]];

if (@available(iOS 8.0, macOS 10.10, tvOS 9.0, watchOS 2.0, *)) {
self->_progressImage.dataTask.priority = dataTaskPriorityWithImageManagerPriority(priority);
}
}];
}

Expand Down
21 changes: 19 additions & 2 deletions Source/Classes/PINRemoteImageManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ typedef void(^PINRemoteImageManagerMetrics)(NSURL * __nonnull url, NSURLSession
alternativeRepresentationProvider:(nullable id <PINRemoteImageManagerAlternateRepresentationProvider>)alternateRepDelegate
imageCache:(nullable id<PINRemoteImageCaching>)imageCache NS_DESIGNATED_INITIALIZER;


/**
Get the shared instance of PINRemoteImageManager

Expand Down Expand Up @@ -456,6 +455,25 @@ typedef void(^PINRemoteImageManagerMetrics)(NSURL * __nonnull url, NSURLSession
progressDownload:(nullable PINRemoteImageManagerProgressDownload)progressDownload
completion:(nullable PINRemoteImageManagerImageCompletion)completion;

/**
Download or retrieve from cache the image found at the url and process it before calling completion. All completions are called on an arbitrary callback queue unless called on the main thread and the result is in the memory cache (this is an optimization to allow synchronous results for the UI when an object is cached in memory).

@param url NSURL where the image to download resides.
@param options PINRemoteImageManagerDownloadOptions options with which to fetch the image.
@param priority PINRemoteImageManagerPriority which indicates the priority of the download task.
@param progressImage PINRemoteImageManagerImageCompletion block which will be called to update progress of the image download.
@param progressDownload PINRemoteImageManagerDownloadProgress block which will be called to update progress in bytes of the image download. NOTE: For performance reasons, this block is not called on the main thread every time, if you need to update your UI ensure that you dispatch to the main thread first.
@param completion PINRemoteImageManagerImageCompletion block to call when image has been fetched from the cache or downloaded.

@return An NSUUID which uniquely identifies this request. To be used for canceling requests and verifying that the callback is for the request you expect (see categories for example).
*/
- (nullable NSUUID *)downloadImageWithURL:(nonnull NSURL *)url
options:(PINRemoteImageManagerDownloadOptions)options
priority:(PINRemoteImageManagerPriority)priority
progressImage:(nullable PINRemoteImageManagerImageCompletion)progressImage
progressDownload:(nullable PINRemoteImageManagerProgressDownload)progressDownload
completion:(nullable PINRemoteImageManagerImageCompletion)completion;

/**
Download or retrieve from cache the image found at the url and process it before calling completion. All completions are called on an arbitrary callback queue unless called on the main thread and the result is in the memory cache (this is an optimization to allow synchronous results for the UI when an object is cached in memory).

Expand Down Expand Up @@ -651,5 +669,4 @@ typedef void(^PINRemoteImageManagerMetrics)(NSURL * __nonnull url, NSURLSession

@property (nonatomic, readonly) _Nonnull id<PINRequestRetryStrategy> (^_Nonnull retryStrategyCreationBlock)(void);


@end
43 changes: 25 additions & 18 deletions Source/Classes/PINRemoteImageManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,9 @@ - (NSUUID *)downloadImageWithURL:(NSURL *)url
{
return [self downloadImageWithURL:url
options:options
priority:PINRemoteImageManagerPriorityDefault
processorKey:nil
processor:nil
progressImage:progressImage
progressDownload:nil
completion:completion
inputUUID:nil];
completion:completion];
}

- (NSUUID *)downloadImageWithURL:(NSURL *)url
Expand All @@ -532,13 +528,9 @@ - (NSUUID *)downloadImageWithURL:(NSURL *)url
{
return [self downloadImageWithURL:url
options:options
priority:PINRemoteImageManagerPriorityDefault
processorKey:nil
processor:nil
progressImage:nil
progressDownload:progressDownload
completion:completion
inputUUID:nil];
completion:completion];
}

- (NSUUID *)downloadImageWithURL:(NSURL *)url
Expand All @@ -550,6 +542,21 @@ - (NSUUID *)downloadImageWithURL:(NSURL *)url
return [self downloadImageWithURL:url
options:options
priority:PINRemoteImageManagerPriorityDefault
progressImage:progressImage
progressDownload:progressDownload
completion:completion];
}

- (nullable NSUUID *)downloadImageWithURL:(nonnull NSURL *)url
options:(PINRemoteImageManagerDownloadOptions)options
priority:(PINRemoteImageManagerPriority)priority
progressImage:(PINRemoteImageManagerImageCompletion)progressImage
progressDownload:(nullable PINRemoteImageManagerProgressDownload)progressDownload
completion:(nullable PINRemoteImageManagerImageCompletion)completion;
{
return [self downloadImageWithURL:url
options:options
priority:priority
processorKey:nil
processor:nil
progressImage:progressImage
Expand Down Expand Up @@ -583,14 +590,14 @@ - (NSUUID *)downloadImageWithURL:(NSURL *)url
completion:(PINRemoteImageManagerImageCompletion)completion
{
return [self downloadImageWithURL:url
options:options
priority:PINRemoteImageManagerPriorityDefault
processorKey:processorKey
processor:processor
progressImage:nil
progressDownload:progressDownload
completion:completion
inputUUID:nil];
options:options
priority:PINRemoteImageManagerPriorityDefault
processorKey:processorKey
processor:processor
progressImage:nil
progressDownload:progressDownload
completion:completion
inputUUID:nil];
}

- (NSUUID *)downloadImageWithURL:(NSURL *)url
Expand Down
6 changes: 5 additions & 1 deletion Source/Classes/PINURLSessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#import <Foundation/Foundation.h>

#import "PINRemoteImageManager.h"

extern NSErrorDomain _Nonnull const PINURLErrorDomain;

@protocol PINURLSessionManagerDelegate <NSObject>
Expand All @@ -29,7 +31,9 @@ typedef void (^PINURLSessionDataTaskCompletion)(NSURLSessionTask * _Nonnull task

- (nonnull instancetype)initWithSessionConfiguration:(nullable NSURLSessionConfiguration *)configuration;

- (nonnull NSURLSessionDataTask *)dataTaskWithRequest:(nonnull NSURLRequest *)request completionHandler:(nonnull PINURLSessionDataTaskCompletion)completionHandler;
- (nonnull NSURLSessionDataTask *)dataTaskWithRequest:(nonnull NSURLRequest *)request
priority:(PINRemoteImageManagerPriority)priority
completionHandler:(nonnull PINURLSessionDataTaskCompletion)completionHandler;

- (void)invalidateSessionAndCancelTasks;

Expand Down
7 changes: 6 additions & 1 deletion Source/Classes/PINURLSessionManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ - (void)invalidateSessionAndCancelTasks
[self unlock];
}

- (nonnull NSURLSessionDataTask *)dataTaskWithRequest:(nonnull NSURLRequest *)request completionHandler:(nonnull PINURLSessionDataTaskCompletion)completionHandler
- (nonnull NSURLSessionDataTask *)dataTaskWithRequest:(nonnull NSURLRequest *)request
priority:(PINRemoteImageManagerPriority)priority
completionHandler:(nonnull PINURLSessionDataTaskCompletion)completionHandler
{
[self lock];
NSURLSessionDataTask *dataTask = [self.session dataTaskWithRequest:request];
if (@available(iOS 8.0, macOS 10.10, tvOS 9.0, watchOS 2.0, *)) {
dataTask.priority = dataTaskPriorityWithImageManagerPriority(priority);
}
if (completionHandler) {
[self.completions setObject:completionHandler forKey:@(dataTask.taskIdentifier)];
}
Expand Down