Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-45819: Avoid releasing the GIL in nonblocking socket operations#29579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Previously, every operation in `socket` would release the GIL, allowingother threads to progress. This makes sense when using threads andblocking sockets, but when using non-blocking sockets (as done withasyncio) these operations return quickly. In the presence of backgroundthreads this can lead to the "convoy effect", where the GIL is acquiredand 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 GILless frequently. In the case of asyncio, the following changes wereneeded:- Don't release the GIL for socket operations on non-blocking sockets.- Don't release the GIL for select (epoll, select, kqueue, ...) callswith a timeout of 0.With these changes, asyncio doesn't see as degraded of an output when abackground thread holds the GIL.
@@ -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) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I added some helper macros here, but am not sure if that's best practice. Happy to change this if others have a better suggestion.
result = epoll_ctl(epfd, op, fd, &ev); | ||
Py_END_ALLOW_THREADS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As far as I can tellepoll_ctl
operations like this always complete quickly, so releasing the GIL here isn't really necessary.
res = sock_func(s, data); | ||
Py_END_ALLOW_THREADS | ||
_Py_END_ALLOW_THREADS_COND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There are likely other socket operations where releasing the GIL could be avoided, but this gets the bulk of those done frequently in a loop (send
/recv
, ...).socket.close()
is also handled below.
This PR is stale because it has been open for 30 days with no activity. |
Uh oh!
There was an error while loading.Please reload this page.
Py_BEGIN_ALLOW_THREADS | ||
_Py_BEGIN_ALLOW_THREADS_COND(s->sock_timeout) | ||
res = SOCKETCLOSE(fd); | ||
Py_END_ALLOW_THREADS | ||
_Py_END_ALLOW_THREADS_COND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Don't change here.s->sock_timeout
doesn't affect to close.
Unlike send/recv, close can not be called frequently more than creating socket. So this shouldn't be a big performance problem.
bedevere-bot commentedFeb 2, 2022
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Uh oh!
There was an error while loading.Please reload this page.
Previously, every operation in
socket
would release the GIL, allowingother 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:
with a timeout of 0.
See the corresponding issue athttps://bugs.python.org/issue45819
for more information.
https://bugs.python.org/issue45819