gh-134637: Fix performance regression in calling ctypes function pointer in free threading. (#134702)

Fix performance regression in calling `ctypes` function pointer in `free threading`.
This commit is contained in:
Kumar Aditya 2025-05-26 23:56:40 +05:30 committed by GitHub
parent 56743afe87
commit 3c0525126e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 91 additions and 52 deletions

View File

@ -0,0 +1 @@
Fix performance regression in calling a :mod:`ctypes` function pointer in :term:`free threading`.

View File

@ -3610,6 +3610,45 @@ generic_pycdata_new(ctypes_state *st,
PyCFuncPtr_Type PyCFuncPtr_Type
*/ */
static inline void
atomic_xsetref(PyObject **field, PyObject *value)
{
#ifdef Py_GIL_DISABLED
PyObject *old = *field;
_Py_atomic_store_ptr(field, value);
Py_XDECREF(old);
#else
Py_XSETREF(*field, value);
#endif
}
/*
This function atomically loads the reference from *field, and
tries to get a new reference to it. If the incref fails,
it acquires critical section of obj and returns a new reference to the *field.
In the general case, this avoids contention on acquiring the critical section.
*/
static inline PyObject *
atomic_xgetref(PyObject *obj, PyObject **field)
{
#ifdef Py_GIL_DISABLED
PyObject *value = _Py_atomic_load_ptr(field);
if (value == NULL) {
return NULL;
}
if (_Py_TryIncrefCompare(field, value)) {
return value;
}
Py_BEGIN_CRITICAL_SECTION(obj);
value = Py_XNewRef(*field);
Py_END_CRITICAL_SECTION();
return value;
#else
return Py_XNewRef(*field);
#endif
}
/*[clinic input] /*[clinic input]
@critical_section @critical_section
@setter @setter
@ -3626,7 +3665,7 @@ _ctypes_CFuncPtr_errcheck_set_impl(PyCFuncPtrObject *self, PyObject *value)
return -1; return -1;
} }
Py_XINCREF(value); Py_XINCREF(value);
Py_XSETREF(self->errcheck, value); atomic_xsetref(&self->errcheck, value);
return 0; return 0;
} }
@ -3658,12 +3697,10 @@ static int
_ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value) _ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value)
/*[clinic end generated code: output=0be0a086abbabf18 input=683c3bef4562ccc6]*/ /*[clinic end generated code: output=0be0a086abbabf18 input=683c3bef4562ccc6]*/
{ {
PyObject *checker, *oldchecker; PyObject *checker;
if (value == NULL) { if (value == NULL) {
oldchecker = self->checker; atomic_xsetref(&self->restype, NULL);
self->checker = NULL; atomic_xsetref(&self->checker, NULL);
Py_CLEAR(self->restype);
Py_XDECREF(oldchecker);
return 0; return 0;
} }
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self))); ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
@ -3679,11 +3716,9 @@ _ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value)
if (PyObject_GetOptionalAttr(value, &_Py_ID(_check_retval_), &checker) < 0) { if (PyObject_GetOptionalAttr(value, &_Py_ID(_check_retval_), &checker) < 0) {
return -1; return -1;
} }
oldchecker = self->checker;
self->checker = checker;
Py_INCREF(value); Py_INCREF(value);
Py_XSETREF(self->restype, value); atomic_xsetref(&self->checker, checker);
Py_XDECREF(oldchecker); atomic_xsetref(&self->restype, value);
return 0; return 0;
} }
@ -3728,16 +3763,16 @@ _ctypes_CFuncPtr_argtypes_set_impl(PyCFuncPtrObject *self, PyObject *value)
PyObject *converters; PyObject *converters;
if (value == NULL || value == Py_None) { if (value == NULL || value == Py_None) {
Py_CLEAR(self->converters); atomic_xsetref(&self->argtypes, NULL);
Py_CLEAR(self->argtypes); atomic_xsetref(&self->converters, NULL);
} else { } else {
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self))); ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
converters = converters_from_argtypes(st, value); converters = converters_from_argtypes(st, value);
if (!converters) if (!converters)
return -1; return -1;
Py_XSETREF(self->converters, converters); atomic_xsetref(&self->converters, converters);
Py_INCREF(value); Py_INCREF(value);
Py_XSETREF(self->argtypes, value); atomic_xsetref(&self->argtypes, value);
} }
return 0; return 0;
} }
@ -4533,16 +4568,11 @@ _build_result(PyObject *result, PyObject *callargs,
} }
static PyObject * static PyObject *
PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds)
{ {
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); PyObject *result = NULL;
PyObject *restype; PyObject *callargs = NULL;
PyObject *converters; PyObject *ret = NULL;
PyObject *checker;
PyObject *argtypes;
PyObject *result;
PyObject *callargs;
PyObject *errcheck;
#ifdef MS_WIN32 #ifdef MS_WIN32
IUnknown *piunk = NULL; IUnknown *piunk = NULL;
#endif #endif
@ -4560,13 +4590,24 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
} }
assert(info); /* Cannot be NULL for PyCFuncPtrObject instances */ assert(info); /* Cannot be NULL for PyCFuncPtrObject instances */
restype = self->restype ? self->restype : info->restype; PyObject *restype = atomic_xgetref(op, &self->restype);
converters = self->converters ? self->converters : info->converters; if (restype == NULL) {
checker = self->checker ? self->checker : info->checker; restype = Py_XNewRef(info->restype);
argtypes = self->argtypes ? self->argtypes : info->argtypes; }
/* later, we probably want to have an errcheck field in stginfo */ PyObject *converters = atomic_xgetref(op, &self->converters);
errcheck = self->errcheck /* ? self->errcheck : info->errcheck */; if (converters == NULL) {
converters = Py_XNewRef(info->converters);
}
PyObject *checker = atomic_xgetref(op, &self->checker);
if (checker == NULL) {
checker = Py_XNewRef(info->checker);
}
PyObject *argtypes = atomic_xgetref(op, &self->argtypes);
if (argtypes == NULL) {
argtypes = Py_XNewRef(info->argtypes);
}
/* later, we probably want to have an errcheck field in stginfo */
PyObject *errcheck = atomic_xgetref(op, &self->errcheck);
pProc = *(void **)self->b_ptr; pProc = *(void **)self->b_ptr;
#ifdef MS_WIN32 #ifdef MS_WIN32
@ -4577,25 +4618,25 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
if (!this) { if (!this) {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"native com method call without 'this' parameter"); "native com method call without 'this' parameter");
return NULL; goto finally;
} }
if (!CDataObject_Check(st, this)) { if (!CDataObject_Check(st, this)) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
"Expected a COM this pointer as first argument"); "Expected a COM this pointer as first argument");
return NULL; goto finally;
} }
/* there should be more checks? No, in Python */ /* there should be more checks? No, in Python */
/* First arg is a pointer to an interface instance */ /* First arg is a pointer to an interface instance */
if (!this->b_ptr || *(void **)this->b_ptr == NULL) { if (!this->b_ptr || *(void **)this->b_ptr == NULL) {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"NULL COM pointer access"); "NULL COM pointer access");
return NULL; goto finally;
} }
piunk = *(IUnknown **)this->b_ptr; piunk = *(IUnknown **)this->b_ptr;
if (NULL == piunk->lpVtbl) { if (NULL == piunk->lpVtbl) {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"COM method call without VTable"); "COM method call without VTable");
return NULL; goto finally;
} }
pProc = ((void **)piunk->lpVtbl)[self->index - 0x1000]; pProc = ((void **)piunk->lpVtbl)[self->index - 0x1000];
} }
@ -4603,8 +4644,9 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
callargs = _build_callargs(st, self, argtypes, callargs = _build_callargs(st, self, argtypes,
inargs, kwds, inargs, kwds,
&outmask, &inoutmask, &numretvals); &outmask, &inoutmask, &numretvals);
if (callargs == NULL) if (callargs == NULL) {
return NULL; goto finally;
}
if (converters) { if (converters) {
int required = Py_SAFE_DOWNCAST(PyTuple_GET_SIZE(converters), int required = Py_SAFE_DOWNCAST(PyTuple_GET_SIZE(converters),
@ -4623,7 +4665,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
required, required,
required == 1 ? "" : "s", required == 1 ? "" : "s",
actual); actual);
return NULL; goto finally;
} }
} else if (required != actual) { } else if (required != actual) {
Py_DECREF(callargs); Py_DECREF(callargs);
@ -4632,7 +4674,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
required, required,
required == 1 ? "" : "s", required == 1 ? "" : "s",
actual); actual);
return NULL; goto finally;
} }
} }
@ -4663,23 +4705,19 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
if (v == NULL || v != callargs) { if (v == NULL || v != callargs) {
Py_DECREF(result); Py_DECREF(result);
Py_DECREF(callargs); Py_DECREF(callargs);
return v; ret = v;
goto finally;
} }
Py_DECREF(v); Py_DECREF(v);
} }
ret = _build_result(result, callargs, outmask, inoutmask, numretvals);
return _build_result(result, callargs, finally:
outmask, inoutmask, numretvals); Py_XDECREF(restype);
} Py_XDECREF(converters);
Py_XDECREF(checker);
static PyObject * Py_XDECREF(argtypes);
PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds) Py_XDECREF(errcheck);
{ return ret;
PyObject *result;
Py_BEGIN_CRITICAL_SECTION(op);
result = PyCFuncPtr_call_lock_held(op, inargs, kwds);
Py_END_CRITICAL_SECTION();
return result;
} }
static int static int