Skip to content

Commit 67fbfb4

Browse files
authored
gh-131586: Avoid refcount contention in some "special" calls (#131588)
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 3d4ac1a commit 67fbfb4

16 files changed

+450
-374
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__)
@@ -113,6 +114,7 @@ struct _Py_global_strings {
113114
STRUCT_FOR_ID(__file__)
114115
STRUCT_FOR_ID(__firstlineno__)
115116
STRUCT_FOR_ID(__float__)
117+
STRUCT_FOR_ID(__floor__)
116118
STRUCT_FOR_ID(__floordiv__)
117119
STRUCT_FOR_ID(__format__)
118120
STRUCT_FOR_ID(__fspath__)
@@ -218,6 +220,7 @@ struct _Py_global_strings {
218220
STRUCT_FOR_ID(__subclasscheck__)
219221
STRUCT_FOR_ID(__subclasshook__)
220222
STRUCT_FOR_ID(__truediv__)
223+
STRUCT_FOR_ID(__trunc__)
221224
STRUCT_FOR_ID(__type_params__)
222225
STRUCT_FOR_ID(__typing_is_unpacked_typevartuple__)
223226
STRUCT_FOR_ID(__typing_prepare_subst__)

Include/internal/pycore_object.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,12 @@ extern bool _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name,
891891
extern PyObject *_PyType_LookupRefAndVersion(PyTypeObject *, PyObject *,
892892
unsigned int *);
893893

894+
// Internal API to look for a name through the MRO.
895+
// This stores a stack reference in out and returns the value of
896+
// type->tp_version or zero if name is missing. It doesn't set an exception!
897+
extern unsigned int
898+
_PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef *out);
899+
894900
// Cache the provided init method in the specialization cache of type if the
895901
// provided type version matches the current version of the type.
896902
//
@@ -946,6 +952,14 @@ extern int _PyObject_IsInstanceDictEmpty(PyObject *);
946952
PyAPI_FUNC(PyObject*) _PyObject_LookupSpecial(PyObject *, PyObject *);
947953
PyAPI_FUNC(PyObject*) _PyObject_LookupSpecialMethod(PyObject *self, PyObject *attr, PyObject **self_or_null);
948954

955+
// Calls the method named `attr` on `self`, but does not set an exception if
956+
// the attribute does not exist.
957+
PyAPI_FUNC(PyObject *)
958+
_PyObject_MaybeCallSpecialNoArgs(PyObject *self, PyObject *attr);
959+
960+
PyAPI_FUNC(PyObject *)
961+
_PyObject_MaybeCallSpecialOneArg(PyObject *self, PyObject *attr, PyObject *arg);
962+
949963
extern int _PyObject_IsAbstract(PyObject *);
950964

951965
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: 34 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,28 @@ 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+
tstate_impl->c_stack_refs = ref->next;
661+
#endif
662+
PyStackRef_XCLOSE(ref->ref);
663+
}
664+
643665
#ifdef Py_GIL_DISABLED
644666

645667
static inline int
@@ -656,6 +678,17 @@ _Py_TryIncrefCompareStackRef(PyObject **src, PyObject *op, _PyStackRef *out)
656678
return 0;
657679
}
658680

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

661694
// Like Py_VISIT but for _PyStackRef fields

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)

Lib/test/test_math.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,8 @@ def testFloor(self):
573573
#self.assertEqual(math.ceil(NINF), NINF)
574574
#self.assertTrue(math.isnan(math.floor(NAN)))
575575

576+
class TestFloorIsNone(float):
577+
__floor__ = None
576578
class TestFloor:
577579
def __floor__(self):
578580
return 42
@@ -588,6 +590,7 @@ class TestBadFloor:
588590
self.assertEqual(math.floor(FloatLike(41.9)), 41)
589591
self.assertRaises(TypeError, math.floor, TestNoFloor())
590592
self.assertRaises(ValueError, math.floor, TestBadFloor())
593+
self.assertRaises(TypeError, math.floor, TestFloorIsNone(3.5))
591594

592595
t = TestNoFloor()
593596
t.__floor__ = lambda *args: args

0 commit comments

Comments
 (0)