Skip to content

Commit 8c69901

Browse files
[3.14] gh-134637: Fix performance regression in calling ctypes function pointer in free threading. (GH-134702) (#134742)
gh-134637: Fix performance regression in calling `ctypes` function pointer in `free threading`. (GH-134702) Fix performance regression in calling `ctypes` function pointer in `free threading`. (cherry picked from commit 3c05251) Co-authored-by: Kumar Aditya <[email protected]>
1 parent b187e94 commit 8c69901

File tree

2 files changed

+91
-52
lines changed

2 files changed

+91
-52
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix performance regression in calling a :mod:`ctypes` function pointer in :term:`free threading`.

Modules/_ctypes/_ctypes.c

Lines changed: 90 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -3591,6 +3591,45 @@ generic_pycdata_new(ctypes_state *st,
35913591
PyCFuncPtr_Type
35923592
*/
35933593

3594+
static inline void
3595+
atomic_xsetref(PyObject **field, PyObject *value)
3596+
{
3597+
#ifdef Py_GIL_DISABLED
3598+
PyObject *old = *field;
3599+
_Py_atomic_store_ptr(field, value);
3600+
Py_XDECREF(old);
3601+
#else
3602+
Py_XSETREF(*field, value);
3603+
#endif
3604+
}
3605+
/*
3606+
This function atomically loads the reference from *field, and
3607+
tries to get a new reference to it. If the incref fails,
3608+
it acquires critical section of obj and returns a new reference to the *field.
3609+
In the general case, this avoids contention on acquiring the critical section.
3610+
*/
3611+
static inline PyObject *
3612+
atomic_xgetref(PyObject *obj, PyObject **field)
3613+
{
3614+
#ifdef Py_GIL_DISABLED
3615+
PyObject *value = _Py_atomic_load_ptr(field);
3616+
if (value == NULL) {
3617+
return NULL;
3618+
}
3619+
if (_Py_TryIncrefCompare(field, value)) {
3620+
return value;
3621+
}
3622+
Py_BEGIN_CRITICAL_SECTION(obj);
3623+
value = Py_XNewRef(*field);
3624+
Py_END_CRITICAL_SECTION();
3625+
return value;
3626+
#else
3627+
return Py_XNewRef(*field);
3628+
#endif
3629+
}
3630+
3631+
3632+
35943633
/*[clinic input]
35953634
@critical_section
35963635
@setter
@@ -3607,7 +3646,7 @@ _ctypes_CFuncPtr_errcheck_set_impl(PyCFuncPtrObject *self, PyObject *value)
36073646
return -1;
36083647
}
36093648
Py_XINCREF(value);
3610-
Py_XSETREF(self->errcheck, value);
3649+
atomic_xsetref(&self->errcheck, value);
36113650
return 0;
36123651
}
36133652

@@ -3639,12 +3678,10 @@ static int
36393678
_ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value)
36403679
/*[clinic end generated code: output=0be0a086abbabf18 input=683c3bef4562ccc6]*/
36413680
{
3642-
PyObject *checker, *oldchecker;
3681+
PyObject *checker;
36433682
if (value == NULL) {
3644-
oldchecker = self->checker;
3645-
self->checker = NULL;
3646-
Py_CLEAR(self->restype);
3647-
Py_XDECREF(oldchecker);
3683+
atomic_xsetref(&self->restype, NULL);
3684+
atomic_xsetref(&self->checker, NULL);
36483685
return 0;
36493686
}
36503687
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
@@ -3660,11 +3697,9 @@ _ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value)
36603697
if (PyObject_GetOptionalAttr(value, &_Py_ID(_check_retval_), &checker) < 0) {
36613698
return -1;
36623699
}
3663-
oldchecker = self->checker;
3664-
self->checker = checker;
36653700
Py_INCREF(value);
3666-
Py_XSETREF(self->restype, value);
3667-
Py_XDECREF(oldchecker);
3701+
atomic_xsetref(&self->checker, checker);
3702+
atomic_xsetref(&self->restype, value);
36683703
return 0;
36693704
}
36703705

@@ -3709,16 +3744,16 @@ _ctypes_CFuncPtr_argtypes_set_impl(PyCFuncPtrObject *self, PyObject *value)
37093744
PyObject *converters;
37103745

37113746
if (value == NULL || value == Py_None) {
3712-
Py_CLEAR(self->converters);
3713-
Py_CLEAR(self->argtypes);
3747+
atomic_xsetref(&self->argtypes, NULL);
3748+
atomic_xsetref(&self->converters, NULL);
37143749
} else {
37153750
ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self)));
37163751
converters = converters_from_argtypes(st, value);
37173752
if (!converters)
37183753
return -1;
3719-
Py_XSETREF(self->converters, converters);
3754+
atomic_xsetref(&self->converters, converters);
37203755
Py_INCREF(value);
3721-
Py_XSETREF(self->argtypes, value);
3756+
atomic_xsetref(&self->argtypes, value);
37223757
}
37233758
return 0;
37243759
}
@@ -4514,16 +4549,11 @@ _build_result(PyObject *result, PyObject *callargs,
45144549
}
45154550

45164551
static PyObject *
4517-
PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
4552+
PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds)
45184553
{
4519-
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op);
4520-
PyObject *restype;
4521-
PyObject *converters;
4522-
PyObject *checker;
4523-
PyObject *argtypes;
4524-
PyObject *result;
4525-
PyObject *callargs;
4526-
PyObject *errcheck;
4554+
PyObject *result = NULL;
4555+
PyObject *callargs = NULL;
4556+
PyObject *ret = NULL;
45274557
#ifdef MS_WIN32
45284558
IUnknown *piunk = NULL;
45294559
#endif
@@ -4541,13 +4571,24 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
45414571
}
45424572
assert(info); /* Cannot be NULL for PyCFuncPtrObject instances */
45434573

4544-
restype = self->restype ? self->restype : info->restype;
4545-
converters = self->converters ? self->converters : info->converters;
4546-
checker = self->checker ? self->checker : info->checker;
4547-
argtypes = self->argtypes ? self->argtypes : info->argtypes;
4548-
/* later, we probably want to have an errcheck field in stginfo */
4549-
errcheck = self->errcheck /* ? self->errcheck : info->errcheck */;
4550-
4574+
PyObject *restype = atomic_xgetref(op, &self->restype);
4575+
if (restype == NULL) {
4576+
restype = Py_XNewRef(info->restype);
4577+
}
4578+
PyObject *converters = atomic_xgetref(op, &self->converters);
4579+
if (converters == NULL) {
4580+
converters = Py_XNewRef(info->converters);
4581+
}
4582+
PyObject *checker = atomic_xgetref(op, &self->checker);
4583+
if (checker == NULL) {
4584+
checker = Py_XNewRef(info->checker);
4585+
}
4586+
PyObject *argtypes = atomic_xgetref(op, &self->argtypes);
4587+
if (argtypes == NULL) {
4588+
argtypes = Py_XNewRef(info->argtypes);
4589+
}
4590+
/* later, we probably want to have an errcheck field in stginfo */
4591+
PyObject *errcheck = atomic_xgetref(op, &self->errcheck);
45514592

45524593
pProc = *(void **)self->b_ptr;
45534594
#ifdef MS_WIN32
@@ -4558,34 +4599,35 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
45584599
if (!this) {
45594600
PyErr_SetString(PyExc_ValueError,
45604601
"native com method call without 'this' parameter");
4561-
return NULL;
4602+
goto finally;
45624603
}
45634604
if (!CDataObject_Check(st, this)) {
45644605
PyErr_SetString(PyExc_TypeError,
45654606
"Expected a COM this pointer as first argument");
4566-
return NULL;
4607+
goto finally;
45674608
}
45684609
/* there should be more checks? No, in Python */
45694610
/* First arg is a pointer to an interface instance */
45704611
if (!this->b_ptr || *(void **)this->b_ptr == NULL) {
45714612
PyErr_SetString(PyExc_ValueError,
45724613
"NULL COM pointer access");
4573-
return NULL;
4614+
goto finally;
45744615
}
45754616
piunk = *(IUnknown **)this->b_ptr;
45764617
if (NULL == piunk->lpVtbl) {
45774618
PyErr_SetString(PyExc_ValueError,
45784619
"COM method call without VTable");
4579-
return NULL;
4620+
goto finally;
45804621
}
45814622
pProc = ((void **)piunk->lpVtbl)[self->index - 0x1000];
45824623
}
45834624
#endif
45844625
callargs = _build_callargs(st, self, argtypes,
45854626
inargs, kwds,
45864627
&outmask, &inoutmask, &numretvals);
4587-
if (callargs == NULL)
4588-
return NULL;
4628+
if (callargs == NULL) {
4629+
goto finally;
4630+
}
45894631

45904632
if (converters) {
45914633
int required = Py_SAFE_DOWNCAST(PyTuple_GET_SIZE(converters),
@@ -4604,7 +4646,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
46044646
required,
46054647
required == 1 ? "" : "s",
46064648
actual);
4607-
return NULL;
4649+
goto finally;
46084650
}
46094651
} else if (required != actual) {
46104652
Py_DECREF(callargs);
@@ -4613,7 +4655,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
46134655
required,
46144656
required == 1 ? "" : "s",
46154657
actual);
4616-
return NULL;
4658+
goto finally;
46174659
}
46184660
}
46194661

@@ -4644,23 +4686,19 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds)
46444686
if (v == NULL || v != callargs) {
46454687
Py_DECREF(result);
46464688
Py_DECREF(callargs);
4647-
return v;
4689+
ret = v;
4690+
goto finally;
46484691
}
46494692
Py_DECREF(v);
46504693
}
4651-
4652-
return _build_result(result, callargs,
4653-
outmask, inoutmask, numretvals);
4654-
}
4655-
4656-
static PyObject *
4657-
PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds)
4658-
{
4659-
PyObject *result;
4660-
Py_BEGIN_CRITICAL_SECTION(op);
4661-
result = PyCFuncPtr_call_lock_held(op, inargs, kwds);
4662-
Py_END_CRITICAL_SECTION();
4663-
return result;
4694+
ret = _build_result(result, callargs, outmask, inoutmask, numretvals);
4695+
finally:
4696+
Py_XDECREF(restype);
4697+
Py_XDECREF(converters);
4698+
Py_XDECREF(checker);
4699+
Py_XDECREF(argtypes);
4700+
Py_XDECREF(errcheck);
4701+
return ret;
46644702
}
46654703

46664704
static int

0 commit comments

Comments
 (0)