diff --git a/Lib/test/test_capi/test_tuple.py b/Lib/test/test_capi/test_tuple.py index b6d6da008d0b7b..e2d7f6e4f8b382 100644 --- a/Lib/test/test_capi/test_tuple.py +++ b/Lib/test/test_capi/test_tuple.py @@ -14,6 +14,12 @@ class TupleSubclass(tuple): class CAPITest(unittest.TestCase): + def _not_tracked(self, t): + self.assertFalse(gc.is_tracked(t), t) + + def _tracked(self, t): + self.assertTrue(gc.is_tracked(t), t) + def test_check(self): # Test PyTuple_Check() check = _testlimitedcapi.tuple_check @@ -52,11 +58,14 @@ def test_tuple_new(self): self.assertEqual(tup1, ()) self.assertEqual(size(tup1), 0) self.assertIs(type(tup1), tuple) + self._not_tracked(tup1) + tup2 = tuple_new(1) self.assertIs(type(tup2), tuple) self.assertEqual(size(tup2), 1) self.assertIsNot(tup2, tup1) self.assertTrue(checknull(tup2, 0)) + self._tracked(tup2) self.assertRaises(SystemError, tuple_new, -1) self.assertRaises(SystemError, tuple_new, PY_SSIZE_T_MIN) @@ -69,6 +78,12 @@ def test_tuple_fromarray(self): tup = tuple([i] for i in range(5)) copy = tuple_fromarray(tup) self.assertEqual(copy, tup) + self._tracked(copy) + + tup = tuple((42**i, 0) for i in range(5)) + copy = tuple_fromarray(tup) + self.assertEqual(copy, tup) + self._not_tracked(copy) tup = () copy = tuple_fromarray(tup) @@ -92,6 +107,10 @@ def test_tuple_pack(self): self.assertEqual(pack(1, [1]), ([1],)) self.assertEqual(pack(2, [1], [2]), ([1], [2])) + self._tracked(pack(1, [1])) + self._tracked(pack(2, [1], b'abc')) + self._not_tracked(pack(2, 42, b'abc')) + self.assertRaises(SystemError, pack, PY_SSIZE_T_MIN) self.assertRaises(SystemError, pack, -1) self.assertRaises(MemoryError, pack, PY_SSIZE_T_MAX) diff --git a/Lib/test/test_tuple.py b/Lib/test/test_tuple.py index 9ce80c5e8ea009..97fe894ef75a3c 100644 --- a/Lib/test/test_tuple.py +++ b/Lib/test/test_tuple.py @@ -291,9 +291,8 @@ def test_repr(self): self.assertEqual(repr(a2), "(0, 1, 2)") def _not_tracked(self, t): - # Nested tuples can take several collections to untrack - gc.collect() - gc.collect() + # There is no need for gc.collect() + # since we don't track these tuples at all. self.assertFalse(gc.is_tracked(t), t) def _tracked(self, t): @@ -323,6 +322,19 @@ def test_track_literals(self): self._tracked((set(),)) self._tracked((x, y, z)) + @support.cpython_only + def test_track_after_operations(self): + x, y, z = 1.5, "a", [] + + self._not_tracked((x, y) + (x, y)) + self._tracked((x, y) + (z,)) + + self._not_tracked((x, y) * 5) + self._tracked((y, z) * 5) + + self._not_tracked((x, y, z)[:2]) + self._tracked((x, y, z)[2:]) + def check_track_dynamic(self, tp, always_track): x, y, z = 1.5, "a", [] @@ -343,6 +355,14 @@ def check_track_dynamic(self, tp, always_track): self._tracked(tp(tuple([obj]) for obj in [x, y, z])) self._tracked(tuple(tp([obj]) for obj in [x, y, z])) + t = tp([1, x, y, z]) + self.assertEqual(type(t), tp) + self._tracked(t) + self.assertEqual(type(t[:]), tuple) + self._tracked(t[:]) + self.assertEqual(type(t[:3]), tuple) + self._not_tracked(t[:3]) + @support.cpython_only def test_track_dynamic(self): # Test GC-optimization of dynamically constructed tuples. diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-15-02-11.gh-issue-139951.tqbIsW.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-15-02-11.gh-issue-139951.tqbIsW.rst new file mode 100644 index 00000000000000..39b02a1635ac4c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-15-02-11.gh-issue-139951.tqbIsW.rst @@ -0,0 +1,2 @@ +Do not track immutable tuples in GC. +Patch by Sergey Miryanov and Mikhail Efimov diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index cd90b06d499faf..eff6f7fccc2609 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -145,17 +145,33 @@ _PyTuple_MaybeUntrack(PyObject *op) t = (PyTupleObject *) op; n = Py_SIZE(t); for (i = 0; i < n; i++) { - PyObject *elt = PyTuple_GET_ITEM(t, i); + PyObject *item = t->ob_item[i]; /* Tuple with NULL elements aren't fully constructed, don't untrack them yet. */ - if (!elt || - _PyObject_GC_MAY_BE_TRACKED(elt)) + if (!item || + _PyObject_GC_MAY_BE_TRACKED(item)) return; } _PyObject_GC_UNTRACK(op); } +/* gh-139951: This function helps to avoid performance issue in the GC, + * we don't track immutable tuples. */ +static bool +tuple_need_tracking(PyTupleObject *self) +{ + Py_ssize_t n = PyTuple_GET_SIZE(self); + for (Py_ssize_t i = 0; i < n; i++) { + PyObject *item = self->ob_item[i]; + assert(item); + if (_PyObject_GC_MAY_BE_TRACKED(item)) { + return true; + } + } + return false; +} + PyObject * PyTuple_Pack(Py_ssize_t n, ...) { @@ -169,19 +185,21 @@ PyTuple_Pack(Py_ssize_t n, ...) } va_start(vargs, n); - PyTupleObject *result = tuple_alloc(n); - if (result == NULL) { + PyTupleObject *tuple = tuple_alloc(n); + if (tuple == NULL) { va_end(vargs); return NULL; } - items = result->ob_item; + items = tuple->ob_item; for (i = 0; i < n; i++) { o = va_arg(vargs, PyObject *); items[i] = Py_NewRef(o); } va_end(vargs); - _PyObject_GC_TRACK(result); - return (PyObject *)result; + if (tuple_need_tracking(tuple)) { + _PyObject_GC_TRACK(tuple); + } + return (PyObject *)tuple; } @@ -381,7 +399,9 @@ PyTuple_FromArray(PyObject *const *src, Py_ssize_t n) PyObject *item = src[i]; dst[i] = Py_NewRef(item); } - _PyObject_GC_TRACK(tuple); + if (tuple_need_tracking(tuple)) { + _PyObject_GC_TRACK(tuple); + } return (PyObject *)tuple; } @@ -399,7 +419,9 @@ _PyTuple_FromStackRefStealOnSuccess(const _PyStackRef *src, Py_ssize_t n) for (Py_ssize_t i = 0; i < n; i++) { dst[i] = PyStackRef_AsPyObjectSteal(src[i]); } - _PyObject_GC_TRACK(tuple); + if (tuple_need_tracking(tuple)) { + _PyObject_GC_TRACK(tuple); + } return (PyObject *)tuple; } @@ -421,7 +443,9 @@ _PyTuple_FromArraySteal(PyObject *const *src, Py_ssize_t n) PyObject *item = src[i]; dst[i] = item; } - _PyObject_GC_TRACK(tuple); + if (tuple_need_tracking(tuple)) { + _PyObject_GC_TRACK(tuple); + } return (PyObject *)tuple; } @@ -494,7 +518,9 @@ tuple_concat(PyObject *aa, PyObject *bb) dest[i] = Py_NewRef(v); } - _PyObject_GC_TRACK(np); + if (_PyObject_GC_IS_TRACKED(a) || _PyObject_GC_IS_TRACKED(b)) { + _PyObject_GC_TRACK(np); + } return (PyObject *)np; } @@ -543,8 +569,10 @@ tuple_repeat(PyObject *self, Py_ssize_t n) _Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size, sizeof(PyObject *)*input_size); } - _PyObject_GC_TRACK(np); - return (PyObject *) np; + if (_PyObject_GC_IS_TRACKED(a)) { + _PyObject_GC_TRACK(np); + } + return (PyObject *)np; } /*[clinic input] @@ -821,7 +849,9 @@ tuple_subscript(PyObject *op, PyObject* item) dest[i] = it; } - _PyObject_GC_TRACK(result); + if (tuple_need_tracking(result)) { + _PyObject_GC_TRACK(result); + } return (PyObject *)result; } }