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
This commit is contained in:
Garrett Moon 2017-12-04 14:56:04 -08:00 committed by GitHub
parent c1f517a7eb
commit 0b6d41f872
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 64 additions and 46 deletions

View File

@ -55,6 +55,7 @@ NSString *const ASAnimatedImageDefaultRunLoopMode = NSRunLoopCommonModes;
} }
id <ASAnimatedImageProtocol> previousAnimatedImage = _animatedImage; id <ASAnimatedImageProtocol> previousAnimatedImage = _animatedImage;
_animatedImage = animatedImage; _animatedImage = animatedImage;
if (animatedImage != nil) { if (animatedImage != nil) {
@ -80,6 +81,11 @@ NSString *const ASAnimatedImageDefaultRunLoopMode = NSRunLoopCommonModes;
} }
[self animatedImageSet:_animatedImage previousAnimatedImage:previousAnimatedImage]; [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 <ASAnimatedImageProtocol>)newAnimatedImage previousAnimatedImage:(id <ASAnimatedImageProtocol>)previousAnimatedImage - (void)animatedImageSet:(id <ASAnimatedImageProtocol>)newAnimatedImage previousAnimatedImage:(id <ASAnimatedImageProtocol>)previousAnimatedImage

View File

@ -713,55 +713,67 @@
} else { } else {
__weak __typeof__(self) weakSelf = self; __weak __typeof__(self) weakSelf = self;
auto finished = ^(id <ASImageContainerProtocol>imageContainer, NSError *error, id downloadIdentifier, ASNetworkImageSource imageSource) { auto finished = ^(id <ASImageContainerProtocol>imageContainer, NSError *error, id downloadIdentifier, ASNetworkImageSource imageSource) {
ASPerformBlockOnBackgroundThread(^{
__typeof__(self) strongSelf = weakSelf; __typeof__(self) strongSelf = weakSelf;
if (strongSelf == nil) { if (strongSelf == nil) {
return; 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;
}
//No longer in preload range, no point in setting the results (they won't be cleared in exit preload range) as_log_verbose(ASImageLoadingLog(), "Downloaded image for %@ img: %@ urls: %@", self, [imageContainer asdk_image], URLs);
if (ASInterfaceStateIncludesPreload(self->_interfaceState) == NO) {
return; // Grab the lock for the rest of the block
} ASDN::MutexLocker l(strongSelf->__instanceLock__);
if (imageContainer != nil) { //Getting a result back for a different download identifier, download must not have been successfully canceled
[strongSelf _locked_setCurrentImageQuality:1.0]; if (ASObjectIsEqual(strongSelf->_downloadIdentifier, downloadIdentifier) == NO && downloadIdentifier != nil) {
if ([imageContainer asdk_animatedImageData] && strongSelf->_downloaderFlags.downloaderImplementsAnimatedImage) { return;
id animatedImage = [strongSelf->_downloader animatedImageWithData:[imageContainer asdk_animatedImageData]];
[strongSelf _locked_setAnimatedImage:animatedImage];
} else {
[strongSelf _locked__setImage:[imageContainer asdk_image]];
} }
strongSelf->_imageLoaded = YES;
} //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) {
strongSelf->_downloadIdentifier = nil; return;
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];
} }
} else if (error && strongSelf->_delegateFlags.delegateDidFailWithError) {
ASDN::MutexUnlocker u(strongSelf->__instanceLock__); if (imageContainer != nil) {
[delegate imageNode:strongSelf didFailWithError:error]; [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 // As the _cache and _downloader is only set once in the intializer we don't have to use a