Skip to content
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

gh-111926: Avoid locking in PyType_IsSubtype #117275

Merged
merged 5 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Temporarily remove thread safety when performing PyType_IsSubtype
  • Loading branch information
mpage committed Mar 28, 2024
commit e5c7b59d2f79a90e365e833ab99c9a363014fe33
5 changes: 0 additions & 5 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,18 @@ extern "C" {
#endif

#ifdef Py_GIL_DISABLED
#define FT_ATOMIC_LOAD_PTR(value) _Py_atomic_load_ptr(&value)
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
_Py_atomic_load_ssize_relaxed(&value)
#define FT_ATOMIC_STORE_PTR(value, new_value) \
_Py_atomic_store_ptr(&value, new_value)
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
_Py_atomic_store_ptr_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
_Py_atomic_store_ptr_release(&value, new_value)
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
_Py_atomic_store_ssize_relaxed(&value, new_value)
#else
#define FT_ATOMIC_LOAD_PTR(value) value
#define FT_ATOMIC_LOAD_SSIZE(value) value
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
Expand Down
7 changes: 0 additions & 7 deletions Objects/tupleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1161,13 +1161,6 @@ static inline int
maybe_freelist_push(PyTupleObject *op)
{
#ifdef WITH_FREELISTS
#ifdef Py_GIL_DISABLED
if (_PyObject_GC_IS_SHARED_INLINE((PyObject *) op)) {
// There may still be threads that are concurrently reading from the
// tuple.
return 0;
}
#endif
struct _Py_object_freelists *freelists = _Py_object_freelists_GET();
if (Py_SIZE(op) == 0) {
return 0;
Expand Down
18 changes: 6 additions & 12 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#include "Python.h"
#include "pycore_abstract.h" // _PySequence_IterSearch()
#include "pycore_pyatomic_ft_wrappers.h"
#include "pycore_call.h" // _PyObject_VectorcallTstate()
#include "pycore_code.h" // CO_FAST_FREE
#include "pycore_dict.h" // _PyDict_KeysSize()
Expand Down Expand Up @@ -367,14 +366,15 @@ clear_tp_bases(PyTypeObject *self)
static inline PyObject *
lookup_tp_mro(PyTypeObject *self)
{
return FT_ATOMIC_LOAD_PTR(self->tp_mro);
ASSERT_TYPE_LOCK_HELD();
return self->tp_mro;
}

PyObject *
_PyType_GetMRO(PyTypeObject *self)
{
PyObject *mro = lookup_tp_mro(self);
#ifdef Py_GIL_DISABLED
PyObject *mro = _Py_atomic_load_ptr_relaxed(&self->tp_mro);
if (mro == NULL) {
return NULL;
}
Expand All @@ -388,7 +388,7 @@ _PyType_GetMRO(PyTypeObject *self)
END_TYPE_LOCK()
return mro;
#else
return Py_XNewRef(mro);
return Py_XNewRef(lookup_tp_mro(self));
#endif
}

Expand All @@ -404,13 +404,7 @@ set_tp_mro(PyTypeObject *self, PyObject *mro)
/* Other checks are done via set_tp_bases. */
_Py_SetImmortal(mro);
}
#ifdef Py_GIL_DISABLED
if (self->tp_mro != NULL) {
// Allow concurrent reads from PyType_IsSubtype
_PyObject_GC_SET_SHARED_INLINE(self->tp_mro);
}
#endif
FT_ATOMIC_STORE_PTR(self->tp_mro, mro);
self->tp_mro = mro;
}

static inline void
Expand Down Expand Up @@ -2347,7 +2341,7 @@ is_subtype_with_mro(PyObject *a_mro, PyTypeObject *a, PyTypeObject *b)
int
PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b)
{
return is_subtype_with_mro(lookup_tp_mro(a), a, b);
return is_subtype_with_mro(a->tp_mro, a, b);
}

/* Routines to do a method lookup in the type without looking in the
Expand Down
Loading