Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove NSMutableArray for retaining sublayout elements #1030

Merged
merged 5 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 2 additions & 8 deletions Source/Layout/ASLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
73 changes: 31 additions & 42 deletions Source/Layout/ASLayout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <AsyncDisplayKit/ASLayout.h>

#import <atomic>
#import <queue>

#import <AsyncDisplayKit/ASCollections.h>
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.");
}
Expand All @@ -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));
Expand All @@ -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];
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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];
}
Expand Down