From 2b106e5c9f5be31b51c3f6677344612ba532ab49 Mon Sep 17 00:00:00 2001 From: Christian Aurich Zanettini Martins Date: Thu, 8 Oct 2026 15:38:32 -0300 Subject: [PATCH 1/4] gh-158803: Borrow bytes items in bytes.join() while the list is locked bytes.join() and bytearray.join() took a new reference to every exact bytes item. In the free-threaded build that is an atomic operation on objects shared between threads, and since GH-158910 it runs while the list's lock is held. The critical section keeps the items alive, so borrow them, as _PyUnicode_JoinArray() does. References are taken only before something can suspend the critical section: PyObject_GetBuffer() on an item that is not bytes, or releasing the thread state to copy a large result. --- Lib/test/test_bytes.py | 11 ++++--- .../test_free_threading/test_bytes_object.py | 21 +++++++++++++ ...-10-08-17-36-53.gh-issue-158803.4YpkIS.rst | 4 +++ Objects/stringlib/join.h | 31 ++++++++++++++++--- 4 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 96190a7f5821709..bb759415d2926a2 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -671,12 +671,13 @@ def test_join_concurrent_buffer_mutation(self): # mutate the joined sequence (simulated here by mutating in __buffer__). # See: https://github.com/python/cpython/issues/151295 def make_seq(mutate): - # Item is only referenced from the list slot, so mutate() frees it. + # The first two items are only referenced from their list slots, + # so mutate() frees them. class Item: def __buffer__(self, flags): mutate(seq) return memoryview(b'x') - seq = [b'a', Item(), b'c'] + seq = [bytes(2), Item(), b'c'] return seq for sep in (self.type2test(b''), self.type2test(b'::')): @@ -686,11 +687,11 @@ def __buffer__(self, flags): self.assertRaises(RuntimeError, sep.join, seq) # The list length is unchanged, so the size-change recheck - # cannot fire: only keeping the item alive avoids the crash. + # cannot fire: only keeping the items alive avoids the crash. def replace(seq): - seq[1] = b'z' + seq[0] = seq[1] = b'z' seq = make_seq(replace) - self.assertEqual(sep.join(seq), sep.join([b'a', b'x', b'c'])) + self.assertEqual(sep.join(seq), sep.join([bytes(2), b'x', b'c'])) def test_count(self): b = self.type2test(b'mississippi') diff --git a/Lib/test/test_free_threading/test_bytes_object.py b/Lib/test/test_free_threading/test_bytes_object.py index a9017bac6de0295..666805a5d93626f 100644 --- a/Lib/test/test_free_threading/test_bytes_object.py +++ b/Lib/test/test_free_threading/test_bytes_object.py @@ -54,6 +54,27 @@ def reader(): threading_helper.run_concurrently([writer] + [reader] * 4) + def test_racing_join_replace_large(self): + # join() suspends the critical section while it copies a result + # of 1 MiB or more, so the list can drop its items meanwhile. + size = 1 << 18 + lst = [bytes(size) for _ in range(8)] + done = Event() + + def writer(): + try: + for _ in range(100): + for i in range(len(lst)): + lst[i] = bytes(size) + finally: + done.set() + + def reader(): + while not done.is_set(): + self.assertEqual(b''.join(lst), bytes(size * len(lst))) + + threading_helper.run_concurrently([writer] + [reader] * 4) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst new file mode 100644 index 000000000000000..36502336d6b0843 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst @@ -0,0 +1,4 @@ +Speed up :meth:`bytes.join` and :meth:`bytearray.join` by not taking a new +reference to each :class:`bytes` item, which recovers the overhead of the +lock added in the :term:`free-threaded build`. Patch by Christian Aurich +Zanettini Martins. diff --git a/Objects/stringlib/join.h b/Objects/stringlib/join.h index 4764baac181b2ad..f9f08f39a0175f3 100644 --- a/Objects/stringlib/join.h +++ b/Objects/stringlib/join.h @@ -14,6 +14,8 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) Py_ssize_t seqlen = 0; Py_ssize_t sz = 0; Py_ssize_t i, nbufs; + /* buffers[:nborrowed] hold borrowed references */ + Py_ssize_t nborrowed = 0; PyObject *item; Py_buffer *buffers = NULL; #define NB_STATIC_BUFFERS 10 @@ -53,14 +55,28 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) Py_ssize_t itemlen; item = PySequence_Fast_GET_ITEM(seq, i); if (PyBytes_CheckExact(item)) { - /* Fast path. */ - buffers[i].obj = Py_NewRef(item); + /* Fast path. While the critical section is held, seq keeps + the item alive, so it can be borrowed. */ + buffers[i].obj = item; buffers[i].buf = PyBytes_AS_STRING(item); buffers[i].len = PyBytes_GET_SIZE(item); + if (nborrowed == i) { + /* Nothing has suspended the critical section yet. */ + nborrowed++; + } + else { + Py_INCREF(item); + } } else { - /* item is only borrowed; its __buffer__() may run Python that - drops the sequence's last reference to it. */ + /* PyObject_GetBuffer() can run Python code (__buffer__()) or + wait for a lock, which suspends the critical section. The + sequence may then drop its items, this one included, so take + references to them first. */ + for (Py_ssize_t j = 0; j < nborrowed; j++) { + Py_INCREF(buffers[j].obj); + } + nborrowed = 0; Py_INCREF(item); if (PyObject_GetBuffer(item, &buffers[i], PyBUF_SIMPLE) != 0) { PyErr_Format(PyExc_TypeError, @@ -113,6 +129,11 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) drop_gil = 0; /* Benefits are likely outweighed by the overheads */ } if (drop_gil) { + /* This suspends the critical section too. */ + for (i = 0; i < nborrowed; i++) { + Py_INCREF(buffers[i].obj); + } + nborrowed = 0; save = PyEval_SaveThread(); } if (!seplen) { @@ -148,7 +169,7 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) error: res = NULL; done: - for (i = 0; i < nbufs; i++) + for (i = nborrowed; i < nbufs; i++) PyBuffer_Release(&buffers[i]); if (buffers != static_buffers) PyMem_Free(buffers); From ad4a1802e0ed00e35ec52e16b154fc97605b99d5 Mon Sep 17 00:00:00 2001 From: Christian Aurich Zanettini Martins Date: Sun, 11 Oct 2026 01:10:04 -0300 Subject: [PATCH 2/4] gh-158803: Copy exact bytes items directly in bytes.join() When every item is an exact bytes object and the result is below the 1 MiB threshold, copy straight from the items, without filling a Py_buffer or taking a reference for each one. The caller's critical section keeps the items alive. Items could only stay borrowed under the previous commit in that same case, so the per-item borrow bookkeeping is dropped and the general path takes references as before. In the general path, release exact bytes items with a plain decref, since bytes has no bf_releasebuffer. Co-authored-by: Pieter Eendebak --- Lib/test/test_bytes.py | 1 + ...-10-08-17-36-53.gh-issue-158803.4YpkIS.rst | 7 +- Objects/stringlib/join.h | 79 ++++++++++++------- 3 files changed, 56 insertions(+), 31 deletions(-) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index bb759415d2926a2..d41f2ec8fafa04a 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -644,6 +644,7 @@ def test_join(self): self.assertEqual(self.type2test(b"").join(lst), b"abc") self.assertEqual(self.type2test(b"").join(tuple(lst)), b"abc") self.assertEqual(self.type2test(b"").join(iter(lst)), b"abc") + self.assertEqual(self.type2test(b".").join([b"ab", b"cd"]), b"ab.cd") dot_join = self.type2test(b".:").join self.assertEqual(dot_join([b"ab", b"cd"]), b"ab.:cd") self.assertEqual(dot_join([memoryview(b"ab"), b"cd"]), b"ab.:cd") diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst index 36502336d6b0843..60f2a95ca5f3435 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst @@ -1,4 +1,3 @@ -Speed up :meth:`bytes.join` and :meth:`bytearray.join` by not taking a new -reference to each :class:`bytes` item, which recovers the overhead of the -lock added in the :term:`free-threaded build`. Patch by Christian Aurich -Zanettini Martins. +Speed up :meth:`bytes.join` and :meth:`bytearray.join` when all items are +:class:`bytes`. Patch by Pieter Eendebak and Christian Aurich Zanettini +Martins. diff --git a/Objects/stringlib/join.h b/Objects/stringlib/join.h index f9f08f39a0175f3..a2067212db94863 100644 --- a/Objects/stringlib/join.h +++ b/Objects/stringlib/join.h @@ -14,8 +14,6 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) Py_ssize_t seqlen = 0; Py_ssize_t sz = 0; Py_ssize_t i, nbufs; - /* buffers[:nborrowed] hold borrowed references */ - Py_ssize_t nborrowed = 0; PyObject *item; Py_buffer *buffers = NULL; #define NB_STATIC_BUFFERS 10 @@ -36,6 +34,44 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) } } #endif + + /* Fast path: all items are exact bytes and the result is too small to + * release the GIL for. Nothing here runs Python code or suspends the + * critical section, so seq keeps the items alive and they can be copied + * without taking references. */ + Py_ssize_t limit = GIL_THRESHOLD - seplen; + for (i = 0; i < seqlen; i++) { + item = PySequence_Fast_GET_ITEM(seq, i); + if (!PyBytes_CheckExact(item) + || PyBytes_GET_SIZE(item) >= limit - sz) { + break; + } + sz += PyBytes_GET_SIZE(item) + seplen; + } + if (i == seqlen) { + res = STRINGLIB_NEW(NULL, sz - seplen); + if (res == NULL) { + return NULL; + } + p = STRINGLIB_STR(res); + for (i = 0; i < seqlen; i++) { + if (i != 0 && seplen != 0) { + if (seplen == 1) { + *p++ = *sepstr; + } + else { + memcpy(p, sepstr, seplen); + p += seplen; + } + } + item = PySequence_Fast_GET_ITEM(seq, i); + memcpy(p, PyBytes_AS_STRING(item), PyBytes_GET_SIZE(item)); + p += PyBytes_GET_SIZE(item); + } + return res; + } + sz = 0; + if (seqlen > NB_STATIC_BUFFERS) { buffers = PyMem_NEW(Py_buffer, seqlen); if (buffers == NULL) { @@ -55,28 +91,14 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) Py_ssize_t itemlen; item = PySequence_Fast_GET_ITEM(seq, i); if (PyBytes_CheckExact(item)) { - /* Fast path. While the critical section is held, seq keeps - the item alive, so it can be borrowed. */ - buffers[i].obj = item; + /* Fast path. */ + buffers[i].obj = Py_NewRef(item); buffers[i].buf = PyBytes_AS_STRING(item); buffers[i].len = PyBytes_GET_SIZE(item); - if (nborrowed == i) { - /* Nothing has suspended the critical section yet. */ - nborrowed++; - } - else { - Py_INCREF(item); - } } else { - /* PyObject_GetBuffer() can run Python code (__buffer__()) or - wait for a lock, which suspends the critical section. The - sequence may then drop its items, this one included, so take - references to them first. */ - for (Py_ssize_t j = 0; j < nborrowed; j++) { - Py_INCREF(buffers[j].obj); - } - nborrowed = 0; + /* item is only borrowed; its __buffer__() may run Python that + drops the sequence's last reference to it. */ Py_INCREF(item); if (PyObject_GetBuffer(item, &buffers[i], PyBUF_SIMPLE) != 0) { PyErr_Format(PyExc_TypeError, @@ -129,11 +151,6 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) drop_gil = 0; /* Benefits are likely outweighed by the overheads */ } if (drop_gil) { - /* This suspends the critical section too. */ - for (i = 0; i < nborrowed; i++) { - Py_INCREF(buffers[i].obj); - } - nborrowed = 0; save = PyEval_SaveThread(); } if (!seplen) { @@ -169,8 +186,16 @@ STRINGLIB(bytes_join_lock_held)(PyObject *sep, PyObject *seq) error: res = NULL; done: - for (i = nborrowed; i < nbufs; i++) - PyBuffer_Release(&buffers[i]); + for (i = 0; i < nbufs; i++) { + PyObject *obj = buffers[i].obj; + if (obj != NULL && PyBytes_CheckExact(obj)) { + /* bytes has no bf_releasebuffer */ + Py_DECREF(obj); + } + else { + PyBuffer_Release(&buffers[i]); + } + } if (buffers != static_buffers) PyMem_Free(buffers); return res; From e73c74ce2ba3411798b1251abec9b9772f706b45 Mon Sep 17 00:00:00 2001 From: Christian Aurich Zanettini Martins Date: Sun, 11 Oct 2026 15:42:32 -0300 Subject: [PATCH 3/4] gh-159191: Move the NEWS entry to the new issue --- ....4YpkIS.rst => 2026-10-08-17-36-53.gh-issue-159191.4YpkIS.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Core_and_Builtins/{2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst => 2026-10-08-17-36-53.gh-issue-159191.4YpkIS.rst} (100%) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-159191.4YpkIS.rst similarity index 100% rename from Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-158803.4YpkIS.rst rename to Misc/NEWS.d/next/Core_and_Builtins/2026-10-08-17-36-53.gh-issue-159191.4YpkIS.rst From 6794be4952ab29969778bbcdba1c0df7a50aaf3f Mon Sep 17 00:00:00 2001 From: Christian Aurich Zanettini Martins Date: Mon, 12 Oct 2026 00:42:37 -0300 Subject: [PATCH 4/4] gh-159191: Stress test bytes.join() with non-bytes items The existing stress tests now only take the fast path. test_bytes is part of the PGO training set, so the general path was barely trained, and PGO builds stopped inlining PyBuffer_Release() there: joining a list that is half bytearrays became about 14% slower than on main. With this test it is back to main's speed. --- Lib/test/test_bytes.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index d41f2ec8fafa04a..df14c48e18d874e 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -655,6 +655,8 @@ def test_join(self): seq = [b"abc"] * 100000 expected = b"abc" + b".:abc" * 99999 self.assertEqual(dot_join(seq), expected) + seq = [bytearray(b"abc")] * 100000 + self.assertEqual(dot_join(seq), expected) # Stress test with empty separator seq = [b"abc"] * 100000 expected = b"abc" * 100000