Repository: cython/cython
Pull request: Limited API call speed improvments
1. Speed up `__Pyx_PyMethod_New` just by caching a reference to the MethodType. It's potentially called a lot so this is worthwhile.
2. Fix `__Pyx__IsSameCyOrCFunction` - this applies to the regular Cython build too (although in all cases it's only an optimization so doesn't give wrong results). It's most often called with a method object. In this case it always returns false which isn't the intent.
3. Fix logic for cpdef "override" check. For the Limited API the HEAP_TYPE check always passes so it always does the full check. It's hard to avoid this, but at least over the common case where the type is the exact extension type expected.

This gives something like a 20% speed improvement based on some very crude measurements. If fixes #6258 to the extent that compiling Cython with the Limited API is no longer a pessimization compared to not compiling Cython.

Problem statement:
Limited API performance
### Describe your issue

I've been looking at the performance of the Limited API. Just a very simple test where I compile Cython with the Limited API (you need to exclude refnanny right now) and then run `time cython Cython/Compiler/ExprNodes.py`

With Python 3.12 the broad conclusion is that an uncompiled build takes about 10s to process ExprNodes, a regular build takes 6.5s, and a limited api build takes about 12s.

So it's currently a performance regression vs running it in plain Python. My one conclusion so far is that it _isn't_ due to  `CYTHON_USE_MODULE_STATE` (which was my initial suspicion).

Some of this won't apply to a more typical use of Cython (calling external C functions, memoryviews, heavily typed code) since a lot of this won't be affected by the Limited API.

Hopefully we can do better long-term.

Hints from issue discussion:

Commit message: Merge branch 'master' into limited-api-performance

Patch to review:
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index cc11b636103..e41641d5646 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -4962,14 +4962,22 @@ def generate_execution_code(self, code):
             code.putln("else {")
         else:
             code.putln("else if (")
-            code.putln("#if CYTHON_USE_TYPE_SLOTS || CYTHON_COMPILING_IN_PYPY")
-            code.putln(f"unlikely(Py_TYPE({self_arg})->tp_dictoffset != 0)")
+            code.putln("#if !(CYTHON_USE_TYPE_SLOTS || CYTHON_COMPILING_IN_PYPY)")
+            # If CYTHON_USE_TYPE_SPECS then all extension types are heap-types so the check below automatically
+            # passes and thus takes the slow route.
+            # Therefore we do a less thorough check - if they type hasn't changed then clearly it hasn't
+            # been overridden, and if the type isn't GC then it also won't have been overridden.
+            if self.py_func.entry.scope.lookup_here("__dict__"):
+                code.putln("(1)")
+            else:
+                code.putln(f"unlikely(Py_TYPE({self_arg}) != "
+                           f"{self.py_func.entry.scope.parent_type.typeptr_cname} &&")
+                code.putln(f"__Pyx_PyType_HasFeature(Py_TYPE({self_arg}), Py_TPFLAGS_HAVE_GC))")
             code.putln("#else")
-            dict_str_const = code.get_py_string_const(EncodedString("__dict__"))
-            code.putln(f'PyObject_HasAttr({self_arg}, {dict_str_const})')
+            code.putln(f"unlikely(Py_TYPE({self_arg})->tp_dictoffset != 0 || "
+                       f"__Pyx_PyType_HasFeature(Py_TYPE({self_arg}), (Py_TPFLAGS_IS_ABSTRACT | Py_TPFLAGS_HEAPTYPE)))")
             code.putln("#endif")
-            code.putln(" || unlikely(__Pyx_PyType_HasFeature(Py_TYPE(%s), (Py_TPFLAGS_IS_ABSTRACT | Py_TPFLAGS_HEAPTYPE)))) {" % (
-                self_arg))
+            code.putln(") {")
 
         code.putln("#if CYTHON_USE_DICT_VERSIONS && CYTHON_USE_PYTYPE_LOOKUP && CYTHON_USE_TYPE_SLOTS")
         code.globalstate.use_utility_code(
diff --git a/Cython/Utility/CythonFunction.c b/Cython/Utility/CythonFunction.c
index 8551c6df897..a99ec5333d8 100644
--- a/Cython/Utility/CythonFunction.c
+++ b/Cython/Utility/CythonFunction.c
@@ -116,10 +116,11 @@ static PyObject * __Pyx_CyFunction_Vectorcall_FASTCALL_KEYWORDS_METHOD(PyObject
 //@requires: ObjectHandling.c::PyVectorcallFastCallDict
 //@requires: ModuleSetupCode.c::IncludeStructmemberH
 //@requires: ObjectHandling.c::PyObjectGetAttrStr
+//@requires: ObjectHandling.c::CachedMethodType
 //@requires: ExtensionTypes.c::CallTypeTraverse
 
 #if CYTHON_COMPILING_IN_LIMITED_API
-static CYTHON_INLINE int __Pyx__IsSameCyOrCFunction(PyObject *func, void *cfunc) {
+static CYTHON_INLINE int __Pyx__IsSameCyOrCFunctionNoMethod(PyObject *func, void *cfunc) {
     if (__Pyx_CyFunction_Check(func)) {
         return PyCFunction_GetFunction(((__pyx_CyFunctionObject*)func)->func) == (PyCFunction) cfunc;
     } else if (PyCFunction_Check(func)) {
@@ -127,8 +128,26 @@ static CYTHON_INLINE int __Pyx__IsSameCyOrCFunction(PyObject *func, void *cfunc)
     }
     return 0;
 }
+
+static CYTHON_INLINE int __Pyx__IsSameCyOrCFunction(PyObject *func, void *cfunc) {
+    if ((PyObject*)Py_TYPE(func) == __Pyx_CachedMethodType) {
+        int result;
+        PyObject *newFunc = PyObject_GetAttr(func, PYIDENT("__func__"));
+        if (unlikely(!newFunc)) {
+            PyErr_Clear(); // It's only an optimization, so don't throw an error
+            return 0;
+        }
+        result = __Pyx__IsSameCyOrCFunctionNoMethod(newFunc, cfunc);
+        Py_DECREF(newFunc);
+        return result;
+    }
+    return __Pyx__IsSameCyOrCFunctionNoMethod(func, cfunc);
+}
 #else
 static CYTHON_INLINE int __Pyx__IsSameCyOrCFunction(PyObject *func, void *cfunc) {
+    if (PyMethod_Check(func)) {
+        func = PyMethod_GET_FUNCTION(func);
+    }
     return __Pyx_CyOrPyCFunction_Check(func) && __Pyx_CyOrPyCFunction_GET_FUNCTION(func) == (PyCFunction) cfunc;
 }
 #endif
diff --git a/Cython/Utility/ObjectHandling.c b/Cython/Utility/ObjectHandling.c
index 9fa2c6f50d0..cd488865ea4 100644
--- a/Cython/Utility/ObjectHandling.c
+++ b/Cython/Utility/ObjectHandling.c
@@ -2685,22 +2685,42 @@ static CYTHON_INLINE int __Pyx_object_dict_version_matches(PyObject* obj, PY_UIN
 }
 #endif
 
+////////////// CachedMethodType.proto //////////////
+
+#if CYTHON_COMPILING_IN_LIMITED_API
+static PyObject *__Pyx_CachedMethodType = NULL;
+#endif
+
+////////////// CachedMethodType.init //////////////
+
+#if CYTHON_COMPILING_IN_LIMITED_API
+{
+    PyObject *typesModule=NULL;
+    typesModule = PyImport_ImportModule("types");
+    if (typesModule) {
+        __Pyx_CachedMethodType = PyObject_GetAttrString(typesModule, "MethodType");
+        Py_DECREF(typesModule);
+    }
+} // error handling follows
+#endif
 
 /////////////// PyMethodNew.proto ///////////////
+//@requires: CachedMethodType
 
 #if CYTHON_COMPILING_IN_LIMITED_API
 static PyObject *__Pyx_PyMethod_New(PyObject *func, PyObject *self, PyObject *typ) {
-    PyObject *typesModule=NULL, *methodType=NULL, *result=NULL;
+    PyObject *result;
     CYTHON_UNUSED_VAR(typ);
     if (!self)
         return __Pyx_NewRef(func);
-    typesModule = PyImport_ImportModule("types");
-    if (!typesModule) return NULL;
-    methodType = PyObject_GetAttrString(typesModule, "MethodType");
-    Py_DECREF(typesModule);
-    if (!methodType) return NULL;
-    result = PyObject_CallFunctionObjArgs(methodType, func, self, NULL);
-    Py_DECREF(methodType);
+    #if __PYX_LIMITED_VERSION_HEX >= 0x030C0000
+    {
+        PyObject *args[] = {func, self};
+        result = PyObject_Vectorcall(__Pyx_CachedMethodType, args, 2, NULL);
+    }
+    #else
+    result = PyObject_CallFunctionObjArgs(__Pyx_CachedMethodType, func, self, NULL);
+    #endif
     return result;
 }
 #else
diff --git a/tests/run/pstats_profile_test.pyx b/tests/run/pstats_profile_test.pyx
index 944bdd9fb26..7a3bd28f0f4 100644
--- a/tests/run/pstats_profile_test.pyx
+++ b/tests/run/pstats_profile_test.pyx
@@ -47,7 +47,7 @@ u"""
     200
     >>> short_stats['m_cdef']
     100
-    >>> short_stats['m_cpdef'] - (200 if CPDEF_METHODS_COUNT_TWICE else 0)  # FIXME!
+    >>> short_stats['m_cpdef']
     300
 
     >>> try:
@@ -118,14 +118,6 @@ u"""
 cimport cython
 
 
-# FIXME: With type specs, cpdef methods are currently counted twice.
-# https://github.com/cython/cython/issues/2137
-cdef extern from *:
-    int CYTHON_USE_TYPE_SPECS
-
-CPDEF_METHODS_COUNT_TWICE = CYTHON_USE_TYPE_SPECS
-
-
 def callees(pstats, target_caller):
     pstats.calc_callees()
     for (_, _, caller), callees in pstats.all_callees.items():


Code review comments:
diff_hunk: @@ -4962,14 +4962,22 @@ def generate_execution_code(self, code):
             code.putln("else {")
         else:
             code.putln("else if (")
-            code.putln("#if CYTHON_USE_TYPE_SLOTS || CYTHON_COMPILING_IN_PYPY")
-            code.putln(f"unlikely(Py_TYPE({self_arg})->tp_dictoffset != 0)")
+            code.putln("#if !(CYTHON_USE_TYPE_SLOTS || CYTHON_COMPILING_IN_PYPY)")
+            # If CYTHON_USE_TYPE_SPECS then all extension types are heap-types so the check below automatically
+            # passes and thus takes the slow route.
+            # Therefore we do a less thorough check - if they type hasn't changed then clearly it hasn't
+            # been overridden, and if the type isn't GC then it also won't have been overridden.
+            if self.py_func.entry.scope.lookup_here("__dict__"):
+                code.putln("(1)")
path: Cython/Compiler/Nodes.py
text: @user1:
Isn't this something general that could go next to the `is_module_scope` above, i.e. use no C code condition at all?

@author:
Moved.... I was originally trying not to change the non-limited-api path at all, but this should be fairly minor.