bpo-36051: Drop GIL during large bytes.join() (GH-17757)

Improve multi-threaded performance by dropping the GIL in the fast path
of bytes.join. To avoid increasing overhead for small joins, it is only
done if the output size exceeds a threshold.
This commit is contained in:
Bruce Merry 2020-01-29 09:09:24 +02:00 committed by GitHub
parent 6a65eba44b
commit d07d9f4c43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 48 additions and 19 deletions

View File

@ -547,9 +547,13 @@ class BaseBytesTest:
self.assertEqual(dot_join([bytearray(b"ab"), b"cd"]), b"ab.:cd") self.assertEqual(dot_join([bytearray(b"ab"), b"cd"]), b"ab.:cd")
self.assertEqual(dot_join([b"ab", bytearray(b"cd")]), b"ab.:cd") self.assertEqual(dot_join([b"ab", bytearray(b"cd")]), b"ab.:cd")
# Stress it with many items # Stress it with many items
seq = [b"abc"] * 1000 seq = [b"abc"] * 100000
expected = b"abc" + b".:abc" * 999 expected = b"abc" + b".:abc" * 99999
self.assertEqual(dot_join(seq), expected) self.assertEqual(dot_join(seq), expected)
# Stress test with empty separator
seq = [b"abc"] * 100000
expected = b"abc" * 100000
self.assertEqual(self.type2test(b"").join(seq), expected)
self.assertRaises(TypeError, self.type2test(b" ").join, None) self.assertRaises(TypeError, self.type2test(b" ").join, None)
# Error handling and cleanup when some item in the middle of the # Error handling and cleanup when some item in the middle of the
# sequence has the wrong type. # sequence has the wrong type.

View File

@ -1106,6 +1106,7 @@ Ezio Melotti
Doug Mennella Doug Mennella
Dimitri Merejkowsky Dimitri Merejkowsky
Brian Merrell Brian Merrell
Bruce Merry
Alexis Métaireau Alexis Métaireau
Luke Mewburn Luke Mewburn
Carl Meyer Carl Meyer

View File

@ -0,0 +1 @@
Drop the GIL during large ``bytes.join`` operations. Patch by Bruce Merry.

View File

@ -18,6 +18,9 @@ STRINGLIB(bytes_join)(PyObject *sep, PyObject *iterable)
Py_buffer *buffers = NULL; Py_buffer *buffers = NULL;
#define NB_STATIC_BUFFERS 10 #define NB_STATIC_BUFFERS 10
Py_buffer static_buffers[NB_STATIC_BUFFERS]; Py_buffer static_buffers[NB_STATIC_BUFFERS];
#define GIL_THRESHOLD 1048576
int drop_gil = 1;
PyThreadState *save;
seq = PySequence_Fast(iterable, "can only join an iterable"); seq = PySequence_Fast(iterable, "can only join an iterable");
if (seq == NULL) { if (seq == NULL) {
@ -65,12 +68,21 @@ STRINGLIB(bytes_join)(PyObject *sep, PyObject *iterable)
buffers[i].buf = PyBytes_AS_STRING(item); buffers[i].buf = PyBytes_AS_STRING(item);
buffers[i].len = PyBytes_GET_SIZE(item); buffers[i].len = PyBytes_GET_SIZE(item);
} }
else if (PyObject_GetBuffer(item, &buffers[i], PyBUF_SIMPLE) != 0) { else {
PyErr_Format(PyExc_TypeError, if (PyObject_GetBuffer(item, &buffers[i], PyBUF_SIMPLE) != 0) {
"sequence item %zd: expected a bytes-like object, " PyErr_Format(PyExc_TypeError,
"%.80s found", "sequence item %zd: expected a bytes-like object, "
i, Py_TYPE(item)->tp_name); "%.80s found",
goto error; i, Py_TYPE(item)->tp_name);
goto error;
}
/* If the backing objects are mutable, then dropping the GIL
* opens up race conditions where another thread tries to modify
* the object which we hold a buffer on it. Such code has data
* races anyway, but this is a conservative approach that avoids
* changing the behaviour of that data race.
*/
drop_gil = 0;
} }
nbufs = i + 1; /* for error cleanup */ nbufs = i + 1; /* for error cleanup */
itemlen = buffers[i].len; itemlen = buffers[i].len;
@ -102,6 +114,12 @@ STRINGLIB(bytes_join)(PyObject *sep, PyObject *iterable)
/* Catenate everything. */ /* Catenate everything. */
p = STRINGLIB_STR(res); p = STRINGLIB_STR(res);
if (sz < GIL_THRESHOLD) {
drop_gil = 0; /* Benefits are likely outweighed by the overheads */
}
if (drop_gil) {
save = PyEval_SaveThread();
}
if (!seplen) { if (!seplen) {
/* fast path */ /* fast path */
for (i = 0; i < nbufs; i++) { for (i = 0; i < nbufs; i++) {
@ -110,19 +128,23 @@ STRINGLIB(bytes_join)(PyObject *sep, PyObject *iterable)
memcpy(p, q, n); memcpy(p, q, n);
p += n; p += n;
} }
goto done;
} }
for (i = 0; i < nbufs; i++) { else {
Py_ssize_t n; for (i = 0; i < nbufs; i++) {
char *q; Py_ssize_t n;
if (i) { char *q;
memcpy(p, sepstr, seplen); if (i) {
p += seplen; memcpy(p, sepstr, seplen);
p += seplen;
}
n = buffers[i].len;
q = buffers[i].buf;
memcpy(p, q, n);
p += n;
} }
n = buffers[i].len; }
q = buffers[i].buf; if (drop_gil) {
memcpy(p, q, n); PyEval_RestoreThread(save);
p += n;
} }
goto done; goto done;
@ -138,3 +160,4 @@ done:
} }
#undef NB_STATIC_BUFFERS #undef NB_STATIC_BUFFERS
#undef GIL_THRESHOLD