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

Replace pthread specifics with C11 thread-local variables #811

Merged
merged 2 commits into from
Feb 28, 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 @@ -30,6 +30,7 @@
- Fix ASTextNode2 is accessing backgroundColor off main while sizing / layout is happening. [Michael Schneider](https://github.com/maicki) [#794](https://github.com/TextureGroup/Texture/pull/778/)
- Pass scrollViewWillEndDragging delegation through in ASIGListAdapterDataSource for IGListKit integration. [#796](https://github.com/TextureGroup/Texture/pull/796)
- Fix UIResponder handling with view backing ASDisplayNode. [Michael Schneider](https://github.com/maicki) [#789] (https://github.com/TextureGroup/Texture/pull/789/)
- Optimized thread-local storage by replacing pthread_specific with C11 thread-local variables. [Adlai Holler](https://github.com/Adlai-Holler) [#811] (https://github.com/TextureGroup/Texture/pull/811/)

## 2.6
- [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)
Expand Down
2 changes: 0 additions & 2 deletions Source/ASDisplayNode+Subclasses.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
// http://www.apache.org/licenses/LICENSE-2.0
//

#import <pthread.h>

#import <AsyncDisplayKit/ASBlockTypes.h>
#import <AsyncDisplayKit/ASDisplayNode.h>

Expand Down
11 changes: 3 additions & 8 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1046,15 +1046,11 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
restrictedToSize:(ASLayoutElementSize)size
relativeToParentSize:(CGSize)parentSize
{
// Use a pthread specific to mark when this method is called re-entrant on same thread.
// We only want one calculateLayout signpost interval per thread.
// This is fast enough to do it unconditionally.
auto key = ASPthreadStaticKey(NULL);
BOOL isRootCall = (pthread_getspecific(key) == NULL);
static _Thread_local NSInteger tls_callDepth;
as_activity_scope_verbose(as_activity_create("Calculate node layout", AS_ACTIVITY_CURRENT, OS_ACTIVITY_FLAG_DEFAULT));
as_log_verbose(ASLayoutLog(), "Calculating layout for %@ sizeRange %@", self, NSStringFromASSizeRange(constrainedSize));
if (isRootCall) {
pthread_setspecific(key, kCFBooleanTrue);
if (tls_callDepth++ == 0) {
ASSignpostStart(ASSignpostCalculateLayout);
}

Expand All @@ -1063,8 +1059,7 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
ASLayout *result = [self calculateLayoutThatFits:resolvedRange];
as_log_verbose(ASLayoutLog(), "Calculated layout %@", result);

if (isRootCall) {
pthread_setspecific(key, NULL);
if (--tls_callDepth == 0) {
ASSignpostEnd(ASSignpostCalculateLayout);
}
return result;
Expand Down
21 changes: 5 additions & 16 deletions Source/Base/ASAssert.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,18 @@
//

#import <AsyncDisplayKit/ASAssert.h>
#import <Foundation/Foundation.h>

static pthread_key_t ASMainThreadAssertionsDisabledKey()
{
return ASPthreadStaticKey(NULL);
}
static _Thread_local int tls_mainThreadAssertionsDisabledCount;

BOOL ASMainThreadAssertionsAreDisabled() {
return (size_t)pthread_getspecific(ASMainThreadAssertionsDisabledKey()) > 0;
return tls_mainThreadAssertionsDisabledCount > 0;
}

void ASPushMainThreadAssertionsDisabled() {
pthread_key_t key = ASMainThreadAssertionsDisabledKey();
size_t oldValue = (size_t)pthread_getspecific(key);
pthread_setspecific(key, (void *)(oldValue + 1));
tls_mainThreadAssertionsDisabledCount += 1;
}

void ASPopMainThreadAssertionsDisabled() {
pthread_key_t key = ASMainThreadAssertionsDisabledKey();
size_t oldValue = (size_t)pthread_getspecific(key);
if (oldValue > 0) {
pthread_setspecific(key, (void *)(oldValue - 1));
} else {
ASDisplayNodeCFailAssert(@"Attempt to pop thread assertion-disabling without corresponding push.");
}
tls_mainThreadAssertionsDisabledCount -= 1;
ASDisplayNodeCAssert(tls_mainThreadAssertionsDisabledCount >= 0, @"Attempt to pop thread assertion-disabling without corresponding push.");
}
9 changes: 0 additions & 9 deletions Source/Base/ASBaseDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,6 @@
#define AS_SUBCLASSING_RESTRICTED
#endif

#define ASPthreadStaticKey(dtor) ({ \
static dispatch_once_t onceToken; \
static pthread_key_t key; \
dispatch_once(&onceToken, ^{ \
pthread_key_create(&key, dtor); \
}); \
key; \
})

#define ASCreateOnce(expr) ({ \
static dispatch_once_t onceToken; \
static __typeof__(expr) staticVar; \
Expand Down
27 changes: 8 additions & 19 deletions Source/Layout/ASLayoutElement.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,38 +50,27 @@ - (instancetype)init
int32_t const ASLayoutElementContextInvalidTransitionID = 0;
int32_t const ASLayoutElementContextDefaultTransitionID = ASLayoutElementContextInvalidTransitionID + 1;

static void ASLayoutElementDestructor(void *p) {
if (p != NULL) {
ASDisplayNodeCFailAssert(@"Thread exited without clearing layout element context!");
CFBridgingRelease(p);
}
};

pthread_key_t ASLayoutElementContextKey()
{
return ASPthreadStaticKey(ASLayoutElementDestructor);
}
static _Thread_local __unsafe_unretained ASLayoutElementContext *tls_context;

void ASLayoutElementPushContext(ASLayoutElementContext *context)
{
// NOTE: It would be easy to support nested contexts – just use an NSMutableArray here.
ASDisplayNodeCAssertNil(ASLayoutElementGetCurrentContext(), @"Nested ASLayoutElementContexts aren't supported.");
pthread_setspecific(ASLayoutElementContextKey(), CFBridgingRetain(context));
ASDisplayNodeCAssertNil(tls_context, @"Nested ASLayoutElementContexts aren't supported.");

tls_context = (__bridge ASLayoutElementContext *)(__bridge_retained CFTypeRef)context;
}

ASLayoutElementContext *ASLayoutElementGetCurrentContext()
{
// Don't retain here. Caller will retain if it wants to!
return (__bridge __unsafe_unretained ASLayoutElementContext *)pthread_getspecific(ASLayoutElementContextKey());
return tls_context;
}

void ASLayoutElementPopContext()
{
ASLayoutElementContextKey();
ASDisplayNodeCAssertNotNil(ASLayoutElementGetCurrentContext(), @"Attempt to pop context when there wasn't a context!");
auto key = ASLayoutElementContextKey();
CFBridgingRelease(pthread_getspecific(key));
pthread_setspecific(key, NULL);
ASDisplayNodeCAssertNotNil(tls_context, @"Attempt to pop context when there wasn't a context!");
CFRelease((__bridge CFTypeRef)tls_context);
tls_context = nil;
Copy link
Member Author

Choose a reason for hiding this comment

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

Used CFRelease directly here, since we don't actually want to bridge the value into ARC (no need to return id, void is fine.).

}

#pragma mark - ASLayoutElementStyle
Expand Down