gh-146219: Document reusing a thread state across repeated foreign-thread calls#146221
gh-146219: Document reusing a thread state across repeated foreign-thread calls#146221gpshead wants to merge 2 commits intopython:mainfrom
Conversation
Add a subsection under "Non-Python created threads" explaining the performance cost of creating/destroying a PyThreadState on every Ensure/Release cycle and showing how to keep one alive for the thread's lifetime instead.
Doc/c-api/threads.rst
Outdated
|
|
||
| /* Thread startup: create and pin the state. */ | ||
| PyGILState_STATE outer = PyGILState_Ensure(); | ||
| PyThreadState *saved = PyEval_SaveThread(); |
There was a problem hiding this comment.
Hm, what's the point of detaching the thread state again? PyGILState_Ensure is able to handle nested calls.
There was a problem hiding this comment.
I can see how this is a little confusing as written (i oversimplified). check the latest version (commit added).
open question... this is to work around the PyGILState recursion counter and basically "abuse" it as a way to prevent _Release from destroying the Python Thread State it creates internally. I don't really want to mention that detail but maybe to avoid questions this should?
as being an init, do things, finalizer trio where lots of stuff without the GIL held can happen inbetween. There might not be a GIL but when used in builds where there is you don't want to hold it. There's an internal recursion counter within the PyGILState APIs, if it goes to 0 on Release, any Python thread state that it created is destroyed. We're working around that. Should I mention that internals detail?
| does not hold the GIL while off doing non-Python work. Stash ``outer`` | ||
| and ``saved`` somewhere that survives for the thread's lifetime (for | ||
| example, in thread-local storage):: | ||
|
|
||
| PyGILState_STATE outer = PyGILState_Ensure(); | ||
| PyThreadState *saved = PyEval_SaveThread(); | ||
|
|
||
| Each subsequent call into Python from this thread reuses the pinned | ||
| state; the inner Release decrements the nesting counter but does not | ||
| destroy the thread state because the outer Ensure is still | ||
| outstanding:: |
There was a problem hiding this comment.
To elaborate on my previous comment: we shouldn't need a "pinned" thread state at all for this example to be correct. If we're trying to demonstrate attaching/detaching, I think we should just use Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS.
Add a subsection under "Non-Python created threads" explaining the performance cost of creating/destroying a PyThreadState on every Ensure/Release cycle and showing how to keep one alive for the thread's lifetime instead.
(read them & see the issue for more details)
📚 Documentation preview 📚: https://cpython-previews--146221.org.readthedocs.build/