Skip to content

gh-124153: Fix unstable optimization of PyType_GetBaseByToken() and friends #124488

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
encukou merged 14 commits into from
Oct 10, 2024
Merged

gh-124153: Fix unstable optimization of PyType_GetBaseByToken() and friends #124488

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
3b5f12b
abandon if-else workaround for maintainability
neonene Sep 25, 2024
a0dae73
do not pressure PyType_GetModuleByDef() and others
neonene Oct 8, 2024
f1fa9f4
ready for moving get_base_by_token_from_mro()
neonene Oct 8, 2024
cae44aa
move get_base_by_token_from_mro() into get/check funcs
neonene Oct 8, 2024
230e06d
simplify check_base_by_token()
neonene Oct 8, 2024
55ca7b8
Merge branch 'main' into opttoken
neonene Oct 8, 2024
9285a99
Merge branch 'main' into opttoken
neonene Oct 8, 2024
75f600f
remove os-specific comments
neonene Oct 8, 2024
2e7bb29
remove redundant code
neonene Oct 8, 2024
d636fa8
remove check_base_by_token()
neonene Oct 9, 2024
b83b63a
reword
neonene Oct 9, 2024
c02f0a6
perf tuning
neonene Oct 9, 2024
07de01b
reduce gotos
neonene Oct 10, 2024
0fc1c90
add comment
neonene Oct 10, 2024
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
Next Next commit
ready for moving get_base_by_token_from_mro() 
PyTuple_GET_SIZE() calls PyTuple_Check()
  • Loading branch information
@neonene
neonene committed Oct 8, 2024
commit f1fa9f44d986ec2d6e90f22c2365616928fdf63d
44 changes: 20 additions & 24 deletions Objects/typeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5315,30 +5315,24 @@ get_base_by_token_recursive(PyObject *bases, void *token)
}

static inline PyTypeObject *
get_base_by_token_from_mro(PyTypeObject *type, void *token)
get_base_by_token_from_mro(PyObject *mro, void *token)
{
// Bypass lookup_tp_mro() as PyType_IsSubtype() does
PyObject *mro = type->tp_mro;
assert(mro != NULL);
assert(PyTuple_Check(mro));
// mro_invoke() ensures that the type MRO cannot be empty.
assert(PyTuple_GET_SIZE(mro) >= 1);
// Also, the first item in the MRO is the type itself, which is supposed
// to be already checked by the caller. We skip it in the loop.
// Also, the first item in the MRO is the type itself, which
// we already checked above. We skip it in the loop.
assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type);
assert(PyType_GetSlot(type, Py_tp_token) != token);

PyTypeObject *res = NULL;
Py_ssize_t n = PyTuple_GET_SIZE(mro);
for (Py_ssize_t i = 1; i < n; i++) {
PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(mro, i));
if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) {
continue;
}
if (((PyHeapTypeObject*)base)->ht_token == token) {
return base;
PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i);
if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)
&& ((PyHeapTypeObject*)base)->ht_token == token) {
res = base;
break;
}
}
return NULL;
return res;
}

static int
Expand All @@ -5359,12 +5353,13 @@ check_base_by_token(PyTypeObject *type, void *token) {
if (((PyHeapTypeObject*)type)->ht_token == token) {
return 1;
}
if (type->tp_mro != NULL) {
// This will not be inlined
return get_base_by_token_from_mro(type, token) ? 1 : 0;
PyObject *mro = type->tp_mro;
if (mro == NULL) {
return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0;
}
else {
return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0;
// This will not be inlined
return get_base_by_token_from_mro(type, token) ? 1 : 0;
}
}

Expand Down Expand Up @@ -5393,12 +5388,13 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result)
}

PyTypeObject *base;
if (type->tp_mro != NULL) {
// Expect this to be inlined
base = get_base_by_token_from_mro(type, token);
PyObject *mro = type->tp_mro; // No lookup, following PyType_IsSubtype()
if (mro == NULL) {
base = get_base_by_token_recursive(lookup_tp_bases(type), token);
}
else {
base = get_base_by_token_recursive(lookup_tp_bases(type), token);
// Expect this to be inlined
base = get_base_by_token_from_mro(type, token);
}
if (base != NULL) {
*result = (PyTypeObject *)Py_NewRef(base);
Expand Down