[Concurrency] Eliminate StatusRecordLockRecord. #80316
Conversation
Move to a recursive lock inline in the Task. This avoids the need to allocate a lock record and simplifies the code somewhat. Change Task's OpaquePrivateStorage to compute its size at build time based on the sizes of its components, rather than having it be a fixed size. It appears that the fixed size was intended to be part of the ABI, but that didn't happen and we're free to change this size. We need to expand it slightly when using pthread_mutex as the recursive lock, as pthread_mutex is pretty big. Other recursive locks allow it to shrink slightly. We don't have a recursive mutex in our Threading support code, so add a RecursiveMutex type. rdar://113898653
|
@swift-ci Please Build Toolchain Linux Platform |
|
@swift-ci Please Build Toolchain macOS Platform |
We're using `CRITICAL_SECTION` here, as that supports recursion. rdar://113898653
|
@swift-ci Please test Windows platform |
include/swift/Threading/Mutex.h
Outdated
| RecursiveMutex(const Mutex &) = delete; | ||
| RecursiveMutex &operator=(const Mutex &) = delete; | ||
| RecursiveMutex(Mutex &&) = delete; | ||
| RecursiveMutex &operator=(Mutex &&) = delete; |
There was a problem hiding this comment.
I think these want to take RecursiveMutex rather than Mutex arguments.
There was a problem hiding this comment.
Copy/pasta trouble, fixed all of these now.
include/swift/Threading/Impl/Linux.h
Outdated
|
|
||
| using recursive_mutex_handle = ::pthread_mutex_t; | ||
|
|
||
| inline void recursive_mutex_init(mutex_handle &handle, bool checked = false) { |
There was a problem hiding this comment.
It so happens that mutex_handle and recursive_mutex_handle are the same here, but really these should take recursive_mutex_handle.
|
|
||
| using recursive_mutex_handle = ::pthread_mutex_t; | ||
|
|
||
| inline void recursive_mutex_init(mutex_handle &handle, bool checked = false) { |
There was a problem hiding this comment.
Similarly to the Linux implementation, these should probably take recursive_mutex_handle arguments.
`ULONG_PTR` is defined slightly differently on `_WIN64` builds. rdar://113898653
|
@swift-ci Please test Windows platform |
|
@swift-ci please test |
|
|
|
@swift-ci please test |
|
Windows failed due to an unrelated issue on |
|
@swift-ci please test windows platform |
| @@ -298,7 +298,18 @@ class AsyncTask : public Job { | |||
|
|
|||
| /// Private storage for the use of the runtime. | |||
| struct alignas(2 * alignof(void*)) OpaquePrivateStorage { | |||
| void *Storage[14]; | |||
| #if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION && SWIFT_POINTER_IS_4_BYTES | |||
There was a problem hiding this comment.
Sorry for the late comment, I'm just slightly confused why there's a #if condition where both paths are identical in terms of text
There was a problem hiding this comment.
Well spotted. The reason is, I messed it up. I have a fix going here: #80447
Draft PR, this still needs a Win32 implementation of
RecursiveMutex.Move to a recursive lock inline in the Task. This avoids the need to allocate a lock record and simplifies the code somewhat.
Change Task's OpaquePrivateStorage to compute its size at build time based on the sizes of its components, rather than having it be a fixed size. It appears that the fixed size was intended to be part of the ABI, but that didn't happen and we're free to change this size. We need to expand it slightly when using pthread_mutex as the recursive lock, as pthread_mutex is pretty big. Other recursive locks allow it to shrink slightly.
We don't have a recursive mutex in our Threading support code, so add a RecursiveMutex type.
rdar://113898653