From e6e7cd8dd841c6abd7d3dcc7fad18cd99fb49f20 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Tue, 26 Feb 2019 21:49:28 -0800 Subject: [PATCH 1/3] Optimize _assertSubnodeState This method is actually pretty painful in today's world. In one iPad mini trace, the first page of nodes spent 6.6ms in this call, just in time profiler. --- Source/ASDisplayNode+Layout.mm | 50 ++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 9fbecf39f..58d04f71a 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -8,6 +8,7 @@ // #import +#import #import #import #import @@ -18,6 +19,7 @@ #import #import #import +#import #pragma mark - ASDisplayNode (ASLayoutElement) @@ -926,35 +928,35 @@ - (void)_assertSubnodeState return; } - NSArray *subnodes = [self subnodes]; - NSArray *sublayouts = _calculatedDisplayNodeLayout.layout.sublayouts; - - const auto currentSubnodes = [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality - capacity:subnodes.count]; - const auto layoutSubnodes = [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality - capacity:sublayouts.count];; - for (ASDisplayNode *subnode in subnodes) { - [currentSubnodes addObject:subnode]; + ASDN::MutexLocker l(__instanceLock__); + NSArray *sublayouts = _calculatedDisplayNodeLayout.layout.sublayouts; + unowned ASLayout *cSublayouts[sublayouts.count]; + [sublayouts getObjects:cSublayouts]; + NSUInteger i = 0; + BOOL matches = YES; + for (ASDisplayNode *subnode in _subnodes) { + if (subnode != cSublayouts[i].layoutElement) { + matches = NO; + } } - - for (ASLayout *sublayout in sublayouts) { - id layoutElement = sublayout.layoutElement; - ASDisplayNodeAssert([layoutElement isKindOfClass:[ASDisplayNode class]], - @"All calculatedLayouts should be flattened and only contain nodes!"); - [layoutSubnodes addObject:(ASDisplayNode *)layoutElement]; + if (matches) { + return; } - // Verify that all subnodes that occur in the current ASLayout tree are present in .subnodes array. - if ([layoutSubnodes isSubsetOfHashTable:currentSubnodes] == NO) { - // Note: This should be converted to an assertion after confirming it is rare. - NSLog(@"Warning: node's layout includes subnodes that have not been added: node = %@, subnodes = %@, subnodes in layout = %@", self, currentSubnodes, layoutSubnodes); + NSArray *layoutNodes = ASArrayByFlatMapping(sublayouts, ASLayout *layout, (ASDisplayNode *)layout.layoutElement); + NSIndexSet *insertions, *deletions; + [_subnodes asdk_diffWithArray:layoutNodes insertions:&insertions deletions:&deletions]; + if (insertions.count > 0) { + NSLog(@"Warning: node's layout includes subnode that has not been added: node = %@, subnodes = %@, subnodes in layout = %@", self, _subnodes, layoutNodes); } - // Verify that everything in the .subnodes array is present in the ASLayout tree (and correct it if not). - [currentSubnodes minusHashTable:layoutSubnodes]; - for (ASDisplayNode *orphanedSubnode in currentSubnodes) { - NSLog(@"Automatically removing orphaned subnode %@, from parent %@", orphanedSubnode, self); - [orphanedSubnode removeFromSupernode]; + // Remove any nodes that are in the tree but should not be. + // Go in reverse order so we don't shift our indexes. + if (deletions) { + for (NSUInteger i = deletions.lastIndex; i != NSNotFound; i = [deletions indexLessThanIndex:i]) { + NSLog(@"Automatically removing orphaned subnode %@, from parent %@", _subnodes[i], self); + [_subnodes[i] removeFromSupernode]; + } } } From f3833906f5bf8c0e0e832d6081e27e6222c0c901 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sun, 3 Mar 2019 09:35:34 -0600 Subject: [PATCH 2/3] Clean it up, check count --- Source/ASDisplayNode+Layout.mm | 23 ++++++++++++++--------- Source/Base/ASBaseDefines.h | 2 ++ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 58d04f71a..3a78785ed 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -931,16 +931,21 @@ - (void)_assertSubnodeState ASDN::MutexLocker l(__instanceLock__); NSArray *sublayouts = _calculatedDisplayNodeLayout.layout.sublayouts; unowned ASLayout *cSublayouts[sublayouts.count]; - [sublayouts getObjects:cSublayouts]; - NSUInteger i = 0; - BOOL matches = YES; - for (ASDisplayNode *subnode in _subnodes) { - if (subnode != cSublayouts[i].layoutElement) { - matches = NO; + [sublayouts getObjects:cSublayouts range:NSMakeRange(0, AS_ARRAY_SIZE(cSublayouts))]; + + // Fast-path if we are in the correct state (likely). + if (sublayouts.count == AS_ARRAY_SIZE(cSublayouts)) { + NSUInteger i = 0; + BOOL matches = YES; + for (ASDisplayNode *subnode in _subnodes) { + if (subnode != cSublayouts[i].layoutElement) { + matches = NO; + } + i++; + } + if (matches) { + return; } - } - if (matches) { - return; } NSArray *layoutNodes = ASArrayByFlatMapping(sublayouts, ASLayout *layout, (ASDisplayNode *)layout.layoutElement); diff --git a/Source/Base/ASBaseDefines.h b/Source/Base/ASBaseDefines.h index ce0e891e6..4e880f902 100644 --- a/Source/Base/ASBaseDefines.h +++ b/Source/Base/ASBaseDefines.h @@ -147,6 +147,8 @@ #define AS_SUBCLASSING_RESTRICTED #endif +#define AS_ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0])) + #define ASCreateOnce(expr) ({ \ static dispatch_once_t onceToken; \ static __typeof__(expr) staticVar; \ From fdd3a9125b05bc75e21d896df01c77e72f9a60a5 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sun, 3 Mar 2019 12:51:37 -0600 Subject: [PATCH 3/3] Check the right value --- Source/ASDisplayNode+Layout.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 3a78785ed..0d45c4140 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -934,7 +934,7 @@ - (void)_assertSubnodeState [sublayouts getObjects:cSublayouts range:NSMakeRange(0, AS_ARRAY_SIZE(cSublayouts))]; // Fast-path if we are in the correct state (likely). - if (sublayouts.count == AS_ARRAY_SIZE(cSublayouts)) { + if (_subnodes.count == AS_ARRAY_SIZE(cSublayouts)) { NSUInteger i = 0; BOOL matches = YES; for (ASDisplayNode *subnode in _subnodes) {