From 4c774089b64122f84549c94c3745cfbedf2f6e22 Mon Sep 17 00:00:00 2001 From: Jim Crist-Harif Date: Mon, 15 Nov 2021 11:52:09 -0600 Subject: [PATCH 1/3] Don't release GIL for non-blocking syscalls Previously, every operation in `socket` would release the GIL, allowing other threads to progress. This makes sense when using threads and blocking sockets, but when using non-blocking sockets (as done with asyncio) these operations return quickly. In the presence of background threads this can lead to the "convoy effect", where the GIL is acquired and held by the background thread every time the IO thread releases it, and then the IO thread blocks until it regains ownership of the GIL, causing a massive decrease in IO operations per second. One possible fix for this is to have IO-heavy threads release the GIL less frequently. In the case of asyncio, the following changes were needed: - Don't release the GIL for socket operations on non-blocking sockets. - Don't release the GIL for select (epoll, select, kqueue, ...) calls with a timeout of 0. With these changes, asyncio doesn't see as degraded of an output when a background thread holds the GIL. --- Include/ceval.h | 14 ++++++++++++++ Modules/selectmodule.c | 28 ++++++++++++++-------------- Modules/socketmodule.c | 6 +++--- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/Include/ceval.h b/Include/ceval.h index 1b57f6ea20f6f0..7258b2ae9a3fe6 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -147,6 +147,20 @@ PyAPI_FUNC(void) PyEval_ReleaseThread(PyThreadState *tstate); #define Py_END_ALLOW_THREADS PyEval_RestoreThread(_save); \ } + +/* Conditionally release and restore the GIL. */ +#define _Py_BEGIN_ALLOW_THREADS_COND(cond) \ + { \ + PyThreadState *_save = NULL; \ + if (cond) { \ + _save = PyEval_SaveThread(); \ + } +#define _Py_END_ALLOW_THREADS_COND \ + if (_save != NULL) { \ + PyEval_RestoreThread(_save); \ + } \ + } + /* Masks and values used by FORMAT_VALUE opcode. */ #define FVC_MASK 0x3 #define FVC_NONE 0x0 diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index ff1c028d0d6720..302d9a084b394b 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -323,10 +323,11 @@ select_select_impl(PyObject *module, PyObject *rlist, PyObject *wlist, } do { - Py_BEGIN_ALLOW_THREADS + int timeout_is_zero = (tvp != NULL && tvp->tv_sec == 0 && tvp->tv_usec == 0); + _Py_BEGIN_ALLOW_THREADS_COND(!timeout_is_zero) errno = 0; n = select(max, &ifdset, &ofdset, &efdset, tvp); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (errno != EINTR) break; @@ -632,10 +633,10 @@ select_poll_poll_impl(pollObject *self, PyObject *timeout_obj) /* call poll() */ async_err = 0; do { - Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_ALLOW_THREADS_COND(ms != 0) errno = 0; poll_result = poll(self->ufds, self->ufd_len, (int)ms); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (errno != EINTR) break; @@ -945,10 +946,10 @@ select_devpoll_poll_impl(devpollObject *self, PyObject *timeout_obj) do { /* call devpoll() */ - Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_ALLOW_THREADS_COND(ms != 0) errno = 0; poll_result = ioctl(self->fd_devpoll, DP_POLL, &dvp); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (errno != EINTR) break; @@ -1415,17 +1416,13 @@ pyepoll_internal_ctl(int epfd, int op, int fd, unsigned int events) case EPOLL_CTL_MOD: ev.events = events; ev.data.fd = fd; - Py_BEGIN_ALLOW_THREADS result = epoll_ctl(epfd, op, fd, &ev); - Py_END_ALLOW_THREADS break; case EPOLL_CTL_DEL: /* In kernel versions before 2.6.9, the EPOLL_CTL_DEL * operation required a non-NULL pointer in event, even * though this argument is ignored. */ - Py_BEGIN_ALLOW_THREADS result = epoll_ctl(epfd, op, fd, &ev); - Py_END_ALLOW_THREADS break; default: result = -1; @@ -1573,10 +1570,10 @@ select_epoll_poll_impl(pyEpoll_Object *self, PyObject *timeout_obj, } do { - Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_ALLOW_THREADS_COND(ms != 0) errno = 0; nfds = epoll_wait(self->epfd, evs, maxevents, (int)ms); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (errno != EINTR) break; @@ -2179,11 +2176,14 @@ select_kqueue_control_impl(kqueue_queue_Object *self, PyObject *changelist, } do { - Py_BEGIN_ALLOW_THREADS + int timeout_is_zero = (ptimeoutspec != NULL && + ptimeoutspec->tv_sec == 0 && + ptimeoutspec->tv_nsec == 0); + _Py_BEGIN_ALLOW_THREADS_COND(!timeout_is_zero) errno = 0; gotevents = kevent(self->kqfd, chl, nchanges, evl, maxevents, ptimeoutspec); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (errno != EINTR) break; diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index ed1043c0c43a50..7e98699f6860a0 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -813,7 +813,7 @@ internal_select(PySocketSockObject *s, int writing, _PyTime_t interval, When the function is retried, recompute the timeout using a monotonic clock. sock_call_ex() must be called with the GIL held. The socket function is - called with the GIL released. */ + called with the GIL released if the socket is blocking. */ static int sock_call_ex(PySocketSockObject *s, int writing, @@ -896,9 +896,9 @@ sock_call_ex(PySocketSockObject *s, /* inner loop to retry sock_func() when sock_func() is interrupted by a signal */ while (1) { - Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_ALLOW_THREADS_COND(s->sock_timeout) res = sock_func(s, data); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND if (res) { /* sock_func() succeeded */ From e313019bf56c06580242c6d62aa479ecaf84385b Mon Sep 17 00:00:00 2001 From: Jim Crist-Harif Date: Tue, 16 Nov 2021 13:15:59 -0600 Subject: [PATCH 2/3] Skip releasing the GIL in nonblocking socket.close --- Modules/socketmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 7e98699f6860a0..40f5dbfd4d68b6 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -3147,9 +3147,9 @@ sock_close(PySocketSockObject *s, PyObject *Py_UNUSED(ignored)) http://lwn.net/Articles/576478/ and http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html for more details. */ - Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_ALLOW_THREADS_COND(s->sock_timeout) res = SOCKETCLOSE(fd); - Py_END_ALLOW_THREADS + _Py_END_ALLOW_THREADS_COND /* bpo-30319: The peer can already have closed the connection. Python ignores ECONNRESET on close(). */ if (res < 0 && errno != ECONNRESET) { From 196f9e6a310c8a3d02d1b2ff30b9b3626200e662 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Thu, 3 Feb 2022 08:47:59 +0900 Subject: [PATCH 3/3] Update Include/ceval.h --- Include/ceval.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Include/ceval.h b/Include/ceval.h index 7258b2ae9a3fe6..558bffa2e6815e 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -156,10 +156,10 @@ PyAPI_FUNC(void) PyEval_ReleaseThread(PyThreadState *tstate); _save = PyEval_SaveThread(); \ } #define _Py_END_ALLOW_THREADS_COND \ - if (_save != NULL) { \ - PyEval_RestoreThread(_save); \ - } \ - } + if (_save != NULL) { \ + PyEval_RestoreThread(_save); \ + } \ + } /* Masks and values used by FORMAT_VALUE opcode. */ #define FVC_MASK 0x3