From 0b6d41f8726c80653c36ea41bf6dd55d2a56efd3 Mon Sep 17 00:00:00 2001 From: Garrett Moon Date: Mon, 4 Dec 2017 14:56:04 -0800 Subject: [PATCH] A couple performance tweaks for animated images #trivial (#634) * A couple performance tweaks for animated images * @nguyenhuy's comments * Avoid calling animatedImageData twice. Thanks @maicki. * Fix call to background deallocation * Good catch by @Adlai-Holler --- Source/ASImageNode+AnimatedImage.mm | 6 ++ Source/ASNetworkImageNode.mm | 104 ++++++++++++++++------------ 2 files changed, 64 insertions(+), 46 deletions(-) diff --git a/Source/ASImageNode+AnimatedImage.mm b/Source/ASImageNode+AnimatedImage.mm index 355ab472ef..246ee49cc2 100644 --- a/Source/ASImageNode+AnimatedImage.mm +++ b/Source/ASImageNode+AnimatedImage.mm @@ -55,6 +55,7 @@ NSString *const ASAnimatedImageDefaultRunLoopMode = NSRunLoopCommonModes; } id previousAnimatedImage = _animatedImage; + _animatedImage = animatedImage; if (animatedImage != nil) { @@ -80,6 +81,11 @@ NSString *const ASAnimatedImageDefaultRunLoopMode = NSRunLoopCommonModes; } [self animatedImageSet:_animatedImage previousAnimatedImage:previousAnimatedImage]; + + // Animated image can take while to dealloc, do it off the main queue + if (previousAnimatedImage != nil) { + ASPerformBackgroundDeallocation(&previousAnimatedImage); + } } - (void)animatedImageSet:(id )newAnimatedImage previousAnimatedImage:(id )previousAnimatedImage diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm index 0a7ee0fbde..6b4a40a891 100755 --- a/Source/ASNetworkImageNode.mm +++ b/Source/ASNetworkImageNode.mm @@ -713,55 +713,67 @@ } else { __weak __typeof__(self) weakSelf = self; auto finished = ^(id imageContainer, NSError *error, id downloadIdentifier, ASNetworkImageSource imageSource) { - - __typeof__(self) strongSelf = weakSelf; - if (strongSelf == nil) { - return; - } - - as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs); - - // Grab the lock for the rest of the block - ASDN::MutexLocker l(strongSelf->__instanceLock__); - - //Getting a result back for a different download identifier, download must not have been successfully canceled - if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { - return; - } + ASPerformBlockOnBackgroundThread(^{ + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } - //No longer in preload range, no point in setting the results (they won't be cleared in exit preload range) - if (ASInterfaceStateIncludesPreload(self->_interfaceState) == NO) { - return; - } - - if (imageContainer != nil) { - [strongSelf _locked_setCurrentImageQuality:1.0]; - if ([imageContainer asdk_animatedImageData] && strongSelf->_downloaderFlags.downloaderImplementsAnimatedImage) { - id animatedImage = [strongSelf->_downloader animatedImageWithData:[imageContainer asdk_animatedImageData]]; - [strongSelf _locked_setAnimatedImage:animatedImage]; - } else { - [strongSelf _locked__setImage:[imageContainer asdk_image]]; + as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs); + + // Grab the lock for the rest of the block + ASDN::MutexLocker l(strongSelf->__instanceLock__); + + //Getting a result back for a different download identifier, download must not have been successfully canceled + if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) { + return; } - strongSelf->_imageLoaded = YES; - } - - strongSelf->_downloadIdentifier = nil; - strongSelf->_cacheUUID = nil; - - if (imageContainer != nil) { - if (strongSelf->_delegateFlags.delegateDidLoadImageWithInfo) { - ASDN::MutexUnlocker u(strongSelf->__instanceLock__); - ASNetworkImageNodeDidLoadInfo info = {}; - info.imageSource = imageSource; - [delegate imageNode:strongSelf didLoadImage:strongSelf.image info:info]; - } else if (strongSelf->_delegateFlags.delegateDidLoadImage) { - ASDN::MutexUnlocker u(strongSelf->__instanceLock__); - [delegate imageNode:strongSelf didLoadImage:strongSelf.image]; + + //No longer in preload range, no point in setting the results (they won't be cleared in exit preload range) + if (ASInterfaceStateIncludesPreload(self->_interfaceState) == NO) { + return; } - } else if (error && strongSelf->_delegateFlags.delegateDidFailWithError) { - ASDN::MutexUnlocker u(strongSelf->__instanceLock__); - [delegate imageNode:strongSelf didFailWithError:error]; - } + + if (imageContainer != nil) { + [strongSelf _locked_setCurrentImageQuality:1.0]; + NSData *animatedImageData = [imageContainer asdk_animatedImageData]; + if (animatedImageData && strongSelf->_downloaderFlags.downloaderImplementsAnimatedImage) { + id animatedImage = [strongSelf->_downloader animatedImageWithData:animatedImageData]; + [strongSelf _locked_setAnimatedImage:animatedImage]; + } else { + [strongSelf _locked__setImage:[imageContainer asdk_image]]; + } + strongSelf->_imageLoaded = YES; + } + + strongSelf->_downloadIdentifier = nil; + strongSelf->_cacheUUID = nil; + + ASPerformBlockOnMainThread(^{ + __typeof__(self) strongSelf = weakSelf; + if (strongSelf == nil) { + return; + } + + // Grab the lock for the rest of the block + ASDN::MutexLocker l(strongSelf->__instanceLock__); + + if (imageContainer != nil) { + if (strongSelf->_delegateFlags.delegateDidLoadImageWithInfo) { + ASDN::MutexUnlocker u(strongSelf->__instanceLock__); + ASNetworkImageNodeDidLoadInfo info = {}; + info.imageSource = imageSource; + [delegate imageNode:strongSelf didLoadImage:strongSelf.image info:info]; + } else if (strongSelf->_delegateFlags.delegateDidLoadImage) { + ASDN::MutexUnlocker u(strongSelf->__instanceLock__); + [delegate imageNode:strongSelf didLoadImage:strongSelf.image]; + } + } else if (error && strongSelf->_delegateFlags.delegateDidFailWithError) { + ASDN::MutexUnlocker u(strongSelf->__instanceLock__); + [delegate imageNode:strongSelf didFailWithError:error]; + } + }); + }); }; // As the _cache and _downloader is only set once in the intializer we don't have to use a