Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

In Manager use multiprocessing.Pool not ProcessPoolExecutor #325

Closed
Tracked by #341 ...
JackKelly opened this issue Nov 2, 2021 · 4 comments · Fixed by #453
Closed
Tracked by #341 ...

In Manager use multiprocessing.Pool not ProcessPoolExecutor #325

JackKelly opened this issue Nov 2, 2021 · 4 comments · Fixed by #453
Assignees
Labels
bug Something isn't working

Comments

@JackKelly
Copy link
Member

JackKelly commented Nov 2, 2021

We probably want the worker processes to be demonic.

If that's not possible then maybe have Manager only send a single batch to each process at a time (so the workers will terminate faster)

noxdafox/pebble#76 suggests that ProcessPoolExecutor is not demonic, whilst multiprocessing.Pool is demonic

https://stackoverflow.com/questions/56237493/how-to-make-tasks-in-processpoolexecutor-behave-like-daemon-process suggests we should be using multiprocessing.Pool

@JackKelly JackKelly added the bug Something isn't working label Nov 2, 2021
@JackKelly JackKelly moved this to Todo in Nowcasting Nov 2, 2021
@JackKelly JackKelly mentioned this issue Nov 2, 2021
30 tasks
@JackKelly JackKelly changed the title If the main process dies then kill all worker processes In Manager use multiprocessing.Pool not ProcessPoolExecutor Nov 5, 2021
@peterdudfield
Copy link
Contributor

could we use 'futures.ThreadPoolExecutor' - I have admit I do get a bit confused with some multiprocess things

@JackKelly
Copy link
Member Author

Hehe, me too!

So, I'm pretty sure we want to use processes not threads because the tasks are CPU bound, and python threads run on a single CPU core due to the python global interpreter lock.

Don't worry, I can have a look at this issue real soon now

@JackKelly
Copy link
Member Author

I'll start working on this now...

@JackKelly
Copy link
Member Author

JackKelly commented Nov 18, 2021

OK, this is implemented in PR #453

This all makes me think that issue #321 would be good to do soon. But maybe not today 🙂

This doesn't seem to have sped things up... but I wasn't really expecting it to... Speeding things up is what I'll work on now (in a separate PR)...

Repository owner moved this from In Progress to Done in Nowcasting Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants