Remove NSMutableArray for retaining sublayout elements (#1030)

* Remove NSMutableArray for retaining sublayout elements

* Kick the CI

* Kick the CI again

* Smash that CI button

* Murder the CI

CHANGELOG.md

@@ -21,6 +21,7 @@
 - Fix build on 32-bit simulator in Xcode 9.3 and later, caused by `Thread-local storage is not supported on this architecture.` [Adlai Holler](https://github.com/Adlai-Holler)
 - Enable locking assertions (and add some more) to improve and enforce locking safety within the framework [Huy Nguyen](https://github.com/nguyenhuy) [#1024](https://github.com/TextureGroup/Texture/pull/1024)
 - Split MapKit, Photos, and AssetsLibrary dependent code into separate subspecs to improve binary size and start time when they're not needed. The default subspec includes all three for backwards compatibility, but this **will change in 3.0**. When using non-Cocoapods build environments, define `AS_USE_PHOTOS, AS_USE_MAPKIT, AS_USE_ASSETS_LIBRARY` to 1 respectively to signal their use. [Adlai Holler](https://github.com/Adlai-Holler)
+- Optimization: Removed an NSMutableArray in flattened layouts. [Adlai Holler](https://github.com/Adlai-Holler)
 
 ## 2.7
 - Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)

Source/Layout/ASLayout.h

@@ -149,15 +149,9 @@ AS_EXTERN ASLayout *ASCalculateLayout(id<ASLayoutElement>layoutElement, const AS
 
 /**
  * Set to YES to tell all ASLayout instances to retain their sublayout elements. Defaults to NO.
- * Can be overridden at instance level.
+ * See `-retainSublayoutElements` to control this per-instance.
  */
-+ (void)setShouldRetainSublayoutLayoutElements:(BOOL)shouldRetain;
-
-/**
- * Whether or not ASLayout instances should retain their sublayout elements.
- * Can be overridden at instance level.
- */
-+ (BOOL)shouldRetainSublayoutLayoutElements;
+@property (class) BOOL shouldRetainSublayoutElements;
 
 /**
  * Recrusively output the description of the layout tree.

Source/Layout/ASLayout.mm

@@ -17,6 +17,7 @@
 
 #import <AsyncDisplayKit/ASLayout.h>
 
+#import <atomic>
 #import <queue>
 
 #import <AsyncDisplayKit/ASCollections.h>
@@ -59,17 +60,11 @@ ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT BOOL ASLayoutIsDisplayNodeType(ASLayo
 @interface ASLayout () <ASDescriptionProvider>
 {
   ASLayoutElementType _layoutElementType;
+  std::atomic_bool _retainSublayoutElements;
 }
 
-/*
- * Caches all sublayouts if set to YES or destroys the sublayout cache if set to NO. Defaults to NO
- */
-@property (nonatomic) BOOL retainSublayoutLayoutElements;
-
-/**
- * Array for explicitly retain sublayout layout elements in case they are created and references in layoutSpecThatFits: and no one else will hold a strong reference on it
- */
-@property (nonatomic) NSMutableArray<id<ASLayoutElement>> *sublayoutLayoutElements;
+/// Call this to keep sublayout elements alive. Multiple calls have no effect.
+- (void)retainSublayoutElements;
 
 @property (nonatomic, readonly) ASRectMap *elementToRectMap;
 
@@ -100,7 +95,7 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
 
   self = [super init];
   if (self) {
-#if DEBUG
+#if ASDISPLAYNODE_ASSERTIONS_ENABLED
     for (ASLayout *sublayout in sublayouts) {
       ASDisplayNodeAssert(ASPointIsNull(sublayout.position) == NO, @"Invalid position is not allowed in sublayout.");
     }
@@ -112,7 +107,7 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
     _layoutElementType = layoutElement.layoutElementType;
 
     if (!ASIsCGSizeValidForSize(size)) {
-      ASDisplayNodeAssert(NO, @"layoutSize is invalid and unsafe to provide to Core Animation! Release configurations will force to 0, 0.  Size = %@, node = %@", NSStringFromCGSize(size), layoutElement);
+      ASDisplayNodeFailAssert(@"layoutSize is invalid and unsafe to provide to Core Animation! Release configurations will force to 0, 0.  Size = %@, node = %@", NSStringFromCGSize(size), layoutElement);
       size = CGSizeZero;
     } else {
       size = CGSizeMake(ASCeilPixelValue(size.width), ASCeilPixelValue(size.height));
@@ -125,7 +120,7 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
       _position = position;
     }
 
-    _sublayouts = sublayouts != nil ? [sublayouts copy] : @[];
+    _sublayouts = [sublayouts copy] ?: @[];
 
     if (_sublayouts.count > 0) {
       _elementToRectMap = [ASRectMap rectMapForWeakObjectPointers];
@@ -134,18 +129,14 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
       }
     }
 
-    self.retainSublayoutLayoutElements = [ASLayout shouldRetainSublayoutLayoutElements];
+    if ([ASLayout shouldRetainSublayoutLayoutElements]) {
+      [self retainSublayoutElements];
+    }
   }
 
   return self;
 }
 
-- (instancetype)init
-{
-  ASDisplayNodeAssert(NO, @"Use the designated initializer");
-  return [self init];
-}
-
 #pragma mark - Class Constructors
 
 + (instancetype)layoutWithLayoutElement:(id<ASLayoutElement>)layoutElement
@@ -177,25 +168,25 @@ + (instancetype)layoutWithLayoutElement:(id<ASLayoutElement>)layoutElement size:
                             sublayouts:nil];
 }
 
+- (void)dealloc
+{
+  if (_retainSublayoutElements.load()) {
+    for (ASLayout *sublayout in _sublayouts) {
+      CFRelease((__bridge CFTypeRef)sublayout);
+    }
+  }
+}
+
 #pragma mark - Sublayout Elements Caching
 
-- (void)setRetainSublayoutLayoutElements:(BOOL)retainSublayoutLayoutElements
+- (void)retainSublayoutElements
 {
-  if (_retainSublayoutLayoutElements != retainSublayoutLayoutElements) {
-    _retainSublayoutLayoutElements = retainSublayoutLayoutElements;
-
-    if (retainSublayoutLayoutElements == NO) {
-      _sublayoutLayoutElements = nil;
-    } else {
-      // Add sublayouts layout elements to an internal array to retain it while the layout lives
-      NSUInteger sublayoutCount = _sublayouts.count;
-      if (sublayoutCount > 0) {
-        _sublayoutLayoutElements = [NSMutableArray arrayWithCapacity:sublayoutCount];
-        for (ASLayout *sublayout in _sublayouts) {
-          [_sublayoutLayoutElements addObject:sublayout.layoutElement];
-        }
-      }
-    }
+  if (_retainSublayoutElements.exchange(true)) {
+    return;
+  }
+
+  for (ASLayout *sublayout in _sublayouts) {
+    CFRetain((__bridge CFTypeRef)sublayout->_layoutElement);
   }
 }
 
@@ -220,9 +211,8 @@ - (BOOL)isFlattened
 - (ASLayout *)filteredNodeLayoutTree NS_RETURNS_RETAINED
 {
   if ([self isFlattened]) {
-    // All flattened layouts must have this flag enabled
-    // to ensure sublayout elements are retained until the layouts are applied.
-    self.retainSublayoutLayoutElements = YES;
+    // All flattened layouts must retain sublayout elements until they are applied.
+    [self retainSublayoutElements];
     return self;
   }
 
@@ -273,9 +263,8 @@ - (ASLayout *)filteredNodeLayoutTree NS_RETURNS_RETAINED
   // flattenedSublayouts is now all nils.
 
   ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size sublayouts:array];
-  // All flattened layouts must have this flag enabled
-  // to ensure sublayout elements are retained until the layouts are applied.
-  layout.retainSublayoutLayoutElements = YES;
+  // All flattened layouts must retain sublayout elements until they are applied.
+  [layout retainSublayoutElements];
   return layout;
 }
 
@@ -387,7 +376,7 @@ - (NSString *)_recursiveDescriptionForLayout:(ASLayout *)layout level:(NSUIntege
 
 ASLayout *ASCalculateLayout(id<ASLayoutElement> layoutElement, const ASSizeRange sizeRange, const CGSize parentSize)
 {
-  ASDisplayNodeCAssertNotNil(layoutElement, @"Not valid layoutElement passed in.");
+  NSCParameterAssert(layoutElement != nil);
 
   return [layoutElement layoutThatFits:sizeRange parentSize:parentSize];
 }