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.

Sign up for GitHub

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

Pulls out speed monitoring and adds support for using cell vs. wifi if speed unavailable. #403

Merged
merged 4 commits into from
Sep 1, 2017
Merged

Pulls out speed monitoring and adds support for using cell vs. wifi if speed unavailable. #403

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f06168a
Add support for imageCompletion on URLs
garrettmoon Aug 30, 2017
c1500a6
Adds support for determining which URL to download based on type of c…
garrettmoon Aug 31, 2017
6246dbc
Add CHANGELOG.md entry
garrettmoon Aug 31, 2017
6699a1d
Address Huy's feedback
garrettmoon Sep 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- [new] Added a way to specify custom retry logic when network error happens [#386](https://github.com/pinterest/PINRemoteImage/pull/386)

- [new] Improve disk cache migration performance [#391](https://github.com/pinterest/PINRemoteImage/pull/391) [chuganzy](https://github.com/chuganzy), [#394](https://github.com/pinterest/PINRemoteImage/pull/394) [nguyenhuy](https://github.com/nguyenhuy)
- [new] Adds support for using cell vs. wifi in leau of speed for determing which URL to download if speed is unavailable. [garrettmoon](https://github.com/garrettmoon)

## 3.0.0 Beta 11
- [fixed] Fixes a deadlock with canceling processor tasks [#374](https://github.com/pinterest/PINRemoteImage/pull/374) [zachwaugh](https://github.com/zachwaugh)
Expand Down
8 changes: 8 additions & 0 deletions PINRemoteImage.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@
6818C2851E551DA800875DB7 /* utils.h in Headers */ = {isa = PBXBuildFile; fileRef = 6818C26B1E551DA800875DB7 /* utils.h */; };
6858C0751C9CC5BA00E420EB /* PINRemoteLock.h in Headers */ = {isa = PBXBuildFile; fileRef = 6858C0731C9CC5BA00E420EB /* PINRemoteLock.h */; };
6858C0761C9CC5BA00E420EB /* PINRemoteLock.m in Sources */ = {isa = PBXBuildFile; fileRef = 6858C0741C9CC5BA00E420EB /* PINRemoteLock.m */; };
6860CB081F578287005E886E /* PINSpeedRecorder.h in Headers */ = {isa = PBXBuildFile; fileRef = 6860CB061F578287005E886E /* PINSpeedRecorder.h */; };
6860CB091F578287005E886E /* PINSpeedRecorder.m in Sources */ = {isa = PBXBuildFile; fileRef = 6860CB071F578287005E886E /* PINSpeedRecorder.m */; };
686D48D01ED38FC0003DB4C2 /* PINRemoteImageTask+Subclassing.h in Headers */ = {isa = PBXBuildFile; fileRef = 686D48CE1ED38FC0003DB4C2 /* PINRemoteImageTask+Subclassing.h */; };
687D3FC91E5650790027B131 /* PINOperation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 687D3FC81E5650790027B131 /* PINOperation.framework */; };
68A0FC1C1E523434000B552D /* PINRemoteImageTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 68A0FC1B1E523434000B552D /* PINRemoteImageTests.m */; };
Expand Down Expand Up @@ -450,6 +452,8 @@
6818C2AE1E564FF900875DB7 /* PINCache.xcodeproj */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.pb-project"; name = PINCache.xcodeproj; path = Carthage/Checkouts/PINCache/PINCache.xcodeproj; sourceTree = "<group>"; };
6858C0731C9CC5BA00E420EB /* PINRemoteLock.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PINRemoteLock.h; sourceTree = "<group>"; };
6858C0741C9CC5BA00E420EB /* PINRemoteLock.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = PINRemoteLock.m; sourceTree = "<group>"; };
6860CB061F578287005E886E /* PINSpeedRecorder.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PINSpeedRecorder.h; sourceTree = "<group>"; };
6860CB071F578287005E886E /* PINSpeedRecorder.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = PINSpeedRecorder.m; sourceTree = "<group>"; };
686D48CE1ED38FC0003DB4C2 /* PINRemoteImageTask+Subclassing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "PINRemoteImageTask+Subclassing.h"; sourceTree = "<group>"; };
687D3FC81E5650790027B131 /* PINOperation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = PINOperation.framework; path = "Carthage/Checkouts/PINCache/Carthage/Checkouts/PINOperation/build/Debug-iphoneos/PINOperation.framework"; sourceTree = "<group>"; };
68A0FC191E523434000B552D /* PINRemoteImageTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = PINRemoteImageTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -866,6 +870,8 @@
68B1F2801E679D7A00ED87C4 /* PINRemoteImageDownloadQueue.m */,
68B7E3B01E736C73000FC887 /* PINResume.h */,
68B7E3B11E736C73000FC887 /* PINResume.m */,
6860CB061F578287005E886E /* PINSpeedRecorder.h */,
6860CB071F578287005E886E /* PINSpeedRecorder.m */,
);
name = Project;
path = Classes;
Expand Down Expand Up @@ -953,6 +959,7 @@
6818C2421E551D7500875DB7 /* vp8enci.h in Headers */,
F1B919161BCF23C900710963 /* PINRemoteImageManager.h in Headers */,
6818C2361E551D7500875DB7 /* histogram.h in Headers */,
6860CB081F578287005E886E /* PINSpeedRecorder.h in Headers */,
9DD47F9E1C699F4B00F12CA0 /* PINImageView+PINRemoteImage.h in Headers */,
68CA92831DB19C20008BECE2 /* PINCache+PINRemoteImageCaching.h in Headers */,
6818C18C1E551CE000875DB7 /* vp8li.h in Headers */,
Expand Down Expand Up @@ -1289,6 +1296,7 @@
6818C1DF1E551D1D00875DB7 /* dec_mips_dsp_r2.c in Sources */,
6818C22F1E551D7500875DB7 /* cost.c in Sources */,
6818C1E91E551D1D00875DB7 /* enc_mips32.c in Sources */,
6860CB091F578287005E886E /* PINSpeedRecorder.m in Sources */,
6818C23F1E551D7500875DB7 /* syntax.c in Sources */,
6818C1EA1E551D1D00875DB7 /* enc_neon.c in Sources */,
6818C23C1E551D7500875DB7 /* picture_tools.c in Sources */,
Expand Down
21 changes: 20 additions & 1 deletion Source/Classes/PINRemoteImageManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ typedef void(^PINRemoteImageManagerProgressDownload)(int64_t completedBytes, int

Unless setShouldUpgradeLowQualityImages is set to YES, this method checks the cache for all URLs and returns the highest quality version stored. It is possible though unlikely for a cached image to not be returned if it is still being cached while a call is made to this method and if network conditions have changed. See source for more details.

@param urls An array of NSURLs of increasing size.
@param urls An array of NSURLs of increasing cost to download.
@param options PINRemoteImageManagerDownloadOptions options with which to fetch the image.
@param progressImage PINRemoteImageManagerImageCompletion block which will be called to update progress of the image download.
@param completion PINRemoteImageManagerImageCompletion block to call when image has been fetched from the cache or downloaded.
Expand All @@ -453,6 +453,25 @@ typedef void(^PINRemoteImageManagerProgressDownload)(int64_t completedBytes, int
progressImage:(nullable PINRemoteImageManagerImageCompletion)progressImage
completion:(nullable PINRemoteImageManagerImageCompletion)completion;

/**
Download or retrieve from cache one of the images found at the urls in the passed in array based on current network performance. URLs should be sorted from lowest quality image URL to highest. 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).

Unless setShouldUpgradeLowQualityImages is set to YES, this method checks the cache for all URLs and returns the highest quality version stored. It is possible though unlikely for a cached image to not be returned if it is still being cached while a call is made to this method and if network conditions have changed. See source for more details.

@param urls An array of NSURLs of increasing cost to download.
@param options PINRemoteImageManagerDownloadOptions options with which to fetch the image.
@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 *)downloadImageWithURLs:(nonnull NSArray <NSURL *> *)urls
options:(PINRemoteImageManagerDownloadOptions)options
progressImage:(nullable PINRemoteImageManagerImageCompletion)progressImage
progressDownload:(nullable PINRemoteImageManagerProgressDownload)progressDownload
completion:(nullable PINRemoteImageManagerImageCompletion)completion;


/**
Adds an image manually into the memory and disk cache.
Expand Down
176 changes: 61 additions & 115 deletions Source/Classes/PINRemoteImageManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@
#import "PINRemoteImageProcessorTask.h"
#import "PINRemoteImageDownloadTask.h"
#import "PINResume.h"
#import "PINURLSessionManager.h"
#import "PINRemoteImageMemoryContainer.h"
#import "PINRemoteImageCaching.h"
#import "PINRequestRetryStrategy.h"
#import "PINRemoteImageDownloadQueue.h"
#import "PINRequestRetryStrategy.h"
#import "PINSpeedRecorder.h"
#import "PINURLSessionManager.h"

#import "NSData+ImageDetectors.h"
#import "PINImage+DecodedImage.h"
Expand Down Expand Up @@ -82,15 +83,6 @@ float dataTaskPriorityWithImageManagerPriority(PINRemoteImageManagerPriority pri
NSString * const PINRemoteImageCacheKeyResumePrefix = @"R-";
typedef void (^PINRemoteImageManagerDataCompletion)(NSData *data, NSURLResponse *response, NSError *error);

@interface PINTaskQOS : NSObject

- (instancetype)initWithBPS:(float)bytesPerSecond endDate:(NSDate *)endDate;

@property (nonatomic, strong) NSDate *endDate;
@property (nonatomic, assign) float bytesPerSecond;

@end

@interface PINRemoteImageManager () <PINURLSessionManagerDelegate>
{
dispatch_queue_t _callbackQueue;
Expand All @@ -116,7 +108,6 @@ @interface PINRemoteImageManager () <PINURLSessionManagerDelegate>
@property (nonatomic, strong) dispatch_queue_t callbackQueue;
@property (nonatomic, strong) PINOperationQueue *concurrentOperationQueue;
@property (nonatomic, strong) PINRemoteImageDownloadQueue *urlSessionTaskQueue;
@property (nonatomic, strong) NSMutableArray <PINTaskQOS *> *taskQOS;
@property (nonatomic, assign) float highQualityBPSThreshold;
@property (nonatomic, assign) float lowQualityBPSThreshold;
@property (nonatomic, assign) BOOL shouldUpgradeLowQualityImages;
Expand All @@ -125,8 +116,6 @@ @interface PINRemoteImageManager () <PINURLSessionManagerDelegate>
@property (nonatomic, copy) PINRemoteImageManagerRequestConfigurationHandler requestConfigurationHandler;
@property (nonatomic, strong) NSMutableDictionary <NSString *, NSString *> *httpHeaderFields;
#if DEBUG
@property (nonatomic, assign) float currentBPS;
@property (nonatomic, assign) BOOL overrideBPS;
@property (nonatomic, assign) NSUInteger totalDownloads;
#endif

Expand Down Expand Up @@ -208,7 +197,6 @@ - (nonnull instancetype)initWithSessionConfiguration:(nullable NSURLSessionConfi
_maxProgressiveRenderSize = CGSizeMake(1024, 1024);
self.tasks = [[NSMutableDictionary alloc] init];
self.canceledTasks = [[NSHashTable alloc] initWithOptions:NSHashTableWeakMemory capacity:5];
self.taskQOS = [[NSMutableArray alloc] initWithCapacity:5];

if (alternateRepProvider == nil) {
_defaultAlternateRepresentationProvider = [[PINAlternateRepresentationProvider alloc] init];
Expand Down Expand Up @@ -1164,85 +1152,30 @@ - (void)didCompleteTask:(NSURLSessionTask *)task withError:(NSError *)error

float bytesPerSecond = task.bytesPerSecond;
if (bytesPerSecond) {
[self addTaskBPS:task.bytesPerSecond endDate:[NSDate date]];
[[PINSpeedRecorder sharedRecorder] addTaskBPS:task.bytesPerSecond endDate:[NSDate date]];
}
}
}

#pragma mark - QOS

- (float)currentBytesPerSecond
{
[self lock];
#if DEBUG
if (self.overrideBPS) {
float currentBPS = self.currentBPS;
[self unlock];
return currentBPS;
}
#endif

const NSTimeInterval validThreshold = 60.0;
__block NSUInteger count = 0;
__block float bps = 0;
__block BOOL valid = NO;

NSDate *threshold = [NSDate dateWithTimeIntervalSinceNow:-validThreshold];
[self.taskQOS enumerateObjectsWithOptions:NSEnumerationReverse usingBlock:^(PINTaskQOS *taskQOS, NSUInteger idx, BOOL *stop) {
if ([taskQOS.endDate compare:threshold] == NSOrderedAscending) {
*stop = YES;
return;
}
valid = YES;
count++;
bps += taskQOS.bytesPerSecond;

}];
[self unlock];

if (valid == NO) {
return -1;
}

return bps / (float)count;
}

- (void)addTaskBPS:(float)bytesPerSecond endDate:(NSDate *)endDate
{
//if bytesPerSecond is less than or equal to zero, ignore.
if (bytesPerSecond <= 0) {
return;
}

[self lock];
if (self.taskQOS.count >= 5) {
[self.taskQOS removeObjectAtIndex:0];
}

PINTaskQOS *taskQOS = [[PINTaskQOS alloc] initWithBPS:bytesPerSecond endDate:endDate];

[self.taskQOS addObject:taskQOS];
[self.taskQOS sortUsingComparator:^NSComparisonResult(PINTaskQOS *obj1, PINTaskQOS *obj2) {
return [obj1.endDate compare:obj2.endDate];
}];

[self unlock];
}

#if DEBUG
- (void)setCurrentBytesPerSecond:(float)currentBPS
{
[self lockOnMainThread];
_overrideBPS = YES;
_currentBPS = currentBPS;
[self unlock];
}
#endif

- (NSUUID *)downloadImageWithURLs:(NSArray <NSURL *> *)urls
options:(PINRemoteImageManagerDownloadOptions)options
progressImage:(PINRemoteImageManagerImageCompletion)progressImage
completion:(PINRemoteImageManagerImageCompletion)completion
{
return [self downloadImageWithURLs:urls
options:options
progressImage:progressImage
progressDownload:nil
completion:completion];
}

- (nullable NSUUID *)downloadImageWithURLs:(nonnull NSArray <NSURL *> *)urls
options:(PINRemoteImageManagerDownloadOptions)options
progressImage:(nullable PINRemoteImageManagerImageCompletion)progressImage
progressDownload:(nullable PINRemoteImageManagerProgressDownload)progressDownload
completion:(nullable PINRemoteImageManagerImageCompletion)completion
{
NSUUID *UUID = [NSUUID UUID];
if (urls.count <= 1) {
Expand All @@ -1253,7 +1186,7 @@ - (NSUUID *)downloadImageWithURLs:(NSArray <NSURL *> *)urls
processorKey:nil
processor:nil
progressImage:progressImage
progressDownload:nil
progressDownload:progressDownload
completion:completion
inputUUID:UUID];
return UUID;
Expand Down Expand Up @@ -1286,28 +1219,17 @@ - (NSUUID *)downloadImageWithURLs:(NSArray <NSURL *> *)urls
}
}];

float currentBytesPerSecond = [strongSelf currentBytesPerSecond];
[strongSelf lock];
float highQualityQPSThreshold = [strongSelf highQualityBPSThreshold];
float lowQualityQPSThreshold = [strongSelf lowQualityBPSThreshold];
BOOL shouldUpgradeLowQualityImages = [strongSelf shouldUpgradeLowQualityImages];
[strongSelf unlock];

NSUInteger desiredImageURLIdx;
if (currentBytesPerSecond == -1 || currentBytesPerSecond >= highQualityQPSThreshold) {
desiredImageURLIdx = urls.count - 1;
} else if (currentBytesPerSecond <= lowQualityQPSThreshold) {
desiredImageURLIdx = 0;
} else if (urls.count == 2) {
desiredImageURLIdx = roundf((currentBytesPerSecond - lowQualityQPSThreshold) / ((highQualityQPSThreshold - lowQualityQPSThreshold) / (float)(urls.count - 1)));
} else {
desiredImageURLIdx = ceilf((currentBytesPerSecond - lowQualityQPSThreshold) / ((highQualityQPSThreshold - lowQualityQPSThreshold) / (float)(urls.count - 2)));
}
NSUInteger desiredImageURLIdx = [strongSelf appropriateImageIdxForURLsGivenHistoricalNetworkConditions:urls];

NSUInteger downloadIdx;
//if the highest quality already downloaded is less than what currentBPS would dictate and shouldUpgrade is
//set, download the new higher quality image. If no image has been cached, download the image dictated by
//current bps

[strongSelf lock];
BOOL shouldUpgradeLowQualityImages = [strongSelf shouldUpgradeLowQualityImages];
[strongSelf unlock];
Copy link
Member

Choose a reason for hiding this comment

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

This may well be a micro-optimization, but to avoid grabbing the lock multiple times, how about taking this chance to get high and low quality thresholds and pass them to -appropriateImageIdxForURLsGivenHistoricalNetworkConditions? Then that method can be static because it no longer needs access to self.


if ((highestQualityDownloadedIdx < desiredImageURLIdx && shouldUpgradeLowQualityImages) || highestQualityDownloadedIdx == -1) {
downloadIdx = desiredImageURLIdx;
} else {
Expand All @@ -1322,7 +1244,7 @@ - (NSUUID *)downloadImageWithURLs:(NSArray <NSURL *> *)urls
processorKey:nil
processor:nil
progressImage:progressImage
progressDownload:nil
progressDownload:progressDownload
completion:^(PINRemoteImageManagerResult *result) {
typeof(self) strongSelf = weakSelf;
//clean out any lower quality images from the cache
Expand All @@ -1339,6 +1261,43 @@ - (NSUUID *)downloadImageWithURLs:(NSArray <NSURL *> *)urls
return UUID;
}

- (NSUInteger)appropriateImageIdxForURLsGivenHistoricalNetworkConditions:(NSArray <NSURL *> *)urls
{
float currentBytesPerSecond = [[PINSpeedRecorder sharedRecorder] currentBytesPerSecond];
[self lock];
float highQualityQPSThreshold = [self highQualityBPSThreshold];
float lowQualityQPSThreshold = [self lowQualityBPSThreshold];
[self unlock];

NSUInteger desiredImageURLIdx;

if (currentBytesPerSecond == -1) {
// Base it on reachability
switch ([[PINSpeedRecorder sharedRecorder] connectionStatus]) {
case PINSpeedRecorderConnectionStatusWiFi:
desiredImageURLIdx = urls.count - 1;
break;

case PINSpeedRecorderConnectionStatusWWAN:
case PINSpeedRecorderConnectionStatusNotReachable:
desiredImageURLIdx = 0;
break;
}
} else {
if (currentBytesPerSecond >= highQualityQPSThreshold) {
desiredImageURLIdx = urls.count - 1;
} else if (currentBytesPerSecond <= lowQualityQPSThreshold) {
desiredImageURLIdx = 0;
} else if (urls.count == 2) {
desiredImageURLIdx = roundf((currentBytesPerSecond - lowQualityQPSThreshold) / ((highQualityQPSThreshold - lowQualityQPSThreshold) / (float)(urls.count - 1)));
} else {
desiredImageURLIdx = ceilf((currentBytesPerSecond - lowQualityQPSThreshold) / ((highQualityQPSThreshold - lowQualityQPSThreshold) / (float)(urls.count - 2)));
}
}

return desiredImageURLIdx;
}

#pragma mark - Caching

- (BOOL)materializeAndCacheObject:(id)object
Expand Down Expand Up @@ -1594,16 +1553,3 @@ - (NSUInteger)totalDownloads
#endif

@end

@implementation PINTaskQOS

- (instancetype)initWithBPS:(float)bytesPerSecond endDate:(NSDate *)endDate
{
if (self = [super init]) {
self.endDate = endDate;
self.bytesPerSecond = bytesPerSecond;
}
return self;
}

@end
Loading