From 3677e290aa3d84a3fe511cc7aa0b0ff6777e3ed0 Mon Sep 17 00:00:00 2001 From: Garrett Moon Date: Fri, 4 May 2018 12:21:42 -0700 Subject: [PATCH] Issue ASNetworkImageNode callbacks off main thread (#908) * The main thread is under a bunch of contention on startup, let's avoid using it. * Update CHANGELOG --- CHANGELOG.md | 1 + Source/ASExperimentalFeatures.h | 1 + Source/ASExperimentalFeatures.m | 3 ++- Source/ASNetworkImageNode.mm | 24 +++++++++++++++++++----- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b171e8d2f..8e79617208 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ - Introduces `ASRecursiveUnfairLock` as an experiment to improve locking performance. [Adlai Holler](https://github.com/Adlai-Holler) - Adds an experiment to shorten init time. [Adlai Holler](https://github.com/Adlai-Holler) - Adds a check that Texture is compiled with stdc++11 as specified by the podfile. gnu++11 can cause subtle issues that are currently being investigated. [Adlai Holler](https://github.com/Adlai-Holler) +- Adds an experiment to call ASNetworkImageNode callbacks off main. [Garrett Moon](https://github.com/garrettmoon) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h index 8b20c22692..12ee2081f7 100644 --- a/Source/ASExperimentalFeatures.h +++ b/Source/ASExperimentalFeatures.h @@ -25,6 +25,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) { ASExperimentalInterfaceStateCoalescing = 1 << 2, // exp_interface_state_coalesce ASExperimentalUnfairLock = 1 << 3, // exp_unfair_lock ASExperimentalLayerDefaults = 1 << 4, // exp_infer_layer_defaults + ASExperimentalNetworkImageQueue = 1 << 5, // exp_network_image_queue ASExperimentalFeatureAll = 0xFFFFFFFF }; diff --git a/Source/ASExperimentalFeatures.m b/Source/ASExperimentalFeatures.m index 8cfb0fb6c1..3636cd0e8e 100644 --- a/Source/ASExperimentalFeatures.m +++ b/Source/ASExperimentalFeatures.m @@ -18,7 +18,8 @@ NSArray *ASExperimentalFeaturesGetNames(ASExperimentalFeatures flags @"exp_text_node", @"exp_interface_state_coalesce", @"exp_unfair_lock", - @"exp_infer_layer_defaults"])); + @"exp_infer_layer_defaults", + @"exp_network_image_queue"])); if (flags == ASExperimentalFeatureAll) { return allNames; diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm index 76eaf7c21e..edb462e451 100755 --- a/Source/ASNetworkImageNode.mm +++ b/Source/ASNetworkImageNode.mm @@ -124,6 +124,20 @@ [self _cancelImageDownloadWithResumePossibility:NO]; } +- (dispatch_queue_t)callbackQueue +{ + static dispatch_once_t onceToken; + static dispatch_queue_t callbackQueue; + dispatch_once(&onceToken, ^{ + if (ASActivateExperimentalFeature(ASExperimentalNetworkImageQueue)) { + callbackQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0); + } else { + callbackQueue = dispatch_get_main_queue(); + } + }); + return callbackQueue; +} + #pragma mark - Public methods -- must lock /// Setter for public image property. It has the side effect of setting an internal _imageWasSetExternally that prevents setting an image internally. Setting an image internally should happen with the _setImage: method @@ -455,7 +469,7 @@ // Unbind from the previous download. if (oldDownloadIDForProgressBlock != nil) { as_log_verbose(ASImageLoadingLog(), "Disabled progress images for %@ id: %@", self, oldDownloadIDForProgressBlock); - [_downloader setProgressImageBlock:nil callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:oldDownloadIDForProgressBlock]; + [_downloader setProgressImageBlock:nil callbackQueue:[self callbackQueue] withDownloadIdentifier:oldDownloadIDForProgressBlock]; } // Bind to the current download. @@ -464,7 +478,7 @@ as_log_verbose(ASImageLoadingLog(), "Enabled progress images for %@ id: %@", self, newDownloadIDForProgressBlock); [_downloader setProgressImageBlock:^(UIImage * _Nonnull progressImage, CGFloat progress, id _Nullable downloadIdentifier) { [weakSelf handleProgressImage:progressImage progress:progress downloadIdentifier:downloadIdentifier]; - } callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:newDownloadIDForProgressBlock]; + } callbackQueue:[self callbackQueue] withDownloadIdentifier:newDownloadIDForProgressBlock]; } // Update state local state with lock held. @@ -484,7 +498,7 @@ // In this case another thread changed the _downloadIdentifierForProgressBlock before we finished registering // the new progress block for newDownloadIDForProgressBlock ID. Let's clear it now and reattempt to register if (newDownloadIDForProgressBlock) { - [_downloader setProgressImageBlock:nil callbackQueue:dispatch_get_main_queue() withDownloadIdentifier:newDownloadIDForProgressBlock]; + [_downloader setProgressImageBlock:nil callbackQueue:[self callbackQueue] withDownloadIdentifier:newDownloadIDForProgressBlock]; } [self _updateProgressImageBlockOnDownloaderIfNeeded]; } @@ -556,7 +570,7 @@ downloadIdentifier = [_downloader downloadImageWithURL:url - callbackQueue:dispatch_get_main_queue() + callbackQueue:[self callbackQueue] downloadProgress:NULL completion:^(id _Nullable imageContainer, NSError * _Nullable error, id _Nullable downloadIdentifier, id _Nullable userInfo) { if (finished != NULL) { @@ -757,7 +771,7 @@ } }; [_cache cachedImageWithURL:URL - callbackQueue:dispatch_get_main_queue() + callbackQueue:[self callbackQueue] completion:completion]; } else { [self _downloadImageWithCompletion:^(id imageContainer, NSError *error, id downloadIdentifier, id userInfo) {