From 3b5f12b5b03ba4703ba4caba8a4862d9b1831a89 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 25 Sep 2024 11:57:39 +0900 Subject: [PATCH 01/12] abandon if-else workaround for maintainability --- Objects/typeobject.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 68e481f8e5163b..c3edf7da3cae06 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5340,24 +5340,23 @@ get_base_by_token_from_mro(PyTypeObject *type, void *token) static int check_base_by_token(PyTypeObject *type, void *token) { - // Chain the branches, which will be optimized exclusive here if (token == NULL) { PyErr_Format(PyExc_SystemError, "PyType_GetBaseByToken called with token=NULL"); return -1; } - else if (!PyType_Check(type)) { + if (!PyType_Check(type)) { PyErr_Format(PyExc_TypeError, "expected a type, got a '%T' object", type); return -1; } - else if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { + if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { return 0; } - else if (((PyHeapTypeObject*)type)->ht_token == token) { + if (((PyHeapTypeObject*)type)->ht_token == token) { return 1; } - else if (type->tp_mro != NULL) { + if (type->tp_mro != NULL) { // This will not be inlined return get_base_by_token_from_mro(type, token) ? 1 : 0; } @@ -5379,26 +5378,25 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) return check_base_by_token(type, token); } - // Chain the branches, which will be optimized exclusive here - PyTypeObject *base; if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { // No static type has a heaptype superclass, // which is ensured by type_ready_mro(). *result = NULL; return 0; } - else if (((PyHeapTypeObject*)type)->ht_token == token) { + if (((PyHeapTypeObject*)type)->ht_token == token) { *result = (PyTypeObject *)Py_NewRef(type); return 1; } - else if (type->tp_mro != NULL) { + + PyTypeObject *base; + if (type->tp_mro != NULL) { // Expect this to be inlined base = get_base_by_token_from_mro(type, token); } else { base = get_base_by_token_recursive(type, token); } - if (base != NULL) { *result = (PyTypeObject *)Py_NewRef(base); return 1; From a0dae738f2c674ce8365ca266b18adf5567c420c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:58:36 +0900 Subject: [PATCH 02/12] do not pressure PyType_GetModuleByDef() and others --- Objects/typeobject.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index c3edf7da3cae06..5dc3d6577fd327 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5289,11 +5289,12 @@ _PyType_GetModuleByDef2(PyTypeObject *left, PyTypeObject *right, static PyTypeObject * -get_base_by_token_recursive(PyTypeObject *type, void *token) +get_base_by_token_recursive(PyObject *bases, void *token) { - assert(PyType_GetSlot(type, Py_tp_token) != token); - PyObject *bases = lookup_tp_bases(type); assert(bases != NULL); + // MSVC: The result should be initialized outside the loop to avoid the c + // file being less PGO-optimized. Returning it from inside seems to be OK. + PyTypeObject *res = NULL; Py_ssize_t n = PyTuple_GET_SIZE(bases); for (Py_ssize_t i = 0; i < n; i++) { PyTypeObject *base = _PyType_CAST(PyTuple_GET_ITEM(bases, i)); @@ -5301,14 +5302,16 @@ get_base_by_token_recursive(PyTypeObject *type, void *token) continue; } if (((PyHeapTypeObject*)base)->ht_token == token) { - return base; + res = base; + break; } - base = get_base_by_token_recursive(base, token); + base = get_base_by_token_recursive(lookup_tp_bases(base), token); if (base != NULL) { - return base; + res = base; + break; } } - return NULL; + return res; // Prefer to return in one place } static inline PyTypeObject * @@ -5361,7 +5364,7 @@ check_base_by_token(PyTypeObject *type, void *token) { return get_base_by_token_from_mro(type, token) ? 1 : 0; } else { - return get_base_by_token_recursive(type, token) ? 1 : 0; + return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0; } } @@ -5395,7 +5398,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) base = get_base_by_token_from_mro(type, token); } else { - base = get_base_by_token_recursive(type, token); + base = get_base_by_token_recursive(lookup_tp_bases(type), token); } if (base != NULL) { *result = (PyTypeObject *)Py_NewRef(base); From f1fa9f44d986ec2d6e90f22c2365616928fdf63d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:00:17 +0900 Subject: [PATCH 03/12] ready for moving get_base_by_token_from_mro() PyTuple_GET_SIZE() calls PyTuple_Check() --- Objects/typeobject.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 5dc3d6577fd327..e3e3c0f6825598 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -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 @@ -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; } } @@ -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); From cae44aa7008a8a5f48b0709d35c288816e65e40c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:01:06 +0900 Subject: [PATCH 04/12] move get_base_by_token_from_mro() into get/check funcs --- Objects/typeobject.c | 65 ++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e3e3c0f6825598..81be8a627081d5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5314,27 +5314,6 @@ get_base_by_token_recursive(PyObject *bases, void *token) return res; // Prefer to return in one place } -static inline PyTypeObject * -get_base_by_token_from_mro(PyObject *mro, void *token) -{ - // 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 - // we already checked above. We skip it in the loop. - assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); - PyTypeObject *res = NULL; - Py_ssize_t n = PyTuple_GET_SIZE(mro); - for (Py_ssize_t i = 1; i < n; i++) { - PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); - if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) - && ((PyHeapTypeObject*)base)->ht_token == token) { - res = base; - break; - } - } - return res; -} - static int check_base_by_token(PyTypeObject *type, void *token) { if (token == NULL) { @@ -5358,8 +5337,22 @@ check_base_by_token(PyTypeObject *type, void *token) { return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0; } else { - // This will not be inlined - return get_base_by_token_from_mro(type, token) ? 1 : 0; + // 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 + // we already checked above. We skip it in the loop. + assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); + PyTypeObject *res = NULL; + Py_ssize_t n = PyTuple_GET_SIZE(mro); + for (Py_ssize_t i = 1; i < n; i++) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); + if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) + && ((PyHeapTypeObject*)base)->ht_token == token) { + res = base; + break; + } + } + return res ? 1 : 0; } } @@ -5375,7 +5368,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) *result = NULL; return check_base_by_token(type, token); } - + // Prefer not to use gotos here for Windows PGO if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { // No static type has a heaptype superclass, // which is ensured by type_ready_mro(). @@ -5387,17 +5380,29 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) return 1; } - PyTypeObject *base; + PyTypeObject *res = NULL; PyObject *mro = type->tp_mro; // No lookup, following PyType_IsSubtype() if (mro == NULL) { - base = get_base_by_token_recursive(lookup_tp_bases(type), token); + res = get_base_by_token_recursive(lookup_tp_bases(type), token); } else { - // Expect this to be inlined - base = get_base_by_token_from_mro(type, token); + // 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 + // we already checked above. We skip it in the loop. + assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); + Py_ssize_t n = PyTuple_GET_SIZE(mro); + for (Py_ssize_t i = 1; i < n; i++) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); + if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) + && ((PyHeapTypeObject*)base)->ht_token == token) { + res = base; + break; + } + } } - if (base != NULL) { - *result = (PyTypeObject *)Py_NewRef(base); + if (res != NULL) { + *result = (PyTypeObject *)Py_NewRef(res); return 1; } else { From 230e06dc46e51782bc40ef1d6109a49f8e45ffd4 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:02:16 +0900 Subject: [PATCH 05/12] simplify check_base_by_token() --- Objects/typeobject.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 81be8a627081d5..f39634badbcb43 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5315,7 +5315,8 @@ get_base_by_token_recursive(PyObject *bases, void *token) } static int -check_base_by_token(PyTypeObject *type, void *token) { +check_base_by_token(PyTypeObject *type, void *token) +{ if (token == NULL) { PyErr_Format(PyExc_SystemError, "PyType_GetBaseByToken called with token=NULL"); @@ -5337,22 +5338,17 @@ check_base_by_token(PyTypeObject *type, void *token) { return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0; } else { - // 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 - // we already checked above. We skip it in the loop. assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); - PyTypeObject *res = NULL; - Py_ssize_t n = PyTuple_GET_SIZE(mro); - for (Py_ssize_t i = 1; i < n; i++) { + // Find from the last when only checking + for (Py_ssize_t i = PyTuple_GET_SIZE(mro) - 1; i >= 1; i--) { PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) && ((PyHeapTypeObject*)base)->ht_token == token) { - res = base; - break; + return 1; } } - return res ? 1 : 0; + return 0; } } From 75f600f460d93ba50ea12b3f7938e3d7a49326b7 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Oct 2024 22:50:32 +0900 Subject: [PATCH 06/12] remove os-specific comments --- Objects/typeobject.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 35ad586e67574e..9c1da5fe74e4dc 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5272,8 +5272,6 @@ static PyTypeObject * get_base_by_token_recursive(PyObject *bases, void *token) { assert(bases != NULL); - // MSVC: The result should be initialized outside the loop to avoid the c - // file being less PGO-optimized. Returning it from inside seems to be OK. PyTypeObject *res = NULL; Py_ssize_t n = PyTuple_GET_SIZE(bases); for (Py_ssize_t i = 0; i < n; i++) { @@ -5291,7 +5289,7 @@ get_base_by_token_recursive(PyObject *bases, void *token) break; } } - return res; // Prefer to return in one place + return res; } static int @@ -5344,7 +5342,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) *result = NULL; return check_base_by_token(type, token); } - // Prefer not to use gotos here for Windows PGO + if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { // No static type has a heaptype superclass, // which is ensured by type_ready_mro(). From 2e7bb29ba095a21f9b03bc0631fa9f7713f08c86 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 9 Oct 2024 01:17:47 +0900 Subject: [PATCH 07/12] remove redundant code --- Objects/typeobject.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9c1da5fe74e4dc..5f330410a32cda 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5283,9 +5283,8 @@ get_base_by_token_recursive(PyObject *bases, void *token) res = base; break; } - base = get_base_by_token_recursive(lookup_tp_bases(base), token); - if (base != NULL) { - res = base; + res = get_base_by_token_recursive(lookup_tp_bases(base), token); + if (res != NULL) { break; } } From d636fa803d28fd470608544fa1acd46c28028d57 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 9 Oct 2024 22:03:35 +0900 Subject: [PATCH 08/12] remove check_base_by_token() --- Objects/typeobject.c | 105 ++++++++++++++++--------------------------- 1 file changed, 39 insertions(+), 66 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 5f330410a32cda..7895270541eda3 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5291,97 +5291,70 @@ get_base_by_token_recursive(PyObject *bases, void *token) return res; } -static int -check_base_by_token(PyTypeObject *type, void *token) +int +PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) { if (token == NULL) { PyErr_Format(PyExc_SystemError, "PyType_GetBaseByToken called with token=NULL"); - return -1; + goto error; } if (!PyType_Check(type)) { PyErr_Format(PyExc_TypeError, "expected a type, got a '%T' object", type); - return -1; - } - if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { - return 0; - } - if (((PyHeapTypeObject*)type)->ht_token == token) { - return 1; - } - PyObject *mro = type->tp_mro; - if (mro == NULL) { - return get_base_by_token_recursive(lookup_tp_bases(type), token) ? 1 : 0; - } - else { - assert(PyTuple_GET_SIZE(mro) >= 1); - assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); - // Find from the last when only checking - for (Py_ssize_t i = PyTuple_GET_SIZE(mro) - 1; i >= 1; i--) { - PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); - if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) - && ((PyHeapTypeObject*)base)->ht_token == token) { - return 1; - } - } - return 0; - } -} - -int -PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) -{ - if (result == NULL) { - // If the `result` is checked only once here, the subsequent - // branches will become trivial to optimize. - return check_base_by_token(type, token); - } - if (token == NULL || !PyType_Check(type)) { - *result = NULL; - return check_base_by_token(type, token); + goto error; } if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { // No static type has a heaptype superclass, // which is ensured by type_ready_mro(). - *result = NULL; - return 0; + goto not_found; } if (((PyHeapTypeObject*)type)->ht_token == token) { - *result = (PyTypeObject *)Py_NewRef(type); +found: + if (result != NULL) { + *result = (PyTypeObject *)Py_NewRef(type); + } return 1; } - - PyTypeObject *res = NULL; PyObject *mro = type->tp_mro; // No lookup, following PyType_IsSubtype() if (mro == NULL) { - res = get_base_by_token_recursive(lookup_tp_bases(type), token); + PyTypeObject *base; + base = get_base_by_token_recursive(lookup_tp_bases(type), token); + if (base != NULL) { + // Copying the given type can cause a slowdown, + // unlike this overwriting. + type = base; + goto found; + } + goto not_found; } - else { - // 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 - // we already checked above. We skip it in the loop. - assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); - Py_ssize_t n = PyTuple_GET_SIZE(mro); - for (Py_ssize_t i = 1; i < n; i++) { - PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); - if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) - && ((PyHeapTypeObject*)base)->ht_token == token) { - res = base; - break; - } + // 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 + // we already checked above. We skip it in the loop. + assert(PyTuple_GET_ITEM(mro, 0) == (PyObject *)type); + Py_ssize_t n = PyTuple_GET_SIZE(mro); + for (Py_ssize_t i = 1; i < n; i++) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); + if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) { + continue; + } + if (((PyHeapTypeObject*)base)->ht_token == token) { + type = base; + goto found; } } - if (res != NULL) { - *result = (PyTypeObject *)Py_NewRef(res); - return 1; +not_found: + if (result != NULL) { + *result = NULL; } - else { + return 0; +error: + if (result != NULL) { *result = NULL; - return 0; } + return -1; } From b83b63a08b0ad1cb1c7f38d62a182243971e9578 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 9 Oct 2024 22:07:25 +0900 Subject: [PATCH 09/12] reword --- Objects/typeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7895270541eda3..b40b07f3d9a1b6 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5323,7 +5323,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) base = get_base_by_token_recursive(lookup_tp_bases(type), token); if (base != NULL) { // Copying the given type can cause a slowdown, - // unlike this overwriting. + // unlike overwriting below. type = base; goto found; } From c02f0a6f6692705ddb6f24a84bd1f77e3cda4c19 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 10 Oct 2024 00:54:55 +0900 Subject: [PATCH 10/12] perf tuning Including the revert: 2e7bb29 --- Objects/typeobject.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b40b07f3d9a1b6..aacf9cd8fdeebf 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5283,8 +5283,9 @@ get_base_by_token_recursive(PyObject *bases, void *token) res = base; break; } - res = get_base_by_token_recursive(lookup_tp_bases(base), token); - if (res != NULL) { + base = get_base_by_token_recursive(lookup_tp_bases(base), token); + if (base != NULL) { + res = base; break; } } @@ -5323,7 +5324,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) base = get_base_by_token_recursive(lookup_tp_bases(type), token); if (base != NULL) { // Copying the given type can cause a slowdown, - // unlike overwriting below. + // unlike the overwrite below. type = base; goto found; } @@ -5337,10 +5338,8 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) Py_ssize_t n = PyTuple_GET_SIZE(mro); for (Py_ssize_t i = 1; i < n; i++) { PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(mro, i); - if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) { - continue; - } - if (((PyHeapTypeObject*)base)->ht_token == token) { + if (_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE) + && ((PyHeapTypeObject*)base)->ht_token == token) { type = base; goto found; } From 07de01b1d84c4da57128a825cf044933867c3c36 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 10 Oct 2024 10:58:40 +0900 Subject: [PATCH 11/12] reduce gotos not to impact PyType_GetSlot() and others. --- Objects/typeobject.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index aacf9cd8fdeebf..5d6f3839a7bc7d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5295,21 +5295,25 @@ get_base_by_token_recursive(PyObject *bases, void *token) int PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) { + if (result != NULL) { + *result = NULL; + } + if (token == NULL) { PyErr_Format(PyExc_SystemError, "PyType_GetBaseByToken called with token=NULL"); - goto error; + return -1; } if (!PyType_Check(type)) { PyErr_Format(PyExc_TypeError, "expected a type, got a '%T' object", type); - goto error; + return -1; } if (!_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) { // No static type has a heaptype superclass, // which is ensured by type_ready_mro(). - goto not_found; + return 0; } if (((PyHeapTypeObject*)type)->ht_token == token) { found: @@ -5328,7 +5332,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) type = base; goto found; } - goto not_found; + return 0; } // mro_invoke() ensures that the type MRO cannot be empty. assert(PyTuple_GET_SIZE(mro) >= 1); @@ -5344,16 +5348,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) goto found; } } -not_found: - if (result != NULL) { - *result = NULL; - } return 0; -error: - if (result != NULL) { - *result = NULL; - } - return -1; } From 0fc1c9093901dd8c19571755102fdf78416faf30 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:01:59 +0900 Subject: [PATCH 12/12] add comment --- Objects/typeobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 5d6f3839a7bc7d..5380633fa1149e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5289,7 +5289,7 @@ get_base_by_token_recursive(PyObject *bases, void *token) break; } } - return res; + return res; // Prefer to return recursively from one place } int @@ -5322,6 +5322,7 @@ PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result) } return 1; } + PyObject *mro = type->tp_mro; // No lookup, following PyType_IsSubtype() if (mro == NULL) { PyTypeObject *base;