gh-124153: Clean up workarounds for PyType_GetBaseByToken() performance

Objects/typeobject.c

@@ -5288,27 +5288,59 @@ _PyType_GetModuleByDef2(PyTypeObject *left, PyTypeObject *right,
 }
 
 
+// PyType_GetBaseByToken() family are better to be placed from most to least
+// frequent in the src and obj files (the decls in random order) for PGO on
+// MSVC. They'll be optimized not for speed if a cold sub function precedes,
+// in which case the unrelated functions above can be slower if there are
+// common conds such as `if (!_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE))`.
+static int
+check_base_by_token(PyTypeObject *, void *);
+static inline PyTypeObject *
+get_base_by_token_from_mro(PyTypeObject *, void *);
 static PyTypeObject *
-get_base_by_token_recursive(PyTypeObject *type, void *token)
+get_base_by_token_recursive(PyTypeObject *, void *);
+
+int
+PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result)
 {
-    assert(PyType_GetSlot(type, Py_tp_token) != token);
-    PyObject *bases = lookup_tp_bases(type);
-    assert(bases != 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));
-        if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) {
-            continue;
-        }
-        if (((PyHeapTypeObject*)base)->ht_token == token) {
-            return base;
-        }
-        base = get_base_by_token_recursive(base, token);
-        if (base != NULL) {
-            return base;
-        }
+    // Goto jumps here can disturb the PGO collection on MSVC
+    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);
+    }
+
+    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;
+    }
+    if (((PyHeapTypeObject*)type)->ht_token == token) {
+        *result = (PyTypeObject *)Py_NewRef(type);
+        return 1;
+    }
+
+    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;
+    }
+    else {
+        *result = NULL;
+        return 0;
     }
-    return NULL;
 }
 
 static inline PyTypeObject *
@@ -5340,24 +5372,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;
     }
@@ -5366,47 +5397,27 @@ check_base_by_token(PyTypeObject *type, void *token) {
     }
 }
 
-int
-PyType_GetBaseByToken(PyTypeObject *type, void *token, PyTypeObject **result)
+static PyTypeObject *
+get_base_by_token_recursive(PyTypeObject *type, void *token)
 {
-    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);
-    }
-
-    // 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) {
-        *result = (PyTypeObject *)Py_NewRef(type);
-        return 1;
-    }
-    else 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;
-    }
-    else {
-        *result = NULL;
-        return 0;
+    assert(PyType_GetSlot(type, Py_tp_token) != token);
+    PyObject *bases = lookup_tp_bases(type);
+    assert(bases != 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));
+        if (!_PyType_HasFeature(base, Py_TPFLAGS_HEAPTYPE)) {
+            continue;
+        }
+        if (((PyHeapTypeObject*)base)->ht_token == token) {
+            return base;
+        }
+        base = get_base_by_token_recursive(base, token);
+        if (base != NULL) {
+            return base;
+        }
     }
+    return NULL;
 }