[Runtime] Use os_unfair_lock for Mutex and StaticMutex on Darwin. #34598
Conversation
|
@swift-ci please test |
|
Build failed |
os_unfair_lock is much smaller than pthread_mutex_t (4 bytes versus 64) and a bit faster. However, it doesn't support condition variables. Most of our uses of Mutex don't use condition variables, but a few do. Introduce ConditionMutex and StaticConditionMutex, which allow condition variables and continue to use pthread_mutex_t. On all other platforms, we continue to use the same backing mutex type for both Mutex and ConditionMutex. rdar://problem/45412121
mikeash
force-pushed
the
os-unfair-lock-mutex
branch
from
3385fed to
dd6c235
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
include/swift/Runtime/Mutex.h
Outdated
| friend class Mutex; | ||
| friend class StaticMutex; | ||
| friend class ConditionMutex; | ||
| friend class StaticConditionMutex; |
There was a problem hiding this comment.
I wonder if there is a better name for these.
There was a problem hiding this comment.
Update: discussed offline with @mikeash and we ended up coming up with ConditionVariable::Mutex for the "special" mutex here.
There was a problem hiding this comment.
I made that change and nested a few other things that made sense. How's it look now?
…r Mutex.h types to be nested.
|
@swift-ci please test |
| Mutex(const Mutex &) = delete; | ||
| Mutex &operator=(const Mutex &) = delete; | ||
| Mutex(Mutex &&) = delete; | ||
| Mutex &operator=(Mutex &&) = delete; |
There was a problem hiding this comment.
These don't have to be private. The = delete will ensure that they cannot be called irrespective of access level.
There was a problem hiding this comment.
This pattern (having them before the public: block with = delete) is repeated throughout this file. Is there any particular reason to change this, or should we just leave it be?
There was a problem hiding this comment.
Naw, leaving it as is for this change is fine. It was more for knowledge sharing purposes. The original reason for these being private is that before C++11, the methods would be looked up by access control and rely on the missing definition to prevent usage. With the newer functionality, they can be public as the compiler now enforces the prevention no matter what the access level.
|
We should probably just come up with more targeted abstractions for blocking a thread until another thread is done with work. Condition variables are a problematic design. |
|
Yeah, we're vulnerable to priority inversions where we use them now, so that's not great. |
os_unfair_lockis much smaller thanpthread_mutex_t(4 bytes versus 64) and a bit faster.However, it doesn't support condition variables. Most of our uses of
Mutexdon't use condition variables, but a few do. IntroduceConditionMutexandStaticConditionMutex, which allow condition variables and continue to usepthread_mutex_t.On all other platforms, we use the same backing mutex type for both
MutexandConditionMutex.rdar://problem/45412121