Skip to content

Commit

Permalink
Revert "New runloop queue to coalesce Interface state update calls. (#…
Browse files Browse the repository at this point in the history 
…788)"

This reverts commit 2618c50.
  • Loading branch information
@garrettmoon
garrettmoon committed Mar 14, 2018
1 parent f99dd68 commit 29076d1
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 444 deletions.
96 changes: 29 additions & 67 deletions Source/ASDisplayNode.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
// We have to forward declare the protocol as this place otherwise it will not compile compiling with an Base SDK < iOS 10
@protocol CALayerDelegate;

@interface ASDisplayNode () <UIGestureRecognizerDelegate, CALayerDelegate, _ASDisplayLayerDelegate, ASCATransactionQueueObserving>
@interface ASDisplayNode () <UIGestureRecognizerDelegate, CALayerDelegate, _ASDisplayLayerDelegate>

/**
* See ASDisplayNodeInternal.h for ivars
Expand Down Expand Up @@ -2836,7 +2836,7 @@ - (void)setHierarchyState:(ASHierarchyState)newState
// Entered or exited range managed state.
if ((newState & ASHierarchyStateRangeManaged) != (oldState & ASHierarchyStateRangeManaged)) {
if (newState & ASHierarchyStateRangeManaged) {
[self enterInterfaceState:self.supernode.pendingInterfaceState];
[self enterInterfaceState:self.supernode.interfaceState];
} else {
// The case of exiting a range-managed state should be fairly rare. Adding or removing the node
// to a view hierarchy will cause its interfaceState to be either fully set or unset (all fields),
Expand Down Expand Up @@ -2879,34 +2879,30 @@ - (void)didExitHierarchy
ASDisplayNodeAssert(_flags.isExitingHierarchy, @"You should never call -didExitHierarchy directly. Appearance is automatically managed by ASDisplayNode");
ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive");
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);

// This case is important when tearing down hierarchies. We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for
// things like data analytics about user content viewing. We cannot call the method in the dealloc as any incidental retain operations in client code would fail.
// Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the
// same runloop. Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed).
// TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer
// integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call.
if (ASInterfaceStateIncludesVisible(_pendingInterfaceState)) {
void(^exitVisibleInterfaceState)(void) = ^{
// This block intentionally retains self.
__instanceLock__.lock();
unsigned isStillInHierarchy = _flags.isInHierarchy;
BOOL isVisible = ASInterfaceStateIncludesVisible(_pendingInterfaceState);
ASInterfaceState newState = (_pendingInterfaceState & ~ASInterfaceStateVisible);
__instanceLock__.unlock();

if (!isStillInHierarchy && isVisible) {
if (![self supportsRangeManagedInterfaceState]) {
newState = ASInterfaceStateNone;

if (![self supportsRangeManagedInterfaceState]) {
self.interfaceState = ASInterfaceStateNone;
} else {
// This case is important when tearing down hierarchies. We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for
// things like data analytics about user content viewing. We cannot call the method in the dealloc as any incidental retain operations in client code would fail.
// Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the
// same runloop. Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed).
// TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer
// integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call.

if (ASInterfaceStateIncludesVisible(self.interfaceState)) {
dispatch_async(dispatch_get_main_queue(), ^{
// This block intentionally retains self.
__instanceLock__.lock();
unsigned isInHierarchy = _flags.isInHierarchy;
BOOL isVisible = ASInterfaceStateIncludesVisible(_interfaceState);
ASInterfaceState newState = (_interfaceState & ~ASInterfaceStateVisible);
__instanceLock__.unlock();

if (!isInHierarchy && isVisible) {
self.interfaceState = newState;
}
self.interfaceState = newState;
}
};

if ([[ASCATransactionQueue sharedQueue] disabled]) {
dispatch_async(dispatch_get_main_queue(), exitVisibleInterfaceState);
} else {
exitVisibleInterfaceState();
});
}
}
}
Expand Down Expand Up @@ -2967,53 +2963,25 @@ - (ASInterfaceState)interfaceState
}

- (void)setInterfaceState:(ASInterfaceState)newState
{
if ([[ASCATransactionQueue sharedQueue] disabled]) {
[self applyPendingInterfaceState:newState];
} else {
ASDN::MutexLocker l(__instanceLock__);
if (_pendingInterfaceState != newState) {
_pendingInterfaceState = newState;
[[ASCATransactionQueue sharedQueue] enqueue:self];
}
}
}

- (ASInterfaceState)pendingInterfaceState
{
ASDN::MutexLocker l(__instanceLock__);
return _pendingInterfaceState;
}

- (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState
{
//This method is currently called on the main thread. The assert has been added here because all of the
//did(Enter|Exit)(Display|Visible|Preload)State methods currently guarantee calling on main.
ASDisplayNodeAssertMainThread();

// It should never be possible for a node to be visible but not be allowed / expected to display.
ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState));
// This method manages __instanceLock__ itself, to ensure the lock is not held while didEnter/Exit(.*)State methods are called, thus avoid potential deadlocks
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);

ASInterfaceState oldState = ASInterfaceStateNone;
ASInterfaceState newState = ASInterfaceStateNone;
{
ASDN::MutexLocker l(__instanceLock__);
// newPendingState will not be used when ASCATransactionQueue is enabled
// and use _pendingInterfaceState instead for interfaceState update.
if ([[ASCATransactionQueue sharedQueue] disabled]) {
_pendingInterfaceState = newPendingState;
}
oldState = _interfaceState;
newState = _pendingInterfaceState;
if (newState == oldState) {
if (_interfaceState == newState) {
return;
}
oldState = _interfaceState;
_interfaceState = newState;
}

// It should never be possible for a node to be visible but not be allowed / expected to display.
ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState));

// TODO: Trigger asynchronous measurement if it is not already cached or being calculated.
// if ((newState & ASInterfaceStateMeasureLayout) != (oldState & ASInterfaceStateMeasureLayout)) {
// }
Expand Down Expand Up @@ -3110,12 +3078,6 @@ - (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState
[self interfaceStateDidChange:newState fromState:oldState];
}

- (void)prepareForCATransactionCommit
{
// Apply _pendingInterfaceState actual _interfaceState, note that ASInterfaceStateNone is not used.
[self applyPendingInterfaceState:ASInterfaceStateNone];
}

- (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfaceState)oldState
{
// Subclass hook
Expand Down
35 changes: 2 additions & 33 deletions Source/ASRunLoopQueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,8 @@

NS_ASSUME_NONNULL_BEGIN

@protocol ASCATransactionQueueObserving <NSObject>
- (void)prepareForCATransactionCommit;
@end

@interface ASAbstractRunLoopQueue : NSObject
@end

AS_SUBCLASSING_RESTRICTED
@interface ASRunLoopQueue<ObjectType> : ASAbstractRunLoopQueue <NSLocking>
@interface ASRunLoopQueue<ObjectType> : NSObject <NSLocking>

/**
* Create a new queue with the given run loop and handler.
Expand All @@ -48,37 +41,13 @@ AS_SUBCLASSING_RESTRICTED

- (void)enqueue:(ObjectType)object;

@property (atomic, readonly) BOOL isEmpty;
@property (nonatomic, readonly) BOOL isEmpty;

@property (nonatomic, assign) NSUInteger batchSize; // Default == 1.
@property (nonatomic, assign) BOOL ensureExclusiveMembership; // Default == YES. Set-like behavior.

@end

AS_SUBCLASSING_RESTRICTED
@interface ASCATransactionQueue : ASAbstractRunLoopQueue

@property (atomic, readonly) BOOL isEmpty;
@property (atomic, readonly) BOOL disabled;
/**
* The queue to run on main run loop before CATransaction commit.
*
* @discussion this queue will run after ASRunLoopQueue and before CATransaction commit
* to get last chance of updating/coalesce info like interface state.
* Each node will only be called once per transaction commit to reflect interface change.
*/
@property (class, atomic, readonly) ASCATransactionQueue *sharedQueue;

- (void)enqueue:(id<ASCATransactionQueueObserving>)object;

/**
* @abstract Apply a node's interfaceState immediately rather than adding to the queue.
*/
- (void)disable;

@end


AS_SUBCLASSING_RESTRICTED
@interface ASDeallocQueue : NSObject

Expand Down
Loading

0 comments on commit 29076d1

Please sign in to comment.