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 use of NSHashTable for interface state delegates #trivial #1122

Merged
merged 5 commits into from
Sep 17, 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
6 changes: 6 additions & 0 deletions Source/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ NS_ASSUME_NONNULL_BEGIN

#define ASDisplayNodeLoggingEnabled 0

#ifndef AS_MAX_INTERFACE_STATE_DELEGATES
#define AS_MAX_INTERFACE_STATE_DELEGATES 4
#endif

@class ASDisplayNode;
@protocol ASContextTransitioning;

Expand Down Expand Up @@ -258,6 +262,8 @@ AS_EXTERN NSInteger const ASDefaultDrawingPriority;
* @abstract Adds a delegate to receive notifications on interfaceState changes.
*
* @warning This must be called from the main thread.
* There is a hard limit on the number of delegates a node can have; see
* AS_MAX_INTERFACE_STATE_DELEGATES above.
*
* @see ASInterfaceState
*/
Expand Down
99 changes: 65 additions & 34 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -452,17 +452,6 @@ - (void)dealloc

#pragma mark - Loading

#define ASDisplayNodeCallInterfaceStateDelegates(method) \
__instanceLock__.lock(); \
NSHashTable *delegates = _copiedInterfaceStateDelegates; \
if (!delegates) { \
delegates = _copiedInterfaceStateDelegates = [_interfaceStateDelegates copy]; \
} \
__instanceLock__.unlock(); \
for (id <ASInterfaceStateDelegate> delegate in delegates) { \
[delegate method]; \
}

- (BOOL)_locked_shouldLoadViewOrLayer
{
ASAssertLocked(__instanceLock__);
Expand Down Expand Up @@ -575,7 +564,9 @@ - (void)_didLoad
for (ASDisplayNodeDidLoadBlock block in onDidLoadBlocks) {
block(self);
}
ASDisplayNodeCallInterfaceStateDelegates(nodeDidLoad);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del nodeDidLoad];
}];
}

- (void)didLoad
Expand Down Expand Up @@ -1280,7 +1271,9 @@ - (void)layout
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeAssertTrue(self.isNodeLoaded);
ASDisplayNodeCallInterfaceStateDelegates(nodeDidLayout);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del nodeDidLayout];
}];
}

#pragma mark Layout Transition
Expand Down Expand Up @@ -1503,7 +1496,9 @@ - (void)_pendingNodeDidDisplay:(ASDisplayNode *)node
if (_pendingDisplayNodes.isEmpty) {

[self hierarchyDisplayDidFinish];
ASDisplayNodeCallInterfaceStateDelegates(hierarchyDisplayDidFinish);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> delegate) {
[delegate hierarchyDisplayDidFinish];
}];

BOOL placeholderShouldPersist = [self placeholderShouldPersist];

Expand Down Expand Up @@ -3218,12 +3213,9 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac
// Subclass hook
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeAssertMainThread();
__instanceLock__.lock();
NSHashTable *delegates = [_interfaceStateDelegates copy];
__instanceLock__.unlock();
for (id <ASInterfaceStateDelegate> delegate in delegates) {
[delegate interfaceStateDidChange:newState fromState:oldState];
}
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del interfaceStateDidChange:newState fromState:oldState];
}];
}

- (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfaceState
Expand All @@ -3236,20 +3228,25 @@ - (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfac
- (void)addInterfaceStateDelegate:(id <ASInterfaceStateDelegate>)interfaceStateDelegate
{
ASDN::MutexLocker l(__instanceLock__);
// Not a fan of lazy loading, but this method won't get called very often and avoiding
// the overhead of creating this is probably worth it.
if (_interfaceStateDelegates == nil) {
_interfaceStateDelegates = [NSHashTable weakObjectsHashTable];
_hasHadInterfaceStateDelegates = YES;
for (int i = 0; i < AS_MAX_INTERFACE_STATE_DELEGATES; i++) {
if (_interfaceStateDelegates[i] == nil) {
_interfaceStateDelegates[i] = interfaceStateDelegate;
return;
}
}
_copiedInterfaceStateDelegates = nil;
[_interfaceStateDelegates addObject:interfaceStateDelegate];
ASDisplayNodeFailAssert(@"Exceeded interface state delegate limit: %d", AS_MAX_INTERFACE_STATE_DELEGATES);
}

- (void)removeInterfaceStateDelegate:(id <ASInterfaceStateDelegate>)interfaceStateDelegate
{
ASDN::MutexLocker l(__instanceLock__);
_copiedInterfaceStateDelegates = nil;
[_interfaceStateDelegates removeObject:interfaceStateDelegate];
for (int i = 0; i < AS_MAX_INTERFACE_STATE_DELEGATES; i++) {
if (_interfaceStateDelegates[i] == interfaceStateDelegate) {
_interfaceStateDelegates[i] = nil;
break;
}
}
}

- (BOOL)isVisible
Expand All @@ -3263,7 +3260,9 @@ - (void)didEnterVisibleState
// subclass override
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeCallInterfaceStateDelegates(didEnterVisibleState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didEnterVisibleState];
}];
#if AS_ENABLE_TIPS
[ASTipsController.shared nodeDidAppear:self];
#endif
Expand All @@ -3274,7 +3273,9 @@ - (void)didExitVisibleState
// subclass override
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeCallInterfaceStateDelegates(didExitVisibleState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didExitVisibleState];
}];
}

- (BOOL)isInDisplayState
Expand All @@ -3288,15 +3289,19 @@ - (void)didEnterDisplayState
// subclass override
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeCallInterfaceStateDelegates(didEnterDisplayState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didEnterDisplayState];
}];
}

- (void)didExitDisplayState
{
// subclass override
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeCallInterfaceStateDelegates(didExitDisplayState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didExitDisplayState];
}];
}

- (BOOL)isInPreloadState
Expand Down Expand Up @@ -3346,14 +3351,18 @@ - (void)didEnterPreloadState
if (self.automaticallyManagesSubnodes) {
[self layoutIfNeeded];
}
ASDisplayNodeCallInterfaceStateDelegates(didEnterPreloadState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didEnterPreloadState];
}];
}

- (void)didExitPreloadState
{
ASDisplayNodeAssertMainThread();
ASAssertUnlocked(__instanceLock__);
ASDisplayNodeCallInterfaceStateDelegates(didExitPreloadState);
[self enumerateInterfaceStateDelegates:^(id<ASInterfaceStateDelegate> del) {
[del didExitPreloadState];
}];
}

- (void)clearContents
Expand All @@ -3380,7 +3389,29 @@ - (void)recursivelyClearContents
});
}

- (void)enumerateInterfaceStateDelegates:(void (NS_NOESCAPE ^)(id<ASInterfaceStateDelegate>))block
{
ASAssertUnlocked(__instanceLock__);

id dels[AS_MAX_INTERFACE_STATE_DELEGATES];
int count = 0;
{
ASLockScopeSelf();
// Fast path for non-delegating nodes.
if (!_hasHadInterfaceStateDelegates) {
return;
}

for (int i = 0; i < AS_MAX_INTERFACE_STATE_DELEGATES; i++) {
if ((dels[count] = _interfaceStateDelegates[i])) {
count++;
}
}
}
for (int i = 0; i < count; i++) {
block(dels[i]);
}
}

#pragma mark - Gesture Recognizing

Expand Down
15 changes: 10 additions & 5 deletions Source/Private/ASDisplayNodeInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,10 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest
NSTimeInterval _debugTimeToAddSubnodeViews;
NSTimeInterval _debugTimeForDidLoad;
#endif

NSHashTable <id <ASInterfaceStateDelegate>> *_interfaceStateDelegates;

// never mutated, used to enumerate delegates outside of lock.
// set to nil when mutating _interfaceStateDelegates.
NSHashTable <id <ASInterfaceStateDelegate>> *_copiedInterfaceStateDelegates;
/// Fast path: tells whether we've ever had an interface state delegate before.
BOOL _hasHadInterfaceStateDelegates;
__weak id<ASInterfaceStateDelegate> _interfaceStateDelegates[AS_MAX_INTERFACE_STATE_DELEGATES];
}

+ (void)scheduleNodeForRecursiveDisplay:(ASDisplayNode *)node;
Expand Down Expand Up @@ -334,6 +332,13 @@ AS_EXTERN NSString * const ASRenderingEngineDidDisplayNodesScheduledBeforeTimest

- (void)applyPendingViewState;

/**
* Makes a local copy of the interface state delegates then calls the block on each.
*
* Lock is not held during block invocation. Method must not be called with the lock held.
*/
- (void)enumerateInterfaceStateDelegates:(void(NS_NOESCAPE ^)(id<ASInterfaceStateDelegate> delegate))block;

/**
* // TODO: NOT YET IMPLEMENTED
*
Expand Down