Repository navigation
Make the workqueue threading backend once again fork safe. - #8195
Conversation
As title, this fixes the workqueue threading backend such that it is fork safe from _any_ thread, not just the main thread. Fixes numba#7872
stuartarchibald
added
Effort - medium
lgarrison
left a comment
lgarrison
left a comment
There was a problem hiding this comment.
Looks good to me! The logic seems pretty straightforward: if a parallel function is called and the thread pool is not yet initialized, start it. queues = NULL is used to signal that the thread pool is not active, via a reset_after_fork() handler.
The other piece seems to be that the global NUM_THREADS must be set by the caller before launch_threads() is entered (i.e. set by _launch_threads() in Python). But once it is set, it will be initialized in any subprocesses (because of NUM_THREADS = _INIT_NUM_THREADS; in the fork handler), so subprocesses don't need to call _launch_threads() again.
I guess this means that subprocesses inherit the same upper limit on threads as the parent process. But this is probably fine, since the upper limit is set explicitly by NUMBA_NUM_THREADS or inferred from the parent process affinity mask. Changing either of these in a child process would be a strange use-case, and so I think the current behavior makes sense.
I checked that this PR fixes the original issue abacusorg/abacusutils#47, and fixes the test cases derived from that issue.
|
|
||
| # now run in a multiprocessing pool to get a fork from a | ||
| # non-main thread | ||
| with multiprocessing.Pool(10) as p: |
There was a problem hiding this comment.
needs to force a fork context here
There was a problem hiding this comment.
Good point, OSX defaults to 'spawn'.
There was a problem hiding this comment.
b07d687 fixes, also adds a test skip for OS which don't have fork(2) and adds an assert to ensure the expected threading layer was used.
Thanks for the detailed review @lgarrison, much appreciated.
I thought the same, an upper thread limit that changes would be unusual, and also quite tricky to implement. Such a behaviour could be emulated by creating a larger than needed upper limit and then using thread masks if desired.
Great, thanks for checking. |
stuartarchibald
added
4 - Waiting on reviewer
sklam
added
5 - Ready to merge
3 participants
As title, this fixes the workqueue threading backend such that it
is fork safe from any thread, not just the main thread.
Fixes #7872