From eac85b6c9a6dbf89d8b310bd8549b72734ecc404 Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Sun, 28 Feb 2016 20:54:56 -0800 Subject: [PATCH] [ASDisplayNode] Optimize -setNeedsDisplay, deep mutable array copies. These optimizations are surprisingly impactful. -setNeedsDisplay being called for every node triggered cancelAsyncDisplay, locking, and memory management overhead that is completely avoidable because Core Animation triggers first display automatically. The mutable array copy optimizations reduced this key cost by over 10x, from 52ms to 5ms on an iPad Air 2 / A8X with a real-world test case. --- AsyncDisplayKit/Details/ASDataController.mm | 6 +- AsyncDisplayKit/Details/_ASDisplayLayer.mm | 66 ++++++++++--------- .../Private/ASDisplayNode+UIViewBridge.mm | 7 +- .../Private/ASMultidimensionalArrayUtils.h | 10 ++- .../Private/ASMultidimensionalArrayUtils.mm | 10 ++- 5 files changed, 60 insertions(+), 39 deletions(-) diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index 32d62adac2..2a6075106b 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -248,7 +248,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; _editingNodes[kind] = editingNodes; // Deep copy is critical here, or future edits to the sub-arrays will pollute state between _editing and _complete on different threads. - NSMutableArray *completedNodes = (NSMutableArray *)ASMultidimensionalArrayDeepMutableCopy(editingNodes); + NSMutableArray *completedNodes = ASTwoDimensionalArrayDeepMutableCopy(editingNodes); [_mainSerialQueue performBlockOnMainThread:^{ _completedNodes[kind] = completedNodes; @@ -290,7 +290,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [_editingNodes[kind] insertObjects:sections atIndexes:indexSet]; // Deep copy is critical here, or future edits to the sub-arrays will pollute state between _editing and _complete on different threads. - NSArray *sectionsForCompleted = (NSMutableArray *)ASMultidimensionalArrayDeepMutableCopy(sections); + NSArray *sectionsForCompleted = ASTwoDimensionalArrayDeepMutableCopy(sections); [_mainSerialQueue performBlockOnMainThread:^{ [_completedNodes[kind] insertObjects:sectionsForCompleted atIndexes:indexSet]; @@ -552,7 +552,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [_mainSerialQueue performBlockOnMainThread:^{ // Deep copy _completedNodes to _externalCompletedNodes. // Any external queries from now on will be done on _externalCompletedNodes, to guarantee data consistency with the delegate. - _externalCompletedNodes = (NSMutableArray *)ASMultidimensionalArrayDeepMutableCopy(_completedNodes[ASDataControllerRowNodeKind]); + _externalCompletedNodes = ASTwoDimensionalArrayDeepMutableCopy(_completedNodes[ASDataControllerRowNodeKind]); LOG(@"endUpdatesWithCompletion - begin updates call to delegate"); [_delegate dataControllerBeginUpdates:self]; diff --git a/AsyncDisplayKit/Details/_ASDisplayLayer.mm b/AsyncDisplayKit/Details/_ASDisplayLayer.mm index 110336be97..fd1b9b1fcb 100644 --- a/AsyncDisplayKit/Details/_ASDisplayLayer.mm +++ b/AsyncDisplayKit/Details/_ASDisplayLayer.mm @@ -57,12 +57,6 @@ _asyncDelegate = asyncDelegate; } -- (void)setContents:(id)contents -{ - ASDisplayNodeAssertMainThread(); - [super setContents:contents]; -} - - (BOOL)isDisplaySuspended { ASDN::MutexLocker l(_displaySuspendedLock); @@ -84,6 +78,20 @@ } } +#if DEBUG // These override is strictly to help detect application-level threading errors. Avoid method overhead in release. +- (void)setContents:(id)contents +{ + ASDisplayNodeAssertMainThread(); + [super setContents:contents]; +} + +- (void)setNeedsLayout +{ + ASDisplayNodeAssertMainThread(); + [super setNeedsLayout]; +} +#endif + - (void)layoutSublayers { [super layoutSublayers]; @@ -99,17 +107,12 @@ } } -- (void)setNeedsLayout -{ - ASDisplayNodeAssertMainThread(); - [super setNeedsLayout]; -} - - (void)setNeedsDisplay { ASDisplayNodeAssertMainThread(); - ASDN::MutexLocker l(_displaySuspendedLock); + _displaySuspendedLock.lock(); + // FIXME: Reconsider whether we should cancel a display in progress. // We should definitely cancel a display that is scheduled, but unstarted display. [self cancelAsyncDisplay]; @@ -118,6 +121,7 @@ if (!_displaySuspended) { [super setNeedsDisplay]; } + _displaySuspendedLock.unlock(); } #pragma mark - @@ -178,18 +182,29 @@ - (void)display:(BOOL)asynchronously { - [self _performBlockWithAsyncDelegate:^(id<_ASDisplayLayerDelegate> asyncDelegate) { - [asyncDelegate displayAsyncLayer:self asynchronously:asynchronously]; - }]; + id<_ASDisplayLayerDelegate> __attribute__((objc_precise_lifetime)) strongAsyncDelegate; + { + _asyncDelegateLock.lock(); + strongAsyncDelegate = _asyncDelegate; + _asyncDelegateLock.unlock(); + } + + [strongAsyncDelegate displayAsyncLayer:self asynchronously:asynchronously]; } - (void)cancelAsyncDisplay { ASDisplayNodeAssertMainThread(); [_displaySentinel increment]; - [self _performBlockWithAsyncDelegate:^(id<_ASDisplayLayerDelegate> asyncDelegate) { - [asyncDelegate cancelDisplayAsyncLayer:self]; - }]; + + id<_ASDisplayLayerDelegate> __attribute__((objc_precise_lifetime)) strongAsyncDelegate; + { + _asyncDelegateLock.lock(); + strongAsyncDelegate = _asyncDelegate; + _asyncDelegateLock.unlock(); + } + + [strongAsyncDelegate cancelDisplayAsyncLayer:self]; } - (NSString *)description @@ -199,17 +214,4 @@ return [NSString stringWithFormat:@"<%@, layer = %@>", self.asyncdisplaykit_node, [super description]]; } -#pragma mark - -#pragma mark Helper Methods - -- (void)_performBlockWithAsyncDelegate:(void(^)(id<_ASDisplayLayerDelegate> asyncDelegate))block -{ - id<_ASDisplayLayerDelegate> __attribute__((objc_precise_lifetime)) strongAsyncDelegate; - { - ASDN::MutexLocker l(_asyncDelegateLock); - strongAsyncDelegate = _asyncDelegate; - } - block(strongAsyncDelegate); -} - @end diff --git a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm index e947ea8ed5..b3b69595f4 100644 --- a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm +++ b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm @@ -321,7 +321,12 @@ if (shouldApply) { _layer.layerProperty = (layerValueExpr); } else { ASDisplayNo // which may call -displayIfNeeded. We want to ensure the needsDisplay flag is set now, and then cleared. _messageToViewOrLayer(setNeedsDisplay); } else { - [ASDisplayNodeGetPendingState(self) setNeedsDisplay]; + if (__loaded(self)) { + // In this case, the node is loaded but we are on a background thread. Apply setNeedsDisplay so it is communicated + // to the backing view / layer in the next main thread flush. If the view / layer are not loaded, then + // Core Animation automatically considers them as "needsDisplay" if applicable when they are added to the hierarchy. + [ASDisplayNodeGetPendingState(self) setNeedsDisplay]; + } } [self __setNeedsDisplay]; } diff --git a/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.h b/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.h index d5058a8622..432df556e6 100644 --- a/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.h +++ b/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.h @@ -18,8 +18,14 @@ ASDISPLAYNODE_EXTERN_C_BEGIN /** - * Deep muutable copy of multidimensional array. - * It will recursively do the multiple copy for each subarray. + * Deep mutable copy of an array that contains arrays, which contain objects. It will go one level deep into the array to copy. + * This method is substantially faster than the generalized version, e.g. about 10x faster, so use it whenever it fits the need. + */ +extern NSMutableArray *ASTwoDimensionalArrayDeepMutableCopy(NSArray *array); + +/** + * Deep mutable copy of multidimensional array. This is completely generalized and supports copying mixed-depth arrays, + * where some subarrays might contain both elements and other subarrays. It will recursively do the multiple copy for each subarray. */ extern NSObject *ASMultidimensionalArrayDeepMutableCopy(NSObject *obj); diff --git a/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.mm b/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.mm index af0f4991ef..2f40e9d21a 100644 --- a/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.mm +++ b/AsyncDisplayKit/Private/ASMultidimensionalArrayUtils.mm @@ -54,7 +54,7 @@ static void ASRecursivelyFindIndexPathsForMultidimensionalArray(NSObject *obj, N NSObject *ASMultidimensionalArrayDeepMutableCopy(NSObject *obj) { if ([obj isKindOfClass:[NSArray class]]) { NSArray *arr = (NSArray *)obj; - NSMutableArray * mutableArr = [NSMutableArray array]; + NSMutableArray * mutableArr = [NSMutableArray arrayWithCapacity:arr.count]; for (NSObject *elem in arr) { [mutableArr addObject:ASMultidimensionalArrayDeepMutableCopy(elem)]; } @@ -64,6 +64,14 @@ NSObject *ASMultidimensionalArrayDeepMutableCopy(NSObject return obj; } +NSMutableArray *ASTwoDimensionalArrayDeepMutableCopy(NSArray *array) { + NSMutableArray *newArray = [NSMutableArray arrayWithCapacity:array.count]; + for (NSArray *subarray in array) { + [newArray addObject:[subarray mutableCopy]]; + } + return newArray; +} + void ASInsertElementsIntoMultidimensionalArrayAtIndexPaths(NSMutableArray *mutableArray, NSArray *indexPaths, NSArray *elements) { ASDisplayNodeCAssert(indexPaths.count == elements.count, @"Inconsistent indexPaths and elements");