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

ASCATransactionQueue interface trashing improvements #1216

Merged
merged 2 commits into from
Nov 11, 2018
Merged
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
29 changes: 5 additions & 24 deletions Source/ASRunLoopQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ - (void)dealloc
#if ASRunLoopQueueLoggingEnabled
- (void)checkRunLoop
{
NSLog(@"<%@> - Jobs: %ld", self, _internalQueue.size());
NSLog(@"<%@> - Jobs: %ld", self, _internalQueue.count);
}
#endif

Expand Down Expand Up @@ -323,10 +323,8 @@ @interface ASCATransactionQueue () {
CFRunLoopRef _runLoop;
CFRunLoopSourceRef _runLoopSource;
CFRunLoopObserverRef _preTransactionObserver;
CFRunLoopObserverRef _postTransactionObserver;
NSPointerArray *_internalQueue;
ASDN::RecursiveMutex _internalQueueLock;
BOOL _CATransactionCommitInProgress;

// In order to not pollute the top-level activities, each queue has 1 root activity.
os_activity_t _rootActivity;
Expand All @@ -343,9 +341,6 @@ @implementation ASCATransactionQueue
// CoreAnimation commit order is 2000000, the goal of this is to process shortly beforehand
// but after most other scheduled work on the runloop has processed.
static int const kASASCATransactionQueueOrder = 1000000;
// This will mark the end of current loop and any node enqueued between kASASCATransactionQueueOrder
// and kASASCATransactionQueuePostOrder will apply interface change immediately.
static int const kASASCATransactionQueuePostOrder = 3000000;

+ (ASCATransactionQueue *)sharedQueue NS_RETURNS_RETAINED
{
Expand Down Expand Up @@ -377,17 +372,13 @@ - (instancetype)init
// __unsafe_unretained allows us to avoid flagging the memory cycle detector.
__unsafe_unretained __typeof__(self) weakSelf = self;
void (^handlerBlock) (CFRunLoopObserverRef observer, CFRunLoopActivity activity) = ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) {
while (weakSelf->_internalQueue.count > 0) {
Copy link
Member

@appleguy appleguy Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation seems wrong here?

[weakSelf processQueue];
};
void (^postHandlerBlock) (CFRunLoopObserverRef observer, CFRunLoopActivity activity) = ^(CFRunLoopObserverRef observer, CFRunLoopActivity activity) {
ASDN::MutexLocker l(_internalQueueLock);
_CATransactionCommitInProgress = NO;
}
};
_preTransactionObserver = CFRunLoopObserverCreateWithHandler(NULL, kCFRunLoopBeforeWaiting, true, kASASCATransactionQueueOrder, handlerBlock);
_postTransactionObserver = CFRunLoopObserverCreateWithHandler(NULL, kCFRunLoopBeforeWaiting, true, kASASCATransactionQueuePostOrder, postHandlerBlock);

CFRunLoopAddObserver(_runLoop, _preTransactionObserver, kCFRunLoopCommonModes);
CFRunLoopAddObserver(_runLoop, _postTransactionObserver, kCFRunLoopCommonModes);

// It is not guaranteed that the runloop will turn if it has no scheduled work, and this causes processing of
// the queue to stop. Attaching a custom loop source to the run loop and signal it if new work needs to be done
Expand Down Expand Up @@ -416,19 +407,14 @@ - (void)dealloc
if (CFRunLoopObserverIsValid(_preTransactionObserver)) {
CFRunLoopObserverInvalidate(_preTransactionObserver);
}
if (CFRunLoopObserverIsValid(_postTransactionObserver)) {
CFRunLoopObserverInvalidate(_postTransactionObserver);
}
CFRelease(_preTransactionObserver);
CFRelease(_postTransactionObserver);
_preTransactionObserver = nil;
_postTransactionObserver = nil;
}

#if ASRunLoopQueueLoggingEnabled
- (void)checkRunLoop
{
NSLog(@"<%@> - Jobs: %ld", self, _internalQueue.size());
NSLog(@"<%@> - Jobs: %ld", self, _internalQueue.count);
}
#endif

Expand All @@ -441,11 +427,6 @@ - (void)processQueue
{
ASDN::MutexLocker l(_internalQueueLock);

// Mark the queue will end coalescing shortly until after CATransactionCommit.
// This will give the queue a chance to apply any further interfaceState changes/enqueue
// immediately within current runloop instead of pushing the work to next runloop cycle.
_CATransactionCommitInProgress = YES;

NSInteger internalQueueCount = _internalQueue.count;
// Early-exit if the queue is empty.
if (internalQueueCount == 0) {
Expand Down Expand Up @@ -502,7 +483,7 @@ - (void)enqueue:(id<ASCATransactionQueueObserving>)object
return;
}

if (!self.enabled || _CATransactionCommitInProgress) {
if (!self.enabled) {
[object prepareForCATransactionCommit];
return;
}
Expand Down