From 66278c51ae6f77b8e6e4c225f34275f9f10d18e0 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 22 Jan 2018 14:35:53 -0500 Subject: [PATCH] Revert "Faster collection operations (#748)" This reverts commit 5c13403ef75c030adc7a4d51a7792a9c6c1c348b. --- .../xcschemes/AsyncDisplayKit.xcscheme | 1 - CHANGELOG.md | 1 - Source/ASCollectionView.mm | 39 +++++++-- Source/ASDisplayNode+Yoga.mm | 10 ++- Source/ASTableView.mm | 12 ++- Source/Base/ASBaseDefines.h | 83 +++++++------------ .../Details/ASCollectionFlowLayoutDelegate.m | 2 +- .../ASCollectionGalleryLayoutDelegate.mm | 6 +- Source/Details/ASElementMap.h | 5 -- Source/Details/ASElementMap.m | 5 -- Source/Layout/ASLayout.mm | 17 ++-- Source/Layout/ASLayoutSpec.mm | 10 ++- Source/Private/ASTwoDimensionalArrayUtils.m | 9 +- Source/_ASTransitionContext.m | 8 +- 14 files changed, 108 insertions(+), 100 deletions(-) diff --git a/AsyncDisplayKit.xcodeproj/xcshareddata/xcschemes/AsyncDisplayKit.xcscheme b/AsyncDisplayKit.xcodeproj/xcshareddata/xcschemes/AsyncDisplayKit.xcscheme index 5387aa675..8cf72597a 100644 --- a/AsyncDisplayKit.xcodeproj/xcshareddata/xcschemes/AsyncDisplayKit.xcscheme +++ b/AsyncDisplayKit.xcodeproj/xcshareddata/xcschemes/AsyncDisplayKit.xcscheme @@ -60,7 +60,6 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - disableMainThreadChecker = "YES" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" diff --git a/CHANGELOG.md b/CHANGELOG.md index e9351912f..1f6ea8c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,6 @@ - [ASCollectionNode] Added support for interactive item movement. [Adlai Holler](https://github.com/Adlai-Holler) - Added an experimental "no-copy" rendering API. See ASGraphicsContext.h for info. [Adlai Holler](https://github.com/Adlai-Holler) - Dropped support for iOS 8. [Adlai Holler](https://github.com/Adlai-Holler) -- Optimize internal collection operations. [Adlai Holler](https://github.com/Adlai-Holler) [#748](https://github.com/TextureGroup/Texture/pull/748) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index dfc5dde0c..1b06d6bef 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -719,9 +719,19 @@ - (NSIndexPath *)convertIndexPathToCollectionNode:(NSIndexPath *)indexPath - (NSArray *)convertIndexPathsToCollectionNode:(NSArray *)indexPaths { - return ASArrayByFlatMapping(indexPaths, NSIndexPath *indexPathInView, ({ - [self convertIndexPathToCollectionNode:indexPathInView]; - })); + if (indexPaths == nil) { + return nil; + } + + NSMutableArray *indexPathsArray = [NSMutableArray arrayWithCapacity:indexPaths.count]; + + for (NSIndexPath *indexPathInView in indexPaths) { + NSIndexPath *indexPath = [self convertIndexPathToCollectionNode:indexPathInView]; + if (indexPath != nil) { + [indexPathsArray addObject:indexPath]; + } + } + return indexPathsArray; } - (ASCellNode *)supplementaryNodeForElementKind:(NSString *)elementKind atIndexPath:(NSIndexPath *)indexPath @@ -737,10 +747,17 @@ - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode - (NSArray *)visibleNodes { NSArray *indexPaths = [self indexPathsForVisibleItems]; + NSMutableArray *visibleNodes = [[NSMutableArray alloc] init]; - return ASArrayByFlatMapping(indexPaths, NSIndexPath *indexPath, ({ - [self nodeForItemAtIndexPath:indexPath]; - })); + for (NSIndexPath *indexPath in indexPaths) { + ASCellNode *node = [self nodeForItemAtIndexPath:indexPath]; + if (node) { + // It is possible for UICollectionView to return indexPaths before the node is completed. + [visibleNodes addObject:node]; + } + } + + return visibleNodes; } - (BOOL)usesSynchronousDataLoading @@ -2159,9 +2176,13 @@ - (void)nodesDidRelayout:(NSArray *)nodes return; } - NSArray *uikitIndexPaths = ASArrayByFlatMapping(nodes, ASCellNode *node, ({ - [self indexPathForNode:node]; - })); + NSMutableArray *uikitIndexPaths = [NSMutableArray arrayWithCapacity:nodes.count]; + for (ASCellNode *node in nodes) { + NSIndexPath *uikitIndexPath = [self indexPathForNode:node]; + if (uikitIndexPath != nil) { + [uikitIndexPaths addObject:uikitIndexPath]; + } + } [_layoutFacilitator collectionViewWillEditCellsAtIndexPaths:uikitIndexPaths batched:NO]; diff --git a/Source/ASDisplayNode+Yoga.mm b/Source/ASDisplayNode+Yoga.mm index d4a0ba115..b52f94821 100644 --- a/Source/ASDisplayNode+Yoga.mm +++ b/Source/ASDisplayNode+Yoga.mm @@ -158,12 +158,14 @@ - (ASLayout *)layoutForYogaNode - (void)setupYogaCalculatedLayout { YGNodeRef yogaNode = self.style.yogaNode; - ASDisplayNodeAssert(YGNodeGetChildCount(yogaNode) == self.yogaChildren.count, + uint32_t childCount = YGNodeGetChildCount(yogaNode); + ASDisplayNodeAssert(childCount == self.yogaChildren.count, @"Yoga tree should always be in sync with .yogaNodes array! %@", self.yogaChildren); - NSArray *sublayouts = ASArrayByFlatMapping(self.yogaChildren, ASDisplayNode *subnode, ({ - [subnode layoutForYogaNode]; - })); + NSMutableArray *sublayouts = [NSMutableArray arrayWithCapacity:childCount]; + for (ASDisplayNode *subnode in self.yogaChildren) { + [sublayouts addObject:[subnode layoutForYogaNode]]; + } // The layout for self should have position CGPointNull, but include the calculated size. CGSize size = CGSizeMake(YGNodeLayoutGetWidth(yogaNode), YGNodeLayoutGetHeight(yogaNode)); diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index fc0a2dbbe..f78a688ea 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -607,9 +607,15 @@ - (NSIndexPath *)convertIndexPathToTableNode:(NSIndexPath *)indexPath return nil; } - return ASArrayByFlatMapping(indexPaths, NSIndexPath *indexPathInView, ({ - [self convertIndexPathToTableNode:indexPathInView]; - })); + NSMutableArray *indexPathsArray = [NSMutableArray new]; + + for (NSIndexPath *indexPathInView in indexPaths) { + NSIndexPath *indexPath = [self convertIndexPathToTableNode:indexPathInView]; + if (indexPath != nil) { + [indexPathsArray addObject:indexPath]; + } + } + return indexPathsArray; } - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode diff --git a/Source/Base/ASBaseDefines.h b/Source/Base/ASBaseDefines.h index 37749e53d..9eb1ec0b0 100755 --- a/Source/Base/ASBaseDefines.h +++ b/Source/Base/ASBaseDefines.h @@ -132,6 +132,22 @@ #define __has_attribute(x) 0 // Compatibility with non-clang compilers. #endif +#ifndef NS_CONSUMED +#if __has_feature(attribute_ns_consumed) +#define NS_CONSUMED __attribute__((ns_consumed)) +#else +#define NS_CONSUMED +#endif +#endif + +#ifndef NS_RETURNS_RETAINED +#if __has_feature(attribute_ns_returns_retained) +#define NS_RETURNS_RETAINED __attribute__((ns_returns_retained)) +#else +#define NS_RETURNS_RETAINED +#endif +#endif + #ifndef CF_RETURNS_RETAINED #if __has_feature(attribute_cf_returns_retained) #define CF_RETURNS_RETAINED __attribute__((cf_returns_retained)) @@ -229,38 +245,26 @@ * Create a new set by mapping `collection` over `work`, ignoring nil. */ #define ASSetByFlatMapping(collection, decl, work) ({ \ - CFTypeRef _cArray[collection.count]; \ - NSUInteger _i = 0; \ + NSMutableSet *s = [NSMutableSet set]; \ for (decl in collection) {\ - if ((_cArray[_i] = (__bridge_retained CFTypeRef)work)) { \ - _i++; \ + id result = work; \ + if (result != nil) { \ + [s addObject:result]; \ } \ } \ - NSSet *result; \ - if (_i == 0) { \ - /** Zero fast path. */ \ - result = [NSSet set]; \ - } else if (_i == 1) { \ - /** NSSingleObjectSet is fast. Create one and release. */ \ - CFTypeRef val = _cArray[0]; \ - result = [NSSet setWithObject:(__bridge id)val]; \ - CFBridgingRelease(val); \ - } else { \ - CFSetCallBacks cb = kCFTypeSetCallBacks; \ - cb.retain = NULL; \ - result = (__bridge NSSet *)CFSetCreate(kCFAllocatorDefault, _cArray, _i, &cb); \ - } \ - result; \ + s; \ }) /** * Create a new ObjectPointerPersonality NSHashTable by mapping `collection` over `work`, ignoring nil. */ #define ASPointerTableByFlatMapping(collection, decl, work) ({ \ - NSHashTable *t = [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality capacity:collection.count]; \ + NSHashTable *t = [NSHashTable hashTableWithOptions:NSHashTableObjectPointerPersonality]; \ for (decl in collection) {\ - /* NSHashTable accepts nil and avoid extra retain/release. */ \ - [t addObject:work]; \ + id result = work; \ + if (result != nil) { \ + [t addObject:result]; \ + } \ } \ t; \ }) @@ -269,37 +273,12 @@ * Create a new array by mapping `collection` over `work`, ignoring nil. */ #define ASArrayByFlatMapping(collection, decl, work) ({ \ - CFTypeRef _cArray[collection.count]; \ - NSUInteger _i = 0; \ - for (decl in collection) {\ - if ((_cArray[_i] = (__bridge_retained CFTypeRef)work)) { \ - _i++; \ - } \ - } \ - NSArray *result; \ - if (_i == 0) { \ - /** Zero array fast path. */ \ - result = @[]; \ - } else if (_i == 1) { \ - /** NSSingleObjectArray is fast. Create one and release. */ \ - CFTypeRef val = _cArray[0]; \ - result = [NSArray arrayWithObject:(__bridge id)val]; \ - CFBridgingRelease(val); \ - } else { \ - CFArrayCallBacks cb = kCFTypeArrayCallBacks; \ - cb.retain = NULL; \ - result = (__bridge NSArray *)CFArrayCreate(kCFAllocatorDefault, _cArray, _i, &cb); \ - } \ - result; \ -}) - -#define ASMutableArrayByFlatMapping(collection, decl, work) ({ \ - id _cArray[collection.count]; \ - NSUInteger _i = 0; \ + NSMutableArray *a = [NSMutableArray array]; \ for (decl in collection) {\ - if ((_cArray[_i] = work)) { \ - _i++; \ + id result = work; \ + if (result != nil) { \ + [a addObject:result]; \ } \ } \ - [[NSMutableArray alloc] initWithObjects:_cArray count:_i]; \ + a; \ }) diff --git a/Source/Details/ASCollectionFlowLayoutDelegate.m b/Source/Details/ASCollectionFlowLayoutDelegate.m index 77e092226..548fa0181 100644 --- a/Source/Details/ASCollectionFlowLayoutDelegate.m +++ b/Source/Details/ASCollectionFlowLayoutDelegate.m @@ -59,7 +59,7 @@ - (id)additionalInfoForLayoutWithElements:(ASElementMap *)elements + (ASCollectionLayoutState *)calculateLayoutWithContext:(ASCollectionLayoutContext *)context { ASElementMap *elements = context.elements; - NSArray *children = ASArrayByFlatMapping(elements.itemElements, ASCollectionElement *element, element.node); + NSMutableArray *children = ASArrayByFlatMapping(elements.itemElements, ASCollectionElement *element, element.node); if (children.count == 0) { return [[ASCollectionLayoutState alloc] initWithContext:context]; } diff --git a/Source/Details/ASCollectionGalleryLayoutDelegate.mm b/Source/Details/ASCollectionGalleryLayoutDelegate.mm index 482123c51..2734e9649 100644 --- a/Source/Details/ASCollectionGalleryLayoutDelegate.mm +++ b/Source/Details/ASCollectionGalleryLayoutDelegate.mm @@ -102,9 +102,9 @@ + (ASCollectionLayoutState *)calculateLayoutWithContext:(ASCollectionLayoutConte return [[ASCollectionLayoutState alloc] initWithContext:context]; } - NSArray<_ASGalleryLayoutItem *> *children = ASArrayByFlatMapping(elements.itemElements, - ASCollectionElement *element, - [[_ASGalleryLayoutItem alloc] initWithItemSize:itemSize collectionElement:element]); + NSMutableArray<_ASGalleryLayoutItem *> *children = ASArrayByFlatMapping(elements.itemElements, + ASCollectionElement *element, + [[_ASGalleryLayoutItem alloc] initWithItemSize:itemSize collectionElement:element]); if (children.count == 0) { return [[ASCollectionLayoutState alloc] initWithContext:context]; } diff --git a/Source/Details/ASElementMap.h b/Source/Details/ASElementMap.h index a73f80a92..eed7eff26 100644 --- a/Source/Details/ASElementMap.h +++ b/Source/Details/ASElementMap.h @@ -31,11 +31,6 @@ NS_ASSUME_NONNULL_BEGIN AS_SUBCLASSING_RESTRICTED @interface ASElementMap : NSObject -/** - * The total number of elements in this map. - */ -@property (readonly) NSUInteger count; - /** * The number of sections (of items) in this map. */ diff --git a/Source/Details/ASElementMap.m b/Source/Details/ASElementMap.m index f5646de78..08e90d401 100644 --- a/Source/Details/ASElementMap.m +++ b/Source/Details/ASElementMap.m @@ -75,11 +75,6 @@ - (instancetype)initWithSections:(NSArray *)sections items:(ASColle return self; } -- (NSUInteger)count -{ - return _elementToIndexPathMap.count; -} - - (NSArray *)itemIndexPaths { return ASIndexPathsForTwoDimensionalArray(_sectionsOfItems); diff --git a/Source/Layout/ASLayout.mm b/Source/Layout/ASLayout.mm index 226b4a89e..f6fbe9127 100644 --- a/Source/Layout/ASLayout.mm +++ b/Source/Layout/ASLayout.mm @@ -203,9 +203,13 @@ - (void)setRetainSublayoutLayoutElements:(BOOL)retainSublayoutLayoutElements _sublayoutLayoutElements = nil; } else { // Add sublayouts layout elements to an internal array to retain it while the layout lives - _sublayoutLayoutElements = ASMutableArrayByFlatMapping(_sublayouts, ASLayout *sublayout, ({ - sublayout.layoutElement; - })); + NSUInteger sublayoutCount = _sublayouts.count; + if (sublayoutCount > 0) { + _sublayoutLayoutElements = [NSMutableArray arrayWithCapacity:sublayoutCount]; + for (ASLayout *sublayout in _sublayouts) { + [_sublayoutLayoutElements addObject:sublayout.layoutElement]; + } + } } } } @@ -232,7 +236,7 @@ - (ASLayout *)filteredNodeLayoutTree queue.push_back({sublayout, sublayout.position}); } - std::vector flattenedSublayouts; + NSMutableArray *flattenedSublayouts = [NSMutableArray array]; while (!queue.empty()) { const Context context = queue.front(); @@ -251,7 +255,7 @@ - (ASLayout *)filteredNodeLayoutTree position:absolutePosition sublayouts:@[]]; } - flattenedSublayouts.push_back(layout); + [flattenedSublayouts addObject:layout]; } else if (sublayoutsCount > 0){ std::vector sublayoutContexts; for (ASLayout *sublayout in sublayouts) { @@ -261,8 +265,7 @@ - (ASLayout *)filteredNodeLayoutTree } } - NSArray *sublayoutsArray = [NSArray arrayWithObjects:flattenedSublayouts.data() count:flattenedSublayouts.size()]; - ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size sublayouts:sublayoutsArray]; + ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size sublayouts:flattenedSublayouts]; // All flattened layouts must have this flag enabled // to ensure sublayout elements are retained until the layouts are applied. layout.retainSublayoutLayoutElements = YES; diff --git a/Source/Layout/ASLayoutSpec.mm b/Source/Layout/ASLayoutSpec.mm index a6aabdb39..76901d1ac 100644 --- a/Source/Layout/ASLayoutSpec.mm +++ b/Source/Layout/ASLayoutSpec.mm @@ -295,15 +295,19 @@ - (instancetype)initWithLayoutElements:(NSArray> *)layoutEle - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize { + NSArray *children = self.children; + NSMutableArray *sublayouts = [NSMutableArray arrayWithCapacity:children.count]; + CGSize size = constrainedSize.min; - NSArray *sublayouts = ASArrayByFlatMapping(self.children, id child, ({ + for (id child in children) { ASLayout *sublayout = [child layoutThatFits:constrainedSize parentSize:constrainedSize.max]; sublayout.position = CGPointZero; size.width = MAX(size.width, sublayout.size.width); size.height = MAX(size.height, sublayout.size.height); - sublayout; - })); + + [sublayouts addObject:sublayout]; + } return [ASLayout layoutWithLayoutElement:self size:size sublayouts:sublayouts]; } diff --git a/Source/Private/ASTwoDimensionalArrayUtils.m b/Source/Private/ASTwoDimensionalArrayUtils.m index b21aeb41b..71a48d450 100644 --- a/Source/Private/ASTwoDimensionalArrayUtils.m +++ b/Source/Private/ASTwoDimensionalArrayUtils.m @@ -27,10 +27,13 @@ NSMutableArray *ASTwoDimensionalArrayDeepMutableCopy(NSArray *array) { - return ASMutableArrayByFlatMapping(array, NSArray *subarray, ({ + NSMutableArray *newArray = [NSMutableArray arrayWithCapacity:array.count]; + NSInteger i = 0; + for (NSArray *subarray in array) { ASDisplayNodeCAssert([subarray isKindOfClass:[NSArray class]], @"This function expects NSArray *"); - [subarray mutableCopy]; - })); + newArray[i++] = [subarray mutableCopy]; + } + return newArray; } void ASDeleteElementsInTwoDimensionalArrayAtIndexPaths(NSMutableArray *mutableArray, NSArray *indexPaths) diff --git a/Source/_ASTransitionContext.m b/Source/_ASTransitionContext.m index b664feaf5..da6350a4a 100644 --- a/Source/_ASTransitionContext.m +++ b/Source/_ASTransitionContext.m @@ -69,9 +69,11 @@ - (CGRect)finalFrameForNode:(ASDisplayNode *)node - (NSArray *)subnodesForKey:(NSString *)key { - return ASArrayByFlatMapping([self layoutForKey:key].sublayouts, ASLayout *sublayout, ({ - (ASDisplayNode *)sublayout.layoutElement; - })); + NSMutableArray *subnodes = [NSMutableArray array]; + for (ASLayout *sublayout in [self layoutForKey:key].sublayouts) { + [subnodes addObject:(ASDisplayNode *)sublayout.layoutElement]; + } + return subnodes; } - (NSArray *)insertedSubnodes