Skip to content

Commit

Permalink
Introduce ASRecursiveUnfairLock and tests (TextureGroup#858)
Browse files Browse the repository at this point in the history
* Introduce ASRecursiveUnfairLock and tests

* Document it and put underscores to scare people away

* Increment changelog

* Integrate it with ASDN::Mutex behind experiment

* Rename the experiment

* Love these license headers oh so much

* Move internal header because we have to

* Address Jon's feedback
  • Loading branch information
Adlai-Holler authored Mar 28, 2018
1 parent cf1c3f6 commit 4bbbd72
Show file tree
Hide file tree
Showing 11 changed files with 426 additions and 130 deletions.
18 changes: 15 additions & 3 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@
CCA282D01E9EBF6C0037E8B7 /* ASTipsWindow.h in Headers */ = {isa = PBXBuildFile; fileRef = CCA282CE1E9EBF6C0037E8B7 /* ASTipsWindow.h */; };
CCA282D11E9EBF6C0037E8B7 /* ASTipsWindow.m in Sources */ = {isa = PBXBuildFile; fileRef = CCA282CF1E9EBF6C0037E8B7 /* ASTipsWindow.m */; };
CCA5F62E1EECC2A80060C137 /* ASAssert.m in Sources */ = {isa = PBXBuildFile; fileRef = CCA5F62D1EECC2A80060C137 /* ASAssert.m */; };
CCAA0B7F206ADBF30057B336 /* ASRecursiveUnfairLock.h in Headers */ = {isa = PBXBuildFile; fileRef = CCAA0B7D206ADBF30057B336 /* ASRecursiveUnfairLock.h */; settings = {ATTRIBUTES = (Public, ); }; };
CCAA0B80206ADBF30057B336 /* ASRecursiveUnfairLock.m in Sources */ = {isa = PBXBuildFile; fileRef = CCAA0B7E206ADBF30057B336 /* ASRecursiveUnfairLock.m */; };
CCAA0B82206ADECB0057B336 /* ASRecursiveUnfairLockTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CCAA0B81206ADECB0057B336 /* ASRecursiveUnfairLockTests.m */; };
CCB1F95A1EFB60A5009C7475 /* ASLog.m in Sources */ = {isa = PBXBuildFile; fileRef = CCB1F9591EFB60A5009C7475 /* ASLog.m */; };
CCB1F95C1EFB6350009C7475 /* ASSignpost.h in Headers */ = {isa = PBXBuildFile; fileRef = CCB1F95B1EFB6316009C7475 /* ASSignpost.h */; };
CCB2F34D1D63CCC6004E6DE9 /* ASDisplayNodeSnapshotTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CCB2F34C1D63CCC6004E6DE9 /* ASDisplayNodeSnapshotTests.m */; };
Expand Down Expand Up @@ -417,7 +420,7 @@
CCED5E3E2020D36800395C40 /* ASNetworkImageLoadInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = CCED5E3C2020D36800395C40 /* ASNetworkImageLoadInfo.h */; settings = {ATTRIBUTES = (Public, ); }; };
CCED5E3F2020D36800395C40 /* ASNetworkImageLoadInfo.m in Sources */ = {isa = PBXBuildFile; fileRef = CCED5E3D2020D36800395C40 /* ASNetworkImageLoadInfo.m */; };
CCED5E412020D49D00395C40 /* ASNetworkImageLoadInfo+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = CCED5E402020D41600395C40 /* ASNetworkImageLoadInfo+Private.h */; settings = {ATTRIBUTES = (Private, ); }; };
CCEDDDCA200C2AC300FFCD0A /* ASConfigurationInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = CCEDDDC8200C2AC300FFCD0A /* ASConfigurationInternal.h */; };
CCEDDDCA200C2AC300FFCD0A /* ASConfigurationInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = CCEDDDC8200C2AC300FFCD0A /* ASConfigurationInternal.h */; settings = {ATTRIBUTES = (Public, ); }; };
CCEDDDCB200C2AC300FFCD0A /* ASConfigurationInternal.m in Sources */ = {isa = PBXBuildFile; fileRef = CCEDDDC9200C2AC300FFCD0A /* ASConfigurationInternal.m */; };
CCEDDDCD200C2CB900FFCD0A /* ASConfiguration.h in Headers */ = {isa = PBXBuildFile; fileRef = CCEDDDCC200C2CB900FFCD0A /* ASConfiguration.h */; settings = {ATTRIBUTES = (Public, ); }; };
CCEDDDCF200C42A200FFCD0A /* ASConfigurationDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = CCEDDDCE200C42A200FFCD0A /* ASConfigurationDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -892,6 +895,9 @@
CCA282CE1E9EBF6C0037E8B7 /* ASTipsWindow.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASTipsWindow.h; sourceTree = "<group>"; };
CCA282CF1E9EBF6C0037E8B7 /* ASTipsWindow.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASTipsWindow.m; sourceTree = "<group>"; };
CCA5F62D1EECC2A80060C137 /* ASAssert.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASAssert.m; sourceTree = "<group>"; };
CCAA0B7D206ADBF30057B336 /* ASRecursiveUnfairLock.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASRecursiveUnfairLock.h; sourceTree = "<group>"; };
CCAA0B7E206ADBF30057B336 /* ASRecursiveUnfairLock.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASRecursiveUnfairLock.m; sourceTree = "<group>"; };
CCAA0B81206ADECB0057B336 /* ASRecursiveUnfairLockTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASRecursiveUnfairLockTests.m; sourceTree = "<group>"; };
CCB1F9591EFB60A5009C7475 /* ASLog.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASLog.m; sourceTree = "<group>"; };
CCB1F95B1EFB6316009C7475 /* ASSignpost.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASSignpost.h; sourceTree = "<group>"; };
CCB2F34C1D63CCC6004E6DE9 /* ASDisplayNodeSnapshotTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASDisplayNodeSnapshotTests.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1115,6 +1121,8 @@
DECBD6E61BE56E1900CF4905 /* ASButtonNode.mm */,
CCEDDDCC200C2CB900FFCD0A /* ASConfiguration.h */,
CCEDDDD0200C488000FFCD0A /* ASConfiguration.m */,
CCEDDDC8200C2AC300FFCD0A /* ASConfigurationInternal.h */,
CCEDDDC9200C2AC300FFCD0A /* ASConfigurationInternal.m */,
CCEDDDCE200C42A200FFCD0A /* ASConfigurationDelegate.h */,
055F1A3A19ABD43F004DAFF1 /* ASCellNode.h */,
AC6456071B0A335000CF11B8 /* ASCellNode.mm */,
Expand Down Expand Up @@ -1268,6 +1276,7 @@
CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */,
ACF6ED5A1B178DC700DA7C62 /* ASRatioLayoutSpecSnapshotTests.mm */,
E52AC9BE1FEA915D00AA4040 /* ASRectMapTests.m */,
CCAA0B81206ADECB0057B336 /* ASRecursiveUnfairLockTests.m */,
7AB338681C55B97B0055FDE8 /* ASRelativeLayoutSpecSnapshotTests.mm */,
4E9127681F64157600499623 /* ASRunLoopQueueTests.m */,
E586F96B1F9F9E2900ECE00E /* ASScrollNodeTests.m */,
Expand Down Expand Up @@ -1367,6 +1376,8 @@
055F1A3619ABD413004DAFF1 /* ASRangeController.h */,
055F1A3719ABD413004DAFF1 /* ASRangeController.mm */,
69F10C851C84C35D0026140C /* ASRangeControllerUpdateRangeProtocol+Beta.h */,
CCAA0B7D206ADBF30057B336 /* ASRecursiveUnfairLock.h */,
CCAA0B7E206ADBF30057B336 /* ASRecursiveUnfairLock.m */,
81EE384D1C8E94F000456208 /* ASRunLoopQueue.h */,
81EE384E1C8E94F000456208 /* ASRunLoopQueue.mm */,
296A0A311A951715005ACEAA /* ASScrollDirection.h */,
Expand Down Expand Up @@ -1414,8 +1425,6 @@
058D0A01195D050800B7D73C /* Private */ = {
isa = PBXGroup;
children = (
CCEDDDC8200C2AC300FFCD0A /* ASConfigurationInternal.h */,
CCEDDDC9200C2AC300FFCD0A /* ASConfigurationInternal.m */,
CCE04B2A1E313EDA006AEBBB /* Collection Data Adapter */,
E52F8AEE1EAE659600B5A912 /* Collection Layout */,
6947B0BB1E36B4E30007C478 /* Layout */,
Expand Down Expand Up @@ -1807,6 +1816,7 @@
693A1DCA1ECC944E00D0C9D2 /* IGListAdapter+AsyncDisplayKit.h in Headers */,
E5E2D72E1EA780C4005C24C6 /* ASCollectionGalleryLayoutDelegate.h in Headers */,
E58E9E461E941D74004CFC59 /* ASCollectionLayoutDelegate.h in Headers */,
CCAA0B7F206ADBF30057B336 /* ASRecursiveUnfairLock.h in Headers */,
E5E281741E71C833006B67C2 /* ASCollectionLayoutState.h in Headers */,
E5B077FF1E69F4EB00C24B5B /* ASElementMap.h in Headers */,
CCCCCCE31EC3EF060087FE10 /* NSParagraphStyle+ASText.h in Headers */,
Expand Down Expand Up @@ -2278,6 +2288,7 @@
ACF6ED631B178DC700DA7C62 /* ASStackLayoutSpecSnapshotTests.mm in Sources */,
E52AC9C01FEA916C00AA4040 /* ASRectMapTests.m in Sources */,
CCE4F9BA1F0DBB5000062E4E /* ASLayoutTestNode.mm in Sources */,
CCAA0B82206ADECB0057B336 /* ASRecursiveUnfairLockTests.m in Sources */,
81E95C141D62639600336598 /* ASTextNodeSnapshotTests.m in Sources */,
3C9C128519E616EF00E942A0 /* ASTableViewTests.mm in Sources */,
AEEC47E41C21D3D200EC1693 /* ASVideoNodeTests.m in Sources */,
Expand Down Expand Up @@ -2357,6 +2368,7 @@
509E68641B3AEDB7009B9150 /* ASCollectionViewLayoutController.m in Sources */,
B35061F91B010EFD0018CF92 /* ASControlNode.mm in Sources */,
8021EC1F1D2B00B100799119 /* UIImage+ASConvenience.m in Sources */,
CCAA0B80206ADBF30057B336 /* ASRecursiveUnfairLock.m in Sources */,
B35062181B010EFD0018CF92 /* ASDataController.mm in Sources */,
CCB1F95A1EFB60A5009C7475 /* ASLog.m in Sources */,
767E7F8E1C90191D0066C000 /* AsyncDisplayKit+Debug.m in Sources */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- [NoCopyRendering] Improved performance & fixed image memory not being tagged in Instruments. [Adlai Holler](https://github.com/Adlai-Holler) [#833](https://github.com/TextureGroup/Texture/pull/833/)
- Use `NS_RETURNS_RETAINED` macro to make our methods a tiny bit faster. [Adlai Holler](https://github.com/Adlai-Holler) [#843](https://github.com/TextureGroup/Texture/pull/843/)
- `ASDisplayNode, ASLayoutSpec, and ASLayoutElementStyle` now conform to `NSLocking`. They act as recursive locks. Useful locking macros have been added as `ASThread.h`. Subclasses / client code can lock these objects but should be careful as usual when dealing with locks. [Adlai Holler](https://github.com/Adlai-Holler)
- Introduces `ASRecursiveUnfairLock` as an experiment to improve locking performance. [Adlai Holler](https://github.com/Adlai-Holler)

## 2.6
- [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)
Expand Down
3 changes: 2 additions & 1 deletion Schemas/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"enum": [
"exp_graphics_contexts",
"exp_text_node",
"exp_interface_state_coalesce"
"exp_interface_state_coalesce",
"exp_unfair_lock"
]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
// http://www.apache.org/licenses/LICENSE-2.0
//

/// Note this has to be public because it's imported by public header ASThread.h =/
/// It will be private again after exp_unfair_lock ends.

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,13 @@ - (BOOL)activateExperimentalFeature:(ASExperimentalFeatures)requested
return (enabled != 0);
}

#if DEBUG
// Define this even when !DEBUG, since we may run our tests in release mode.
+ (void)test_resetWithConfiguration:(ASConfiguration *)configuration
{
ASConfigurationManager *inst = ASGetSharedConfigMgr();
inst->_config = configuration ?: [self defaultConfiguration];
atomic_store(&inst->_activatedExperiments, 0);
}
#endif

@end

Expand Down
1 change: 1 addition & 0 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalGraphicsContexts = 1 << 0, // exp_graphics_contexts
ASExperimentalTextNode = 1 << 1, // exp_text_node
ASExperimentalInterfaceStateCoalescing = 1 << 2, // exp_interface_state_coalesce
ASExperimentalUnfairLock = 1 << 3, // exp_unfair_lock
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down
1 change: 1 addition & 0 deletions Source/AsyncDisplayKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#import <AsyncDisplayKit/ASDisplayNodeExtras.h>
#import <AsyncDisplayKit/ASConfiguration.h>
#import <AsyncDisplayKit/ASConfigurationDelegate.h>
#import <AsyncDisplayKit/ASConfigurationInternal.h>

#import <AsyncDisplayKit/ASControlNode.h>
#import <AsyncDisplayKit/ASImageNode.h>
Expand Down
59 changes: 59 additions & 0 deletions Source/Details/ASRecursiveUnfairLock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//
// ASRecursiveUnfairLock.h
// Texture
//
// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//

#import <Foundation/Foundation.h>
#import <pthread/pthread.h>
#import <os/lock.h>

// Don't import C-only header if we're in a C++ file
#ifndef __cplusplus
#import <stdatomic.h>
#endif

// Note: We don't use ATOMIC_VAR_INIT here because C++ compilers don't like it,
// and it literally does absolutely nothing.
#define AS_RECURSIVE_UNFAIR_LOCK_INIT ((ASRecursiveUnfairLock){ OS_UNFAIR_LOCK_INIT, NULL, 0})

NS_ASSUME_NONNULL_BEGIN

OS_UNFAIR_LOCK_AVAILABILITY
typedef struct {
os_unfair_lock _lock;
_Atomic(pthread_t) _thread; // Write-protected by lock
int _count; // Protected by lock
} ASRecursiveUnfairLock;

CF_EXTERN_C_BEGIN

/**
* Lock, blocking if needed.
*/
OS_UNFAIR_LOCK_AVAILABILITY
void ASRecursiveUnfairLockLock(ASRecursiveUnfairLock *l);

/**
* Try to lock without blocking. Returns whether we took the lock.
*/
OS_UNFAIR_LOCK_AVAILABILITY
BOOL ASRecursiveUnfairLockTryLock(ASRecursiveUnfairLock *l);

/**
* Unlock. Calling this on a thread that does not own
* the lock will result in an assertion failure, and undefined
* behavior if foundation assertions are disabled.
*/
OS_UNFAIR_LOCK_AVAILABILITY
void ASRecursiveUnfairLockUnlock(ASRecursiveUnfairLock *l);

CF_EXTERN_C_END

NS_ASSUME_NONNULL_END
76 changes: 76 additions & 0 deletions Source/Details/ASRecursiveUnfairLock.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//
// ASRecursiveUnfairLock.m
// Texture
//
// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//

#import "ASRecursiveUnfairLock.h"

/**
* For our atomic _thread, we use acquire/release memory order so that we can have
* the minimum possible constraint on the hardware. The default, `memory_order_seq_cst`
* demands that there be a total order of all such modifications as seen by all threads.
* Acquire/release only requires that modifications to this specific atomic are
* synchronized across acquire/release pairs.
* http://en.cppreference.com/w/cpp/atomic/memory_order
*
* Note also that the unfair_lock involves a thread fence as well, so we don't need to
* take care of synchronizing other values. Just the thread value.
*/
#define rul_set_thread(l, t) atomic_store_explicit(&l->_thread, t, memory_order_release)
#define rul_get_thread(l) atomic_load_explicit(&l->_thread, memory_order_acquire)

void ASRecursiveUnfairLockLock(ASRecursiveUnfairLock *l)
{
// Just a cache for pthread_self so that we never call it twice.
pthread_t s = NULL;

// Try to lock without blocking. If we fail, check what thread owns it.
// Note that the owning thread CAN CHANGE freely, but it can't become `self`
// because only we are `self`. And if it's already `self` then we already have
// the lock, because we reset it to NULL before we unlock. So (thread == self) is
// invariant.

if (!os_unfair_lock_trylock(&l->_lock) && (rul_get_thread(l) != (s = pthread_self()))) {
// Owned by other thread. Possibly other threads are waiting too. Block.
os_unfair_lock_lock(&l->_lock);
}
// Now we've got the lock. Update the thread pointer and count.
rul_set_thread(l, s ?: pthread_self());
l->_count++;
}

BOOL ASRecursiveUnfairLockTryLock(ASRecursiveUnfairLock *l)
{
// Same logic as `Lock` function, see comments there.
pthread_t s = NULL;

if (!os_unfair_lock_trylock(&l->_lock) && (rul_get_thread(l) != (s = pthread_self()))) {
return NO;
}
rul_set_thread(l, s ?: pthread_self());
l->_count++;
return YES;
}

void ASRecursiveUnfairLockUnlock(ASRecursiveUnfairLock *l)
{
// Ensure we have the lock. This check may miss some pathological cases,
// but it'll catch 99.999999% of this serious programmer error.
NSCAssert(rul_get_thread(l) == pthread_self(), @"Unlocking from a different thread than locked.");

if (0 == --l->_count) {
// Note that we have to clear this before unlocking because, if another thread
// succeeds in locking above, but hasn't managed to update _thread, and we
// try to re-lock, and fail the -tryLock, and read _thread, then we'll mistakenly
// think that we still own the lock and proceed without blocking.
rul_set_thread(l, NULL);
os_unfair_lock_unlock(&l->_lock);
}
}
Loading

0 comments on commit 4bbbd72

Please sign in to comment.