Skip to content

Commit

Permalink
Use int32_t for sequence lock
Browse files Browse the repository at this point in the history
Use atomic storage instead of exchange
Use relaxed load when starting sequence lock for write
Fix some formatting
  • Loading branch information
DinoV committed Jan 17, 2024
1 parent 6d34906 commit d65909f
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 24 deletions.
3 changes: 3 additions & 0 deletions Include/cpython/pyatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,9 @@ _Py_atomic_store_int_release(int *obj, int value);
static inline int
_Py_atomic_load_int_acquire(const int *obj);

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj);


// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
3 changes: 3 additions & 0 deletions Include/cpython/pyatomic_gcc.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,9 @@ static inline int
_Py_atomic_load_int_acquire(const int *obj)
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); }

// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
11 changes: 11 additions & 0 deletions Include/cpython/pyatomic_msc.h
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,17 @@ _Py_atomic_load_int_acquire(const int *obj)
#endif
}

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{
#if defined(_M_X64) || defined(_M_IX86)
return *(uint32_t volatile *)obj;
#elif defined(_M_ARM64)
return (int)__ldar32((uint32_t volatile *)obj);
#else
# error "no implementation of _Py_atomic_load_uint32_acquire"
#endif
}

// --- _Py_atomic_fence ------------------------------------------------------

Expand Down
7 changes: 7 additions & 0 deletions Include/cpython/pyatomic_std.h
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,13 @@ _Py_atomic_load_int_acquire(const int *obj)
memory_order_acquire);
}

static inline uint32_t
_Py_atomic_load_uint32_acquire(const uint32_t *obj)
{
_Py_USING_STD;
return atomic_load_explicit((const _Atomic(uint32_t)*)obj,
memory_order_acquire);
}


// --- _Py_atomic_fence ------------------------------------------------------
Expand Down
8 changes: 4 additions & 4 deletions Include/internal/pycore_lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ PyAPI_FUNC(void) _PyRWMutex_Unlock(_PyRWMutex *rwmutex);
// The writer can also detect that the undelering data has not changed and abandon the write
// and restore the previous sequence.
typedef struct {
int sequence;
uint32_t sequence;
} _PySeqLock;

// Lock the sequence lock for the writer
Expand All @@ -275,15 +275,15 @@ PyAPI_FUNC(void) _PySeqLock_UnlockWrite(_PySeqLock *seqlock);
PyAPI_FUNC(void) _PySeqLock_AbandonWrite(_PySeqLock *seqlock);

// Begin a read operation and return the current sequence number.
PyAPI_FUNC(int) _PySeqLock_BeginRead(_PySeqLock *seqlock);
PyAPI_FUNC(uint32_t) _PySeqLock_BeginRead(_PySeqLock *seqlock);

// End the read operation and confirm that the sequence number has not changed.
// Returns 1 if the read was successful or 0 if the read should be re-tried.
PyAPI_FUNC(int) _PySeqLock_EndRead(_PySeqLock *seqlock, int previous);
PyAPI_FUNC(uint32_t) _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous);

// Check if the lock was held during a fork and clear the lock. Returns 1
// if the lock was held and any associated datat should be cleared.
PyAPI_FUNC(int) _PySeqLock_AfterFork(_PySeqLock *seqlock);
PyAPI_FUNC(uint32_t) _PySeqLock_AfterFork(_PySeqLock *seqlock);

#ifdef __cplusplus
}
Expand Down
12 changes: 6 additions & 6 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ class object "PyObject *" "&PyBaseObject_Type"
// in odd behaviors w.r.t. running with the GIL as the outer type lock could
// be released and reacquired during a subclass update if there's contention
// on the subclass lock.
#define BEGIN_TYPE_LOCK() \
{ \
_PyCriticalSection _cs; \
_PyCriticalSection_Begin(&_cs, &_PyRuntime.types.type_mutex); \
#define BEGIN_TYPE_LOCK() \
{ \
_PyCriticalSection _cs; \
_PyCriticalSection_Begin(&_cs, &_PyRuntime.types.type_mutex); \

#define END_TYPE_LOCK() \
_PyCriticalSection_End(&_cs); \
#define END_TYPE_LOCK() \
_PyCriticalSection_End(&_cs); \
}

#define ASSERT_TYPE_LOCK_HELD() \
Expand Down
30 changes: 16 additions & 14 deletions Python/lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -465,59 +465,61 @@ _PyRWMutex_Unlock(_PyRWMutex *rwmutex)
void _PySeqLock_LockWrite(_PySeqLock *seqlock)
{
// lock the entry by setting by moving to an odd sequence number
int prev = seqlock->sequence;
uint32_t prev = _Py_atomic_load_uint32_relaxed(&seqlock->sequence);
while (1) {
if (SEQLOCK_IS_UPDATING(prev)) {
// Someone else is currently updating the cache
_Py_yield();
prev = _Py_atomic_load_int32_relaxed(&seqlock->sequence);
} else if (_Py_atomic_compare_exchange_int32(&seqlock->sequence, &prev, prev + 1)) {
prev = _Py_atomic_load_uint32_relaxed(&seqlock->sequence);
}
else if (_Py_atomic_compare_exchange_uint32(&seqlock->sequence, &prev, prev + 1)) {
// We've locked the cache
break;
} else {
}
else {
_Py_yield();
}
}
}

void _PySeqLock_AbandonWrite(_PySeqLock *seqlock)
{
int new_seq = seqlock->sequence - 1;
uint32_t new_seq = seqlock->sequence - 1;
assert(!SEQLOCK_IS_UPDATING(new_seq));
_Py_atomic_exchange_int32(&seqlock->sequence, new_seq);
_Py_atomic_store_uint32(&seqlock->sequence, new_seq);
}

void _PySeqLock_UnlockWrite(_PySeqLock *seqlock)
{
int new_seq = seqlock->sequence + 1;
uint32_t new_seq = seqlock->sequence + 1;
assert(!SEQLOCK_IS_UPDATING(new_seq));
_Py_atomic_exchange_int32(&seqlock->sequence, new_seq);
_Py_atomic_store_uint32(&seqlock->sequence, new_seq);
}

int _PySeqLock_BeginRead(_PySeqLock *seqlock)
uint32_t _PySeqLock_BeginRead(_PySeqLock *seqlock)
{
int sequence = _Py_atomic_load_int_acquire(&seqlock->sequence);
uint32_t sequence = _Py_atomic_load_uint32_acquire(&seqlock->sequence);
while (SEQLOCK_IS_UPDATING(sequence)) {
_Py_yield();
sequence = _Py_atomic_load_int_acquire(&seqlock->sequence);
sequence = _Py_atomic_load_uint32_acquire(&seqlock->sequence);
}

return sequence;
}

int _PySeqLock_EndRead(_PySeqLock *seqlock, int previous)
uint32_t _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous)
{
// Synchronize again and validate that the entry hasn't been updated
// while we were readying the values.
if (_Py_atomic_load_int_acquire(&seqlock->sequence) == previous) {
if (_Py_atomic_load_uint32_acquire(&seqlock->sequence) == previous) {
return 1;
}

_Py_yield();
return 0;
}

int _PySeqLock_AfterFork(_PySeqLock *seqlock)
uint32_t _PySeqLock_AfterFork(_PySeqLock *seqlock)
{
// Synchronize again and validate that the entry hasn't been updated
// while we were readying the values.
Expand Down

0 comments on commit d65909f

Please sign in to comment.