-
Notifications
You must be signed in to change notification settings - Fork 293
address darwin system parallel threading issues, without numpydoc fixes #708
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,11 @@ def g(_obj_td, *args, **kwargs): | |
| _obj_td.result = res | ||
| @wraps(f) | ||
| def _f(*args, **kwargs): | ||
| res = (Thread,Process)[process](target=g, args=args, kwargs=kwargs) | ||
| if process: | ||
| Proc = get_context('fork').Process if sys.platform == 'darwin' else Process | ||
| else: | ||
| Proc = Thread | ||
| res = Proc(target=g, args=args, kwargs=kwargs) | ||
| res._args = (res,)+res._args | ||
| res.start() | ||
| return res | ||
|
|
@@ -123,7 +127,9 @@ def parallel(f, items, *args, n_workers=defaults.cpus, total=None, progress=None | |
| kwpool = {} | ||
| if threadpool: pool = ThreadPoolExecutor | ||
| else: | ||
| if not method and sys.platform == 'darwin': method='fork' | ||
| if not method and sys.platform == 'darwin': | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That certainly helps. Do we want to force that as fix? or |
||
| # Use fork only if function is defined in __main__ (notebooks/REPL), otherwise use spawn | ||
| method = 'fork' if getattr(f, '__module__', None) == '__main__' else 'spawn' | ||
| if method: kwpool['mp_context'] = get_context(method) | ||
| pool = ProcessPoolExecutor | ||
| with pool(n_workers, pause=pause, **kwpool) as ex: | ||
|
|
@@ -158,7 +164,8 @@ async def limited_task(item): | |
| # %% ../nbs/03a_parallel.ipynb | ||
| def run_procs(f, f_done, args): | ||
| "Call `f` for each item in `args` in parallel, yielding `f_done`" | ||
| processes = L(args).map(Process, args=arg0, target=f) | ||
| Proc = get_context('fork').Process if sys.platform == 'darwin' else Process | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| processes = L(args).map(Proc, args=arg0, target=f) | ||
| for o in processes: o.start() | ||
| yield from f_done() | ||
| processes.map(Self.join()) | ||
|
|
||
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 don't think we want this, do we? I'd expect
process=Trueto give us a normalProcess. Why would we want something different on Mac?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.
Disclaimer my understanding of this is not the best, but I know a bit about concurrency // distributed systems.
My understanding is that what
Processdoes depends on default start method of the platform (which I suppose, could change in an update, but not likely).Anyway, I believe for true linux systems
Processforks a copy while macOS (apple silicon, maybe the old intel chips too) spawns a new interpreter.So supposing
threaded(process=True)linux ends up with the forked process and macOS does spawn which carries the implications of a fresh interpreter, reloading the module, and requires picklablity (which is its own headache in its own way)There is a push to using spawn, but at the moment picklability + nested functions will likely lead to errors (which I've encountered)
So the below
get_context('fork').Processis the special-case to makeprocess=Truebehave the same way onmacOSas it does onLinux— not to make it special, but to avoid macOS being the odd one out.Provided that I understand.