Skip to content

[Concurrency] Eliminate StatusRecordLockRecord. #80316

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

Merged
merged 5 commits into from
Mar 28, 2025
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
3 changes: 0 additions & 3 deletions include/swift/ABI/MetadataValues.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ enum {
/// in a non-default distributed actor.
NumWords_NonDefaultDistributedActor = 12,

/// The number of words in a task.
NumWords_AsyncTask = 24,

/// The number of words in a task group.
NumWords_TaskGroup = 32,

Expand Down
15 changes: 12 additions & 3 deletions include/swift/ABI/Task.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well spotted. The reason is, I messed it up. I have a fix going here: #80447

static constexpr size_t ActiveTaskStatusSize = 4 * sizeof(void *);
#else
static constexpr size_t ActiveTaskStatusSize = 4 * sizeof(void *);
#endif

// Private storage is currently 6 pointers, 16 bytes of non-pointer data,
// the ActiveTaskStatus, and a RecursiveMutex.
static constexpr size_t PrivateStorageSize =
6 * sizeof(void *) + 16 + ActiveTaskStatusSize + sizeof(RecursiveMutex);

void *Storage[PrivateStorageSize];

/// Initialize this storage during the creation of a task.
void initialize(JobPriority basePri);
Expand Down Expand Up @@ -755,8 +766,6 @@ class AsyncTask : public Job {
};

// The compiler will eventually assume these.
static_assert(sizeof(AsyncTask) == NumWords_AsyncTask * sizeof(void*),
"AsyncTask size is wrong");
static_assert(alignof(AsyncTask) == 2 * alignof(void*),
"AsyncTask alignment is wrong");
#pragma clang diagnostic push
Expand Down
19 changes: 19 additions & 0 deletions include/swift/Threading/Impl/C11.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,25 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
(void)::mtx_unlock(&handle.mutex);
}

// .. Recursive mutex support .................................................

using recursive_mutex_handle = ::mtx_t;

inline void recursive_mutex_init(recursive_mutex_handle &handle,
bool checked = false) {
SWIFT_C11THREADS_CHECK(::mtx_init(&handle, ::mtx_recursive));
}
inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
::mtx_destroy(&handle);
}

inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
SWIFT_C11THREADS_CHECK(::mtx_lock(&handle));
}
inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
SWIFT_C11THREADS_CHECK(::mtx_unlock(&handle));
}

// .. ConditionVariable support ..............................................

struct cond_handle {
Expand Down
36 changes: 36 additions & 0 deletions include/swift/Threading/Impl/Darwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,42 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
::os_unfair_lock_unlock(&handle);
}

// .. Recursive mutex support .................................................

// The os_unfair_recursive_lock interface is stable, but not in the SDK. Bring
// our own definitions for what we need.

#define OS_UNFAIR_RECURSIVE_LOCK_INIT \
(os_unfair_recursive_lock{OS_UNFAIR_LOCK_INIT, 0})

typedef struct os_unfair_recursive_lock_s {
os_unfair_lock ourl_lock;
uint32_t ourl_count;
} os_unfair_recursive_lock, *os_unfair_recursive_lock_t;

using recursive_mutex_handle = os_unfair_recursive_lock;

extern "C" void
os_unfair_recursive_lock_lock_with_options(os_unfair_recursive_lock_t lock,
uint32_t options);

extern "C" void
os_unfair_recursive_lock_unlock(os_unfair_recursive_lock_t lock);

inline void recursive_mutex_init(recursive_mutex_handle &handle,
bool checked = false) {
handle = OS_UNFAIR_RECURSIVE_LOCK_INIT;
}
inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {}

inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
os_unfair_recursive_lock_lock_with_options(&handle, 0);
}

inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
os_unfair_recursive_lock_unlock(&handle);
}

// .. ConditionVariable support ..............................................

struct cond_handle {
Expand Down
23 changes: 23 additions & 0 deletions include/swift/Threading/Impl/Linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,29 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
(void)::pthread_mutex_unlock(&handle);
}

// .. Recursive mutex support ................................................

using recursive_mutex_handle = ::pthread_mutex_t;

inline void recursive_mutex_init(recursive_mutex_handle &handle, bool checked = false) {
::pthread_mutexattr_t attr;
SWIFT_LINUXTHREADS_CHECK(::pthread_mutexattr_init(&attr));
SWIFT_LINUXTHREADS_CHECK(
::pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE));
SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_init(&handle, &attr));
SWIFT_LINUXTHREADS_CHECK(::pthread_mutexattr_destroy(&attr));
}
inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_destroy(&handle));
}

inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_lock(&handle));
}
inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_unlock(&handle));
}

// .. ConditionVariable support ..............................................

struct cond_handle {
Expand Down
10 changes: 10 additions & 0 deletions include/swift/Threading/Impl/Nothreads.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ inline bool lazy_mutex_try_lock(lazy_mutex_handle &handle) { return true; }
inline void lazy_mutex_unsafe_lock(lazy_mutex_handle &handle) {}
inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {}

// .. Recursive mutex support .................................................

using recursive_mutex_handle = unsigned;

inline void recursive_mutex_init(recursive_mutex_handle &handle,
bool checked = false) {}
inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {}
inline void recursive_mutex_lock(recursive_mutex_handle &handle) {}
inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {}

// .. ConditionVariable support ..............................................

using cond_handle = unsigned;
Expand Down
23 changes: 23 additions & 0 deletions include/swift/Threading/Impl/Pthreads.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,29 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
(void)::pthread_mutex_unlock(&handle);
}

// .. Recursive mutex support ................................................

using recursive_mutex_handle = ::pthread_mutex_t;

inline void recursive_mutex_init(recursive_mutex_handle &handle, bool checked = false) {
::pthread_mutexattr_t attr;
SWIFT_PTHREADS_CHECK(::pthread_mutexattr_init(&attr));
SWIFT_PTHREADS_CHECK(
::pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE));
SWIFT_PTHREADS_CHECK(::pthread_mutex_init(&handle, &attr));
SWIFT_PTHREADS_CHECK(::pthread_mutexattr_destroy(&attr));
}
inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
SWIFT_PTHREADS_CHECK(::pthread_mutex_destroy(&handle));
}

inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
SWIFT_PTHREADS_CHECK(::pthread_mutex_lock(&handle));
}
inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
SWIFT_PTHREADS_CHECK(::pthread_mutex_unlock(&handle));
}

// .. ConditionVariable support ..............................................

struct cond_handle {
Expand Down
20 changes: 20 additions & 0 deletions include/swift/Threading/Impl/Win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,26 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
ReleaseSRWLockExclusive(&handle);
}

// .. Recursive mutex support ................................................

using recursive_mutex_handle = SWIFT_CRITICAL_SECTION;

inline void recursive_mutex_init(recursive_mutex_handle &handle,
bool checked = false) {
InitializeCriticalSection(&handle);
}

inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
DeleteCriticalSection(&handle);
}

inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
EnterCriticalSection(&handle);
}
inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
LeaveCriticalSection(&handle);
}

// .. ConditionVariable support ..............................................

struct cond_handle {
Expand Down
53 changes: 53 additions & 0 deletions include/swift/Threading/Impl/Win32/Win32Defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,14 @@ typedef unsigned char BYTE;
typedef BYTE BOOLEAN;
typedef int BOOL;
typedef unsigned long DWORD;
typedef long LONG;
typedef unsigned long ULONG;
#if defined(_WIN64)
typedef unsigned __int64 ULONG_PTR;
#else
typedef unsigned long ULONG_PTR;
#endif
typedef PVOID HANDLE;

typedef VOID(NTAPI *PFLS_CALLBACK_FUNCTION)(PVOID lpFlsData);

Expand All @@ -46,6 +53,11 @@ typedef PRTL_SRWLOCK PSRWLOCK;
typedef struct _RTL_CONDITION_VARIABLE *PRTL_CONDITION_VARIABLE;
typedef PRTL_CONDITION_VARIABLE PCONDITION_VARIABLE;

typedef struct _RTL_CRITICAL_SECTION_DEBUG *PRTL_CRITICAL_SECTION_DEBUG;
typedef struct _RTL_CRITICAL_SECTION *PRTL_CRITICAL_SECTION;
typedef PRTL_CRITICAL_SECTION PCRITICAL_SECTION;
typedef PRTL_CRITICAL_SECTION LPCRITICAL_SECTION;

// These have to be #defines, to avoid problems with <windows.h>
#define RTL_SRWLOCK_INIT {0}
#define SRWLOCK_INIT RTL_SRWLOCK_INIT
Expand Down Expand Up @@ -83,6 +95,19 @@ WINBASEAPI BOOL WINAPI SleepConditionVariableSRW(
ULONG Flags
);

WINBASEAPI VOID WINAPI InitializeCriticalSection(
LPCRITICAL_SECTION lpCriticalSection
);
WINBASEAPI VOID WINAPI DeleteCriticalSection(
LPCRITICAL_SECTION lpCriticalSection
);
WINBASEAPI VOID WINAPI EnterCriticalSection(
LPCRITICAL_SECTION lpCriticalSection
);
WINBASEAPI VOID WINAPI LeaveCriticalSection(
LPCRITICAL_SECTION lpCriticalSection
);

WINBASEAPI DWORD WINAPI FlsAlloc(PFLS_CALLBACK_FUNCTION lpCallback);
WINBASEAPI PVOID WINAPI FlsGetValue(DWORD dwFlsIndex);
WINBASEAPI BOOL WINAPI FlsSetValue(DWORD dwFlsIndex, PVOID lpFlsData);
Expand Down Expand Up @@ -140,6 +165,34 @@ inline BOOL SleepConditionVariableSRW(PSWIFT_CONDITION_VARIABLE CondVar,
Flags);
}

// And with CRITICAL_SECTION
#pragma pack(push, 8)
typedef struct SWIFT_CRITICAL_SECTION {
PRTL_CRITICAL_SECTION_DEBUG DebugInfo;
LONG LockCount;
LONG RecursionCount;
HANDLE OwningThread;
HANDLE LockSemaphore;
ULONG_PTR SpinCount;
} SWIFT_CRITICAL_SECTION, *PSWIFT_CRITICAL_SECTION;
#pragma pack(pop)

inline VOID InitializeCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
::InitializeCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
}

inline VOID DeleteCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
::DeleteCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
}

inline VOID EnterCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
::EnterCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
}

inline VOID LeaveCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
::LeaveCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
}

} // namespace threading_impl
} // namespace swift

Expand Down
42 changes: 42 additions & 0 deletions include/swift/Threading/Mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,48 @@ class IndirectMutex {
using SmallMutex =
std::conditional_t<sizeof(Mutex) <= sizeof(void *), Mutex, IndirectMutex>;

/// A recursive variant of Mutex.
class RecursiveMutex {

RecursiveMutex(const RecursiveMutex &) = delete;
RecursiveMutex &operator=(const RecursiveMutex &) = delete;
RecursiveMutex(RecursiveMutex &&) = delete;
RecursiveMutex &operator=(RecursiveMutex &&) = delete;

public:
/// Constructs a non-recursive mutex.
///
/// If `checked` is true the mutex will attempt to check for misuse and
/// fatalError when detected. If `checked` is false (the default) the
/// mutex will make little to no effort to check for misuse (more efficient).
explicit RecursiveMutex(bool checked = false) {
threading_impl::recursive_mutex_init(Handle, checked);
}
~RecursiveMutex() { threading_impl::recursive_mutex_destroy(Handle); }

/// The lock() method has the following properties:
/// - Behaves as an atomic operation.
/// - Blocks the calling thread until exclusive ownership of the mutex
/// can be obtained.
/// - Prior m.unlock() operations on the same mutex synchronize-with
/// this lock operation.
/// - Does not throw exceptions but will halt on error (fatalError).
void lock() { threading_impl::recursive_mutex_lock(Handle); }

/// The unlock() method has the following properties:
/// - Behaves as an atomic operation.
/// - Releases the calling thread's ownership of the mutex and
/// synchronizes-with the subsequent successful lock operations on
/// the same object.
/// - The behavior is undefined if the calling thread does not own
/// the mutex.
/// - Does not throw exceptions but will halt on error (fatalError).
void unlock() { threading_impl::recursive_mutex_unlock(Handle); }

protected:
threading_impl::recursive_mutex_handle Handle;
};

} // namespace swift

#endif // SWIFT_THREADING_MUTEX_H
24 changes: 13 additions & 11 deletions stdlib/public/Concurrency/TaskPrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "swift/Runtime/Error.h"
#include "swift/Runtime/Exclusivity.h"
#include "swift/Runtime/HeapObject.h"
#include "swift/Threading/Mutex.h"
#include "swift/Threading/Thread.h"
#include "swift/Threading/ThreadSanitizer.h"
#include <atomic>
Expand Down Expand Up @@ -598,27 +599,25 @@ class alignas(2 * sizeof(void*)) ActiveTaskStatus {
#endif
}

/// Is there a lock on the linked list of status records?
/// Does some thread hold the status record lock?
bool isStatusRecordLocked() const { return Flags & IsStatusRecordLocked; }
ActiveTaskStatus withLockingRecord(TaskStatusRecord *lockRecord) const {
ActiveTaskStatus withStatusRecordLocked() const {
assert(!isStatusRecordLocked());
assert(lockRecord->Parent == Record);
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
return ActiveTaskStatus(lockRecord, Flags | IsStatusRecordLocked, ExecutionLock);
return ActiveTaskStatus(Record, Flags | IsStatusRecordLocked,
ExecutionLock);
#else
return ActiveTaskStatus(lockRecord, Flags | IsStatusRecordLocked);
return ActiveTaskStatus(Record, Flags | IsStatusRecordLocked);
#endif
}
ActiveTaskStatus withoutLockingRecord() const {
ActiveTaskStatus withoutStatusRecordLocked() const {
assert(isStatusRecordLocked());
assert(Record->getKind() == TaskStatusRecordKind::Private_RecordLock);

// Remove the lock record, and put the next one as the head
auto newRecord = Record->Parent;
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked, ExecutionLock);
return ActiveTaskStatus(Record, Flags & ~IsStatusRecordLocked,
ExecutionLock);
#else
return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked);
return ActiveTaskStatus(Record, Flags & ~IsStatusRecordLocked);
#endif
}

Expand Down Expand Up @@ -779,6 +778,9 @@ struct AsyncTask::PrivateStorage {
/// async task stack when it is needed.
TaskDependencyStatusRecord *dependencyRecord = nullptr;

// The lock used to protect more complicated operations on the task status.
RecursiveMutex statusLock;

// Always create an async task with max priority in ActiveTaskStatus = base
// priority. It will be updated later if needed.
PrivateStorage(JobPriority basePri)
Expand Down
Loading