Skip to content

Commit

Permalink
[ASDisplayNode] Implement a std::atomic-based flag system for superb …
Browse files Browse the repository at this point in the history
…performance

Although Objective-C atomics are equally fast, or better that std::atomic when
access through method calls, for the most intense use cases it is best to avoid
method calls entirely.

In ASDisplayNode, we could benefit significantly from avoiding both method calls
(already true today) but also avoid locking the mutex, for both CPU and contention
gains.

There will still be many methods that need locking for transactional
consistency - however, there are currently dozens of accessor methods that could
avoid frequent lock / unlock cycles with use of atomics, and other usages of the
ivars directly where locking could be delayed until after early-return conditions
are checked and passed.
  • Loading branch information
appleguy committed Apr 30, 2017
1 parent d2587bb commit 002364d
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 20 deletions.
35 changes: 20 additions & 15 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ - (instancetype)initWithViewClass:(Class)viewClass
ASDisplayNodeAssert([viewClass isSubclassOfClass:[UIView class]], @"should initialize with a subclass of UIView");

_viewClass = viewClass;
self.synchronous = ![viewClass isSubclassOfClass:[_ASDisplayView class]];
setFlag(Synchronous, ![viewClass isSubclassOfClass:[_ASDisplayView class]]);

return self;
}
Expand All @@ -328,8 +328,8 @@ - (instancetype)initWithLayerClass:(Class)layerClass
ASDisplayNodeAssert([layerClass isSubclassOfClass:[CALayer class]], @"should initialize with a subclass of CALayer");

_layerClass = layerClass;
self.synchronous = ![layerClass isSubclassOfClass:[_ASDisplayLayer class]];
_flags.layerBacked = YES;
setFlag(Synchronous, ![layerClass isSubclassOfClass:[_ASDisplayLayer class]]);

return self;
}
Expand Down Expand Up @@ -378,7 +378,7 @@ - (void)setViewBlock:(ASDisplayNodeViewBlock)viewBlock
ASDisplayNodeAssertNotNil(viewBlock, @"should initialize with a valid block that returns a UIView");

_viewBlock = viewBlock;
self.synchronous = YES;
setFlag(Synchronous, YES);
}

- (void)setLayerBlock:(ASDisplayNodeLayerBlock)layerBlock
Expand All @@ -387,8 +387,8 @@ - (void)setLayerBlock:(ASDisplayNodeLayerBlock)layerBlock
ASDisplayNodeAssertNotNil(layerBlock, @"should initialize with a valid block that returns a CALayer");

_layerBlock = layerBlock;
self.synchronous = YES;
_flags.layerBacked = YES;
setFlag(Synchronous, YES);
}

- (void)onDidLoad:(ASDisplayNodeDidLoadBlock)body
Expand All @@ -411,7 +411,7 @@ - (void)dealloc
_flags.isDeallocating = YES;

// Synchronous nodes may not be able to call the hierarchy notifications, so only enforce for regular nodes.
ASDisplayNodeAssert(self.isSynchronous || !ASInterfaceStateIncludesVisible(_interfaceState), @"Node should always be marked invisible before deallocating. Node: %@", self);
ASDisplayNodeAssert(checkFlag(Synchronous) || !ASInterfaceStateIncludesVisible(_interfaceState), @"Node should always be marked invisible before deallocating. Node: %@", self);

self.asyncLayer.asyncDelegate = nil;
_view.asyncdisplaykit_node = nil;
Expand Down Expand Up @@ -547,7 +547,7 @@ - (void)__unloadNode
{
ASDisplayNodeAssertMainThread();
ASDisplayNodeAssert([self isNodeLoaded], @"Implementation shouldn't call __unloadNode if not loaded: %@", self);
ASDisplayNodeAssert(self.isSynchronous == NO, @"Node created using -initWithViewBlock:/-initWithLayerBlock: cannot be unloaded. Node: %@", self);
ASDisplayNodeAssert(checkFlag(Synchronous) == NO, @"Node created using -initWithViewBlock:/-initWithLayerBlock: cannot be unloaded. Node: %@", self);
ASDN::MutexLocker l(__instanceLock__);

if (_flags.layerBacked) {
Expand Down Expand Up @@ -583,7 +583,7 @@ - (UIView *)_locked_viewToLoad
}

// Special handling of wrapping UIKit components
if (self.isSynchronous) {
if (checkFlag(Synchronous)) {
// UIImageView layers. More details on the flags
if ([_viewClass isSubclassOfClass:[UIImageView class]]) {
_flags.canClearContentsOfLayer = NO;
Expand Down Expand Up @@ -803,6 +803,11 @@ - (_ASDisplayLayer *)_locked_asyncLayer
return [_layer isKindOfClass:[_ASDisplayLayer class]] ? (_ASDisplayLayer *)_layer : nil;
}

- (BOOL)isSynchronous
{
return checkFlag(Synchronous);
}

- (void)setLayerBacked:(BOOL)isLayerBacked
{
// Only call this if assertions are enabled – it could be expensive.
Expand Down Expand Up @@ -830,7 +835,7 @@ - (BOOL)isLayerBacked
- (BOOL)supportsLayerBacking
{
ASDN::MutexLocker l(__instanceLock__);
return !self.isSynchronous && !_flags.viewEverHadAGestureRecognizerAttached && _viewClass == [_ASDisplayView class] && _layerClass == [_ASDisplayLayer class];
return !checkFlag(Synchronous) && !_flags.viewEverHadAGestureRecognizerAttached && _viewClass == [_ASDisplayView class] && _layerClass == [_ASDisplayLayer class];
}

- (BOOL)shouldAnimateSizeChanges
Expand Down Expand Up @@ -1942,7 +1947,7 @@ - (BOOL)displaysAsynchronously
*/
- (BOOL)_locked_displaysAsynchronously
{
return self.isSynchronous == NO && _flags.displaysAsynchronously;
return checkFlag(Synchronous) == NO && _flags.displaysAsynchronously;
}

- (void)setDisplaysAsynchronously:(BOOL)displaysAsynchronously
Expand All @@ -1952,7 +1957,7 @@ - (void)setDisplaysAsynchronously:(BOOL)displaysAsynchronously
ASDN::MutexLocker l(__instanceLock__);

// Can't do this for synchronous nodes (using layers that are not _ASDisplayLayer and so we can't control display prevention/cancel)
if (self.isSynchronous) {
if (checkFlag(Synchronous)) {
return;
}

Expand Down Expand Up @@ -2046,7 +2051,7 @@ - (void)setContentsScaleForDisplay:(CGFloat)contentsScaleForDisplay
- (void)displayImmediately
{
ASDisplayNodeAssertMainThread();
ASDisplayNodeAssert(!self.isSynchronous, @"this method is designed for asynchronous mode only");
ASDisplayNodeAssert(!checkFlag(Synchronous), @"this method is designed for asynchronous mode only");

[self.asyncLayer displayImmediately];
}
Expand All @@ -2067,7 +2072,7 @@ - (void)__setNeedsDisplay
BOOL nowDisplay = ASInterfaceStateIncludesDisplay(_interfaceState);
// FIXME: This should not need to recursively display, so create a non-recursive variant.
// The semantics of setNeedsDisplay (as defined by CALayer behavior) are not recursive.
if (_layer != nil && !self.isSynchronous && nowDisplay && [self _implementsDisplay]) {
if (_layer != nil && !checkFlag(Synchronous) && nowDisplay && [self _implementsDisplay]) {
shouldScheduleForDisplay = YES;
}
}
Expand Down Expand Up @@ -2311,7 +2316,7 @@ - (void)setDisplaySuspended:(BOOL)flag
__instanceLock__.lock();

// Can't do this for synchronous nodes (using layers that are not _ASDisplayLayer and so we can't control display prevention/cancel)
if (self.isSynchronous || _flags.displaySuspended == flag) {
if (checkFlag(Synchronous) || _flags.displaySuspended == flag) {
__instanceLock__.unlock();
return;
}
Expand Down Expand Up @@ -3400,7 +3405,7 @@ - (void)setHierarchyState:(ASHierarchyState)newState

// Entered rasterization state.
if (newState & ASHierarchyStateRasterized) {
ASDisplayNodeAssert(self.isSynchronous == NO, @"Node created using -initWithViewBlock:/-initWithLayerBlock: cannot be added to subtree of node with shouldRasterizeDescendants=YES. Node: %@", self);
ASDisplayNodeAssert(checkFlag(Synchronous) == NO, @"Node created using -initWithViewBlock:/-initWithLayerBlock: cannot be added to subtree of node with shouldRasterizeDescendants=YES. Node: %@", self);
}

// Entered or exited range managed state.
Expand Down Expand Up @@ -3901,7 +3906,7 @@ - (void)_locked_applyPendingViewState
if (_flags.layerBacked) {
[_pendingViewState applyToLayer:self.layer];
} else {
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(self.isSynchronous, _flags.layerBacked);
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), _flags.layerBacked);
[_pendingViewState applyToView:self.view withSpecialPropertiesHandling:specialPropertiesHandling];
}

Expand Down
4 changes: 2 additions & 2 deletions Source/Private/ASDisplayNode+UIViewBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ - (void)setFrame:(CGRect)rect

// For classes like ASTableNode, ASCollectionNode, ASScrollNode and similar - make sure UIView gets setFrame:
struct ASDisplayNodeFlags flags = _flags;
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(self.isSynchronous, flags.layerBacked);
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), flags.layerBacked);

BOOL nodeLoaded = __loaded(self);
BOOL isMainThread = ASDisplayNodeThreadIsMain();
Expand Down Expand Up @@ -635,7 +635,7 @@ - (void)setBackgroundColor:(UIColor *)newBackgroundColor
if (shouldApply) {
CGColorRef oldBackgroundCGColor = _layer.backgroundColor;

BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(self.isSynchronous, _flags.layerBacked);
BOOL specialPropertiesHandling = ASDisplayNodeNeedsSpecialPropertiesHandling(checkFlag(Synchronous), _flags.layerBacked);
if (specialPropertiesHandling) {
_view.backgroundColor = newBackgroundColor;
} else {
Expand Down
15 changes: 12 additions & 3 deletions Source/Private/ASDisplayNodeInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ typedef NS_OPTIONS(NSUInteger, ASDisplayNodeMethodOverrides)
ASDisplayNodeMethodOverrideClearFetchedData = 1 << 6
};

typedef NS_OPTIONS(uint_least32_t, ASDisplayNodeAtomicFlags)
{
Synchronous = 1 << 0,
};

#define checkFlag(flag) ((_atomicFlags.load() & flag) != 0)
// Returns the old value of the flag as a BOOL.
#define setFlag(flag, x) (((x ? _atomicFlags.fetch_or(flag) \
: _atomicFlags.fetch_and(~flag)) & flag) != 0)

FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayScheduledNodesNotification;
FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimestamp;

Expand All @@ -71,6 +81,8 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
UIView *_view;
CALayer *_layer;

std::atomic<ASDisplayNodeAtomicFlags> _atomicFlags;

struct ASDisplayNodeFlags {
// public properties
unsigned viewEverHadAGestureRecognizerAttached:1;
Expand Down Expand Up @@ -207,9 +219,6 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
/// Bitmask to check which methods an object overrides.
@property (nonatomic, assign, readonly) ASDisplayNodeMethodOverrides methodOverrides;

/// Atomic BOOL that indicates whether the node wraps a synchronous layer or view class.
@property (atomic, assign, readwrite, getter=isSynchronous) BOOL synchronous;

/**
* Invoked before a call to setNeedsLayout to the underlying view
*/
Expand Down

0 comments on commit 002364d

Please sign in to comment.