Repository navigation
Move the eval_breaker to PyThreadState #112175
Description
Activity
- addedtype-featureA feature request or enhancementA feature request or enhancementinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)3.13only security fixesonly security fixes
on Nov 16, 2023 If I remember correctly,
eval_breakeralso contains the global version for instrumentation. Will having multipleeval_breakers in different threads work with the current instrumentation mechanism?I don't think it'll be an issue in the default build, but I'll need to think about how instrumentation works in
--disable-gilbuilds with multiple threads.I think Mark's idea works basically like:
- Set the
eval_breakerinPyThreadState.eval_breaker. - On
_PyThreadState_Detach(), copy bits fromPyThreadState.eval_breakertoPyInterpreterState.interp_eval_breaker - On
_PyThreadState_Attach(), copy bits fromPyInterpreterState.interp_eval_breakerback toPyThreadState.eval_breaker
In
--disable-gilbuilds, we will need to loop over all the threads when setting interpreter-wide bits.- Set the
I'm currently working on this, so if anyone has any related ideas/comments, please post them!
@markshannon, do you have any more context on your per-thread
eval_breakeridea that Sam described a couple comments up? Also, this is a heads up that I'm working on this, since you expressed an interest in it in a previous PR comment.I'm fleshing out exactly how all the flags will work in free-threaded vs. normal builds. As Sam says above, we need to loop over all threads for interpreter-wide flags in a free-threaded build, and this is pretty straightforward. All
eval_breakerflags will go in a new member ofPyThreadState. I'm planning on preserving the interpreter-wideinterp_eval_breaker(renamed fromeval_breaker) to keep holding the global instrumentation version. Properly supporting the version number in a free-threaded build will be handled separately from this issue.For normal builds, if we want to avoid looping over all threads, we can set interpreter-wide flags on the active thread and use
interp_eval_breakerto shuffle them between threads when a context switch happens. Each flag is slightly different, so here's my current plan for how to handle them (this is just for the normal build, where we still have the GIL):_PY_SIGNALS_PENDING_BITis easy - signals are only ever handled by the main thread in the main interpreter. We set the bit on that thread when a signal is received._PY_ASYNC_EXCEPTION_BITalso applies to a single, known thread, so we set the bit on that specific thread._PY_GIL_DROP_REQUEST_BITis again set on a specific thread (the one holding the GIL). This is only set while holdinggil->mutex, which should ensure that the thread holding the GIL doesn't change between when we decide to set the flag and when we actually set the flag._PY_GC_SCHEDULED_BITcan be handled by any thread in the targeted interpreter. The one existing caller of the private function_Py_ScheduleGC(PyInterpreterState*)schedules a GC for the current interpreter, so I believe it should be safe to change it to operate on aPyThreadState*instead, and always pass the current thread. The next time that thread checks itseval_breaker, it will run the GC. If it yields before then, it will move the GC bit tointerp_eval_breakerfor the next scheduled thread to pick up. If we want to preserve the ability for one interpreter to schedule a GC in another interpreter, the strategy used for_PY_CALLS_TO_DO(next item) should work._PY_CALLS_TO_DOis the least restricted case, because since gh-104812: Run Pending Calls in any Thread #104813, any thread can add a callback to run in any interpreter, and callbacks can be allowed to run in any thread. The general strategy is similar to_PY_GC_SCHEDULED_BIT: set the bit on the current thread, and if that thread yields before processing the callback, move the bit tointerp_eval_breaker. If no thread in the targeted interpreter holds the GIL, directly setinterp_eval_breaker.- To safely set this signal across interpreters, I will ensure that a) this bit is only set while holding
gil->mutexfor the signaled interpreter (similar to_PY_GIL_DROP_REQUEST), and b) bits will only be transferred betweeneval_breakerandinterp_eval_breakerwhile holdinggil->mutex. Half of part b) already happens withupdate_eval_breaker_from_thread(), called fromtake_gil(). I'll add a reversed version of that indrop_gil(). Both directions will only apply to_PY_GC_SCHEDULED_BITand_PY_CALLS_TO_DO_BIT, since the other flags only apply to a single thread.
- To safely set this signal across interpreters, I will ensure that a) this bit is only set while holding
Does this all sound reasonable, especially for the interpreter-wide flags? If it's too complicated, I could set them by looping over all threads in both build types. That would add some overhead to normal builds, but I expect it wouldn't be measurable overall. It's not much work to try out both implementations, so I could see if there's a measurable performance difference, if that would help make the decision.
Feature or enhancement
The
eval_breakeris a variable that keeps track of requests to break out of the eval loop to handle things like signals, run a garbage collection, or handle asynchronous exceptions. It is currently in the interpreter state (ininterp->ceval.eval_breaker). However, some of the events are specific to a given thread. For example, signals and some pending calls can only be executed on the "main" thread of an interpreter.We should move the
eval_breakertoPyThreadStateto better handle these thread-specific events. This is more important for the--disable-gilbuilds where multiple threads within the same interpreter may be running at the same time.@markshannon suggested a combination of per-interpreter and per-thread state, where the thread copies the per-interpreter eval_breaker state to the per-thread state when it acquires the GIL.
Linked PRs
eval_breakertoPyThreadState#115194