Skip to content

gh-145685: Avoid contention on TYPE_LOCK in super() lookups #145775

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.

Sign up for GitHub

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
colesbury merged 3 commits into from
Mar 11, 2026
+32 −11
Merged

gh-145685: Avoid contention on TYPE_LOCK in super() lookups #145775

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3153d7f
gh-145685: Avoid contention on TYPE_LOCK in super() lookups
colesbury Mar 9, 2026
f6546b5
Refactor out _PyThreadState_PushCStackRefNew
colesbury Mar 10, 2026
5f50767
Add scaling benchmark
colesbury Mar 11, 2026
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
21 changes: 10 additions & 11 deletions Objects/typeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12360,18 +12360,17 @@ _super_lookup_descr(PyTypeObject *su_type, PyTypeObject *su_obj_type, PyObject *
PyObject *mro, *res;
Py_ssize_t i, n;

BEGIN_TYPE_LOCK();
mro = lookup_tp_mro(su_obj_type);
/* keep a strong reference to mro because su_obj_type->tp_mro can be
replaced during PyDict_GetItemRef(dict, name, &res) and because
another thread can modify it after we end the critical section
below */
Py_XINCREF(mro);
END_TYPE_LOCK();

if (mro == NULL)
return NULL;

/* Keep a strong reference to mro because su_obj_type->tp_mro can be
replaced during PyDict_GetItemRef(dict, name, &res). */
PyThreadState *tstate = _PyThreadState_GET();
_PyCStackRef mro_ref;
_PyThreadState_PushCStackRef(tstate, &mro_ref);
mro_ref.ref = PyStackRef_FromPyObjectNew(mro);
Copy link
Copy Markdown
Contributor

@mpage mpage Mar 10, 2026

Choose a reason for hiding this comment

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

Doesn't need to be in this PR, but would it be worth adding another function that combines these two operations?

Copy link
Copy Markdown
Contributor Author

@colesbury colesbury Mar 10, 2026

Choose a reason for hiding this comment

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

Yeah, makes sense.


assert(PyTuple_Check(mro));
n = PyTuple_GET_SIZE(mro);

Expand All @@ -12382,7 +12381,7 @@ _super_lookup_descr(PyTypeObject *su_type, PyTypeObject *su_obj_type, PyObject *
}
i++; /* skip su->type (if any) */
if (i >= n) {
Py_DECREF(mro);
_PyThreadState_PopCStackRef(tstate, &mro_ref);
return NULL;
}

Expand All @@ -12393,13 +12392,13 @@ _super_lookup_descr(PyTypeObject *su_type, PyTypeObject *su_obj_type, PyObject *

if (PyDict_GetItemRef(dict, name, &res) != 0) {
// found or error
Py_DECREF(mro);
_PyThreadState_PopCStackRef(tstate, &mro_ref);
return res;
}

i++;
} while (i < n);
Py_DECREF(mro);
_PyThreadState_PopCStackRef(tstate, &mro_ref);
return NULL;
}

Expand Down
Loading