Skip to content

Commit

Permalink
Get StaticMutex back
Browse files Browse the repository at this point in the history
This reverts part of 5c9815f. Using Mutex for global C++ variables causes Texture experiments to be triggered before main(). Our (internal) experiment framework isn't ready that early in the process.

We can get rid of StaticMutex once the unfair lock experiment is shipped.
  • Loading branch information
Huy Nguyen authored and nguyenhuy committed Nov 26, 2018
1 parent f2bc63f commit f4e2860
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
6 changes: 3 additions & 3 deletions Source/ASImageNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -434,12 +434,12 @@ + (UIImage *)displayWithParameters:(id<NSObject>)parameter isCancelled:(NS_NOESC

static ASWeakMap<ASImageNodeContentsKey *, UIImage *> *cache = nil;
// Allocate cacheLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
static auto *cacheLock = new ASDN::Mutex;
static auto *cacheLock = new ASDN::StaticMutex;

+ (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:(id)drawParameters isCancelled:(asdisplaynode_iscancelled_block_t)isCancelled
{
{
ASDN::MutexLocker l(*cacheLock);
ASDN::StaticMutexLocker l(*cacheLock);
if (!cache) {
cache = [[ASWeakMap alloc] init];
}
Expand All @@ -456,7 +456,7 @@ + (ASWeakMapEntry *)contentsForkey:(ASImageNodeContentsKey *)key drawParameters:
}

{
ASDN::MutexLocker l(*cacheLock);
ASDN::StaticMutexLocker l(*cacheLock);
return [cache setObject:contents forKey:key];
}
}
Expand Down
2 changes: 1 addition & 1 deletion Source/ASTextNode2.mm
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ @implementation ASTextCacheValue
*/
static NS_RETURNS_RETAINED ASTextLayout *ASTextNodeCompatibleLayoutWithContainerAndText(ASTextContainer *container, NSAttributedString *text) {
// Allocate layoutCacheLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
static auto *layoutCacheLock = new ASDN::Mutex;
static auto *layoutCacheLock = new ASDN::StaticMutex;
static NSCache<NSAttributedString *, ASTextCacheValue *> *textLayoutCache;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
Expand Down
6 changes: 3 additions & 3 deletions Source/Details/ASBasicImageDownloader.mm
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ @implementation ASBasicImageDownloaderContext

static NSMutableDictionary *currentRequests = nil;
// Allocate currentRequestsLock on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
static auto *currentRequestsLock = new ASDN::Mutex;
static auto *currentRequestsLock = new ASDN::StaticMutex;

+ (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL
{
ASDN::MutexLocker l(*currentRequestsLock);
ASDN::StaticMutexLocker l(*currentRequestsLock);
if (!currentRequests) {
currentRequests = [[NSMutableDictionary alloc] init];
}
Expand All @@ -57,7 +57,7 @@ + (ASBasicImageDownloaderContext *)contextForURL:(NSURL *)URL

+ (void)cancelContextWithURL:(NSURL *)URL
{
ASDN::MutexLocker l(*currentRequestsLock);
ASDN::StaticMutexLocker l(*currentRequestsLock);
if (currentRequests) {
[currentRequests removeObjectForKey:URL];
}
Expand Down
26 changes: 26 additions & 0 deletions Source/Details/ASThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,34 @@ namespace ASDN {
RecursiveMutex () : Mutex (true) {}
};

/**
If you are creating a static mutex, use StaticMutex. This avoids expensive constructor overhead at startup (or worse, ordering
issues between different static objects). It also avoids running a destructor on app exit time (needless expense).
Note that you can, but should not, use StaticMutex for non-static objects. It will leak its mutex on destruction,
so avoid that!
TODO: Remove StaticMutex and use Mutex everywhere once the unfair lock experiment is shipped
*/
struct StaticMutex
{
StaticMutex () : _m (PTHREAD_MUTEX_INITIALIZER) {}
// non-copyable.
StaticMutex(const StaticMutex&) = delete;
StaticMutex &operator=(const StaticMutex&) = delete;
void lock () {
AS_POSIX_ASSERT_NOERR(pthread_mutex_lock (this->mutex()));
}
void unlock () {
AS_POSIX_ASSERT_NOERR(pthread_mutex_unlock (this->mutex()));
}
pthread_mutex_t *mutex () { return &_m; }
private:
pthread_mutex_t _m;
};

typedef std::lock_guard<Mutex> MutexLocker;
typedef std::unique_lock<Mutex> UniqueLock;
typedef std::lock_guard<StaticMutex> StaticMutexLocker;

} // namespace ASDN

Expand Down
4 changes: 2 additions & 2 deletions Source/TextKit/ASTextKitContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString
if (self = [super init]) {
// Concurrently initialising TextKit components crashes (rdar://18448377) so we use a global lock.
// Allocate mutex on the heap to prevent destruction at app exit (https://github.com/TextureGroup/Texture/issues/136)
static auto *mutex = new ASDN::Mutex;
ASDN::MutexLocker l(*mutex);
static auto *mutex = new ASDN::StaticMutex;
ASDN::StaticMutexLocker l(*mutex);

__instanceLock__ = std::make_shared<ASDN::Mutex>();

Expand Down

0 comments on commit f4e2860

Please sign in to comment.