Skip to content

Commit 896dd00

Browse files
committed
gh-131586: Avoid refcount contention in some "special" calls
In the free threaded build, the `_PyObject_LookupSpecial()` call can lead to reference count contention on the returned function object becuase it doesn't use stackrefs. Refactor some of the callers to use `_PyObject_MaybeCallSpecialNoArgs`, which uses stackrefs internally. This fixes the scaling bottleneck in the "lookup_special" microbenchmark in `ftscalingbench.py`. However, the are still some uses of `_PyObject_LookupSpecial()` that need to be addressed in future PRs.
1 parent b92ee14 commit 896dd00

17 files changed

+453
-376
lines changed

Include/internal/pycore_global_objects_fini_generated.h

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_global_strings.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct _Py_global_strings {
8989
STRUCT_FOR_ID(__bytes__)
9090
STRUCT_FOR_ID(__call__)
9191
STRUCT_FOR_ID(__cantrace__)
92+
STRUCT_FOR_ID(__ceil__)
9293
STRUCT_FOR_ID(__class__)
9394
STRUCT_FOR_ID(__class_getitem__)
9495
STRUCT_FOR_ID(__classcell__)
@@ -112,6 +113,7 @@ struct _Py_global_strings {
112113
STRUCT_FOR_ID(__file__)
113114
STRUCT_FOR_ID(__firstlineno__)
114115
STRUCT_FOR_ID(__float__)
116+
STRUCT_FOR_ID(__floor__)
115117
STRUCT_FOR_ID(__floordiv__)
116118
STRUCT_FOR_ID(__format__)
117119
STRUCT_FOR_ID(__fspath__)
@@ -217,6 +219,7 @@ struct _Py_global_strings {
217219
STRUCT_FOR_ID(__subclasscheck__)
218220
STRUCT_FOR_ID(__subclasshook__)
219221
STRUCT_FOR_ID(__truediv__)
222+
STRUCT_FOR_ID(__trunc__)
220223
STRUCT_FOR_ID(__type_params__)
221224
STRUCT_FOR_ID(__typing_is_unpacked_typevartuple__)
222225
STRUCT_FOR_ID(__typing_prepare_subst__)

Include/internal/pycore_object.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,8 @@ extern bool _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name,
890890
PyObject **attr);
891891
extern PyObject *_PyType_LookupRefAndVersion(PyTypeObject *, PyObject *,
892892
unsigned int *);
893+
extern unsigned int
894+
_PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef *out);
893895

894896
// Cache the provided init method in the specialization cache of type if the
895897
// provided type version matches the current version of the type.
@@ -946,6 +948,14 @@ extern int _PyObject_IsInstanceDictEmpty(PyObject *);
946948
PyAPI_FUNC(PyObject*) _PyObject_LookupSpecial(PyObject *, PyObject *);
947949
PyAPI_FUNC(PyObject*) _PyObject_LookupSpecialMethod(PyObject *self, PyObject *attr, PyObject **self_or_null);
948950

951+
// Calls the method named `attr` on `self`, but does not set an exception if
952+
// the attribute does not exist.
953+
PyAPI_FUNC(PyObject *)
954+
_PyObject_MaybeCallSpecialNoArgs(PyObject *self, PyObject *attr);
955+
956+
PyAPI_FUNC(PyObject *)
957+
_PyObject_MaybeCallSpecialOneArg(PyObject *self, PyObject *attr, PyObject *arg);
958+
949959
extern int _PyObject_IsAbstract(PyObject *);
950960

951961
PyAPI_FUNC(int) _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method);

Include/internal/pycore_runtime_init_generated.h

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_stackref.h

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ PyStackRef_XCLOSE(_PyStackRef ref)
592592

593593
// Note: this is a macro because MSVC (Windows) has trouble inlining it.
594594

595-
#define PyStackRef_Is(a, b) (((a).bits & (~Py_TAG_REFCNT)) == ((b).bits & (~Py_TAG_REFCNT)))
595+
#define PyStackRef_Is(a, b) (((a).bits & (~Py_TAG_BITS)) == ((b).bits & (~Py_TAG_BITS)))
596596

597597
#endif // !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG)
598598

@@ -640,6 +640,29 @@ PyStackRef_FunctionCheck(_PyStackRef stackref)
640640
return PyFunction_Check(PyStackRef_AsPyObjectBorrow(stackref));
641641
}
642642

643+
static inline void
644+
_PyThreadState_PushCStackRef(PyThreadState *tstate, _PyCStackRef *ref)
645+
{
646+
#ifdef Py_GIL_DISABLED
647+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
648+
ref->next = tstate_impl->c_stack_refs;
649+
tstate_impl->c_stack_refs = ref;
650+
#endif
651+
ref->ref = PyStackRef_NULL;
652+
}
653+
654+
static inline void
655+
_PyThreadState_PopCStackRef(PyThreadState *tstate, _PyCStackRef *ref)
656+
{
657+
#ifdef Py_GIL_DISABLED
658+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
659+
assert(tstate_impl->c_stack_refs == ref);
660+
_PyCStackRef *ref = tstate_impl->c_stack_refs;
661+
tstate_impl->c_stack_refs = ref->next;
662+
#endif
663+
PyStackRef_XCLOSE(ref->ref);
664+
}
665+
643666
#ifdef Py_GIL_DISABLED
644667

645668
static inline int
@@ -656,6 +679,17 @@ _Py_TryIncrefCompareStackRef(PyObject **src, PyObject *op, _PyStackRef *out)
656679
return 0;
657680
}
658681

682+
static inline int
683+
_Py_TryXGetStackRef(PyObject **src, _PyStackRef *out)
684+
{
685+
PyObject *op = _Py_atomic_load_ptr_relaxed(src);
686+
if (op == NULL) {
687+
*out = PyStackRef_NULL;
688+
return 1;
689+
}
690+
return _Py_TryIncrefCompareStackRef(src, op, out);
691+
}
692+
659693
#endif
660694

661695
#ifdef __cplusplus

Include/internal/pycore_structs.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ typedef union _PyStackRef {
6565
#endif
6666
} _PyStackRef;
6767

68+
// A stackref that can be stored in a regular C local variable and be visible
69+
// to the GC in the free threading build.
70+
// Used in combination with _PyThreadState_PushCStackRef().
71+
typedef struct _PyCStackRef {
72+
_PyStackRef ref;
73+
#ifdef Py_GIL_DISABLED
74+
struct _PyCStackRef *next;
75+
#endif
76+
} _PyCStackRef;
77+
6878

6979
#ifdef __cplusplus
7080
}

Include/internal/pycore_tstate.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ typedef struct _PyThreadStateImpl {
4747
struct _qsbr_thread_state *qsbr; // only used by free-threaded build
4848
struct llist_node mem_free_queue; // delayed free queue
4949

50-
5150
#ifdef Py_GIL_DISABLED
51+
// Stack references for the current thread that exist on the C stack
52+
struct _PyCStackRef *c_stack_refs;
5253
struct _gc_thread_state gc;
5354
struct _mimalloc_thread_state mimalloc;
5455
struct _Py_freelists freelists;

Include/internal/pycore_unicodeobject_generated.h

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test_bool.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,10 @@ def __len__(self):
383383
__bool__ = None
384384
self.assertRaises(TypeError, bool, B())
385385

386+
class C:
387+
__len__ = None
388+
self.assertRaises(TypeError, bool, C())
389+
386390
def test_real_and_imag(self):
387391
self.assertEqual(True.real, 1)
388392
self.assertEqual(True.imag, 0)

Lib/test/test_builtin.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,6 +1746,11 @@ def test_repr(self):
17461746
a[0] = a
17471747
self.assertEqual(repr(a), '{0: {...}}')
17481748

1749+
def test_repr_blocked(self):
1750+
class C:
1751+
__repr__ = None
1752+
self.assertRaises(TypeError, repr, C())
1753+
17491754
def test_round(self):
17501755
self.assertEqual(round(0.0), 0.0)
17511756
self.assertEqual(type(round(0.0)), int)

0 commit comments

Comments
 (0)