From 54f7eb34243715a1d4bc821ccbadeec12486d22c Mon Sep 17 00:00:00 2001 From: Eric Samelson Date: Mon, 5 Nov 2018 05:38:22 -0800 Subject: [PATCH] Performance improvement for loading cached images on iOS (#20356) Summary: This PR increases the speed at which cached images are loaded and displayed on the screen. Images are currently cached in memory using RCTImageCache, but each time they are loaded, a round trip through RCTNetworking happens before RCTImageCache is even checked. This is likely so that RCTNetworking can handle the caching behavior required by the HTTP headers. However, this means that at the very least, images are read from disk each time they're loaded. This PR makes RCTImageLoader check RCTImageCache _before_ sending a request to RCTNetworking. RCTImageCache stores a bit of information about the response headers so that it can respect Cache-Control fields without needing a roundtrip through RCTNetworking. Here are a couple of graphs showing improved loading times before this change (blue) and after (red) with SDWebImage (yellow) as a baseline comparison. The increase is most evident when loading especially large (hi-res photo size) images, or loading multiple images at a time. https://imgur.com/a/cnL47Z0 More performance gains can potentially be had by increasing the size limit of RCTImageCache: https://github.com/facebook/react-native/blob/1a6666a116fd8b9e8637956de2b41a1c315dd470/Libraries/Image/RCTImageCache.m#L39 but this comes at the tradeoff of being more likely to run into OOM crashes. Pull Request resolved: https://github.com/facebook/react-native/pull/20356 Reviewed By: PeteTheHeat Differential Revision: D12909121 Pulled By: alsun2001 fbshipit-source-id: 7f5e21928c53d7aa53f293b7f1b4ec5c99b5f0c2 --- Libraries/Image/RCTImageCache.m | 68 +++++++++++++++++++++++++---- Libraries/Image/RCTImageLoader.h | 6 +-- Libraries/Image/RCTImageLoader.m | 75 ++++++++++++++++---------------- 3 files changed, 101 insertions(+), 48 deletions(-) diff --git a/Libraries/Image/RCTImageCache.m b/Libraries/Image/RCTImageCache.m index 1c864f505..cf2be6cb3 100644 --- a/Libraries/Image/RCTImageCache.m +++ b/Libraries/Image/RCTImageCache.m @@ -21,23 +21,27 @@ static const NSUInteger RCTMaxCachableDecodedImageSizeInBytes = 1048576; // 1MB static NSString *RCTCacheKeyForImage(NSString *imageTag, CGSize size, CGFloat scale, - RCTResizeMode resizeMode, NSString *responseDate) + RCTResizeMode resizeMode) { - return [NSString stringWithFormat:@"%@|%g|%g|%g|%lld|%@", - imageTag, size.width, size.height, scale, (long long)resizeMode, responseDate]; + return [NSString stringWithFormat:@"%@|%g|%g|%g|%lld", + imageTag, size.width, size.height, scale, (long long)resizeMode]; } @implementation RCTImageCache { NSOperationQueue *_imageDecodeQueue; NSCache *_decodedImageCache; + NSMutableDictionary *_cacheStaleTimes; + + NSDateFormatter *_headerDateFormatter; } - (instancetype)init { _decodedImageCache = [NSCache new]; _decodedImageCache.totalCostLimit = 5 * 1024 * 1024; // 5MB - + _cacheStaleTimes = [[NSMutableDictionary alloc] init]; + [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(clearCache) name:UIApplicationDidReceiveMemoryWarningNotification @@ -58,6 +62,9 @@ static NSString *RCTCacheKeyForImage(NSString *imageTag, CGSize size, CGFloat sc - (void)clearCache { [_decodedImageCache removeAllObjects]; + @synchronized(_cacheStaleTimes) { + [_cacheStaleTimes removeAllObjects]; + } } - (void)addImageToCache:(UIImage *)image @@ -78,9 +85,19 @@ static NSString *RCTCacheKeyForImage(NSString *imageTag, CGSize size, CGFloat sc size:(CGSize)size scale:(CGFloat)scale resizeMode:(RCTResizeMode)resizeMode - responseDate:(NSString *)responseDate { - NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode, responseDate); + NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode); + @synchronized(_cacheStaleTimes) { + id staleTime = _cacheStaleTimes[cacheKey]; + if (staleTime) { + if ([[NSDate new] compare:(NSDate *)staleTime] == NSOrderedDescending) { + // cached image has expired, clear it out to make room for others + [_cacheStaleTimes removeObjectForKey:cacheKey]; + [_decodedImageCache removeObjectForKey:cacheKey]; + return nil; + } + } + } return [_decodedImageCache objectForKey:cacheKey]; } @@ -90,9 +107,44 @@ static NSString *RCTCacheKeyForImage(NSString *imageTag, CGSize size, CGFloat sc scale:(CGFloat)scale resizeMode:(RCTResizeMode)resizeMode responseDate:(NSString *)responseDate + cacheControl:(NSString *)cacheControl { - NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode, responseDate); - return [self addImageToCache:image forKey:cacheKey]; + NSString *cacheKey = RCTCacheKeyForImage(url, size, scale, resizeMode); + BOOL shouldCache = YES; + NSDate *staleTime; + NSArray *components = [cacheControl componentsSeparatedByString:@","]; + for (NSString *component in components) { + if ([component containsString:@"no-cache"] || [component containsString:@"no-store"] || [component hasSuffix:@"max-age=0"]) { + shouldCache = NO; + break; + } else { + NSRange range = [component rangeOfString:@"max-age="]; + if (range.location != NSNotFound) { + NSInteger seconds = [[component substringFromIndex:range.location + range.length] integerValue]; + NSDate *originalDate = [self dateWithHeaderString:responseDate]; + staleTime = [originalDate dateByAddingTimeInterval:(NSTimeInterval)seconds]; + } + } + } + if (shouldCache) { + if (staleTime) { + @synchronized(_cacheStaleTimes) { + _cacheStaleTimes[cacheKey] = staleTime; + } + } + return [self addImageToCache:image forKey:cacheKey]; + } +} + +- (NSDate *)dateWithHeaderString:(NSString *)headerDateString { + if (_headerDateFormatter == nil) { + _headerDateFormatter = [[NSDateFormatter alloc] init]; + _headerDateFormatter.locale = [[NSLocale alloc] initWithLocaleIdentifier:@"en_US_POSIX"]; + _headerDateFormatter.dateFormat = @"EEE',' dd MMM yyyy HH':'mm':'ss 'GMT'"; + _headerDateFormatter.timeZone = [NSTimeZone timeZoneForSecondsFromGMT:0]; + } + + return [_headerDateFormatter dateFromString:headerDateString]; } @end diff --git a/Libraries/Image/RCTImageLoader.h b/Libraries/Image/RCTImageLoader.h index 30d472d52..df879c1fb 100644 --- a/Libraries/Image/RCTImageLoader.h +++ b/Libraries/Image/RCTImageLoader.h @@ -24,15 +24,15 @@ typedef dispatch_block_t RCTImageLoaderCancellationBlock; - (UIImage *)imageForUrl:(NSString *)url size:(CGSize)size scale:(CGFloat)scale - resizeMode:(RCTResizeMode)resizeMode - responseDate:(NSString *)responseDate; + resizeMode:(RCTResizeMode)resizeMode; - (void)addImageToCache:(UIImage *)image URL:(NSString *)url size:(CGSize)size scale:(CGFloat)scale resizeMode:(RCTResizeMode)resizeMode - responseDate:(NSString *)responseDate; + responseDate:(NSString *)responseDate + cacheControl:(NSString *)cacheControl; @end diff --git a/Libraries/Image/RCTImageLoader.m b/Libraries/Image/RCTImageLoader.m index 035bc3535..d296637c1 100644 --- a/Libraries/Image/RCTImageLoader.m +++ b/Libraries/Image/RCTImageLoader.m @@ -321,7 +321,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, resizeMode:(RCTResizeMode)resizeMode progressBlock:(RCTImageLoaderProgressBlock)progressHandler partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadHandler - completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate))completionBlock + completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl))completionBlock { { NSMutableURLRequest *mutableRequest = [request mutableCopy]; @@ -344,15 +344,15 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, BOOL requiresScheduling = [loadHandler respondsToSelector:@selector(requiresScheduling)] ? [loadHandler requiresScheduling] : YES; + BOOL cacheResult = [loadHandler respondsToSelector:@selector(shouldCacheLoadedImages)] ? + [loadHandler shouldCacheLoadedImages] : YES; + __block atomic_bool cancelled = ATOMIC_VAR_INIT(NO); // TODO: Protect this variable shared between threads. __block dispatch_block_t cancelLoad = nil; - void (^completionHandler)(NSError *, id, NSString *) = ^(NSError *error, id imageOrData, NSString *fetchDate) { + void (^completionHandler)(NSError *, id, NSString *, NSString *) = ^(NSError *error, id imageOrData, NSString *fetchDate, NSString *cacheControl) { cancelLoad = nil; - BOOL cacheResult = [loadHandler respondsToSelector:@selector(shouldCacheLoadedImages)] ? - [loadHandler shouldCacheLoadedImages] : YES; - // If we've received an image, we should try to set it synchronously, // if it's data, do decoding on a background thread. if (RCTIsMainQueue() && ![imageOrData isKindOfClass:[UIImage class]]) { @@ -360,11 +360,11 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, // expecting it, and may do expensive post-processing in the callback dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ if (!atomic_load(&cancelled)) { - completionBlock(error, imageOrData, cacheResult, fetchDate); + completionBlock(error, imageOrData, cacheResult, fetchDate, cacheControl); } }); } else if (!atomic_load(&cancelled)) { - completionBlock(error, imageOrData, cacheResult, fetchDate); + completionBlock(error, imageOrData, cacheResult, fetchDate, cacheControl); } }; @@ -378,7 +378,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, progressHandler:progressHandler partialLoadHandler:partialLoadHandler completionHandler:^(NSError *error, UIImage *image){ - completionHandler(error, image, nil); + completionHandler(error, image, nil, nil); }]; } @@ -402,13 +402,25 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, progressHandler:progressHandler partialLoadHandler:partialLoadHandler completionHandler:^(NSError *error, UIImage *image) { - completionHandler(error, image, nil); + completionHandler(error, image, nil, nil); }]; } else { - // Use networking module to load image - cancelLoad = [strongSelf _loadURLRequest:request - progressBlock:progressHandler - completionBlock:completionHandler]; + UIImage *image; + if (cacheResult) { + image = [[strongSelf imageCache] imageForUrl:request.URL.absoluteString + size:size + scale:scale + resizeMode:resizeMode]; + } + + if (image) { + completionHandler(nil, image, nil, nil); + } else { + // Use networking module to load image + cancelLoad = [strongSelf _loadURLRequest:request + progressBlock:progressHandler + completionBlock:completionHandler]; + } } }); @@ -427,7 +439,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, - (RCTImageLoaderCancellationBlock)_loadURLRequest:(NSURLRequest *)request progressBlock:(RCTImageLoaderProgressBlock)progressHandler - completionBlock:(void (^)(NSError *error, id imageOrData, NSString *fetchDate))completionHandler + completionBlock:(void (^)(NSError *error, id imageOrData, NSString *fetchDate, NSString *cacheControl))completionHandler { // Check if networking module is available if (RCT_DEBUG && ![_bridge respondsToSelector:@selector(networking)]) { @@ -449,18 +461,19 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, RCTURLRequestCompletionBlock processResponse = ^(NSURLResponse *response, NSData *data, NSError *error) { // Check for system errors if (error) { - completionHandler(error, nil, nil); + completionHandler(error, nil, nil, nil); return; } else if (!response) { - completionHandler(RCTErrorWithMessage(@"Response metadata error"), nil, nil); + completionHandler(RCTErrorWithMessage(@"Response metadata error"), nil, nil, nil); return; } else if (!data) { - completionHandler(RCTErrorWithMessage(@"Unknown image download error"), nil, nil); + completionHandler(RCTErrorWithMessage(@"Unknown image download error"), nil, nil, nil); return; } // Check for http errors NSString *responseDate; + NSString *cacheControl; if ([response isKindOfClass:[NSHTTPURLResponse class]]) { NSInteger statusCode = ((NSHTTPURLResponse *)response).statusCode; if (statusCode != 200) { @@ -468,15 +481,16 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, NSDictionary *userInfo = @{NSLocalizedDescriptionKey: errorMessage}; completionHandler([[NSError alloc] initWithDomain:NSURLErrorDomain code:statusCode - userInfo:userInfo], nil, nil); + userInfo:userInfo], nil, nil, nil); return; } responseDate = ((NSHTTPURLResponse *)response).allHeaderFields[@"Date"]; + cacheControl = ((NSHTTPURLResponse *)response).allHeaderFields[@"Cache-Control"]; } // Call handler - completionHandler(nil, data, responseDate); + completionHandler(nil, data, responseDate, cacheControl); }; // Download image @@ -498,7 +512,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, } else { someError = RCTErrorWithMessage(@"Unknown image download error"); } - completionHandler(someError, nil, nil); + completionHandler(someError, nil, nil, nil); [strongSelf dequeueTasks]; return; } @@ -564,7 +578,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, }; __weak RCTImageLoader *weakSelf = self; - void (^completionHandler)(NSError *, id, BOOL, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate) { + void (^completionHandler)(NSError *, id, BOOL, NSString *, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl) { __typeof(self) strongSelf = weakSelf; if (atomic_load(&cancelled) || !strongSelf) { return; @@ -576,20 +590,6 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, return; } - // Check decoded image cache - if (cacheResult) { - UIImage *image = [[strongSelf imageCache] imageForUrl:imageURLRequest.URL.absoluteString - size:size - scale:scale - resizeMode:resizeMode - responseDate:fetchDate]; - if (image) { - cancelLoad = nil; - completionBlock(nil, image); - return; - } - } - RCTImageLoaderCompletionBlock decodeCompletionHandler = ^(NSError *error_, UIImage *image) { if (cacheResult && image) { // Store decoded image in cache @@ -598,7 +598,8 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, size:size scale:scale resizeMode:resizeMode - responseDate:fetchDate]; + responseDate:fetchDate + cacheControl:cacheControl]; } cancelLoad = nil; @@ -732,7 +733,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, - (RCTImageLoaderCancellationBlock)getImageSizeForURLRequest:(NSURLRequest *)imageURLRequest block:(void(^)(NSError *error, CGSize size))callback { - void (^completion)(NSError *, id, BOOL, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate) { + void (^completion)(NSError *, id, BOOL, NSString *, NSString *) = ^(NSError *error, id imageOrData, BOOL cacheResult, NSString *fetchDate, NSString *cacheControl) { CGSize size; if ([imageOrData isKindOfClass:[NSData class]]) { NSDictionary *meta = RCTGetImageMetadata(imageOrData);