Skip to content
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

Add option to limit the number of mp workers #14

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

ThadHouse
Copy link
Contributor

Due to python/cpython#89240, any use of mp.Pool with a number of workers greater than 60 fails. This means that by using cpu_count(), any system with more than 60 logical cores will crash when attempting to run.

Solve this by adding a flag to allow limiting the number of workers for users with systems with that many cores.

@BlankSpruce
Copy link
Owner

BlankSpruce commented Jan 11, 2024

I appreciate the attempt. I've got two questions though:

  • I acknowledge that due to that referred issue number of workers should be limited. Wouldn't it be enough to provide such limitation just in the code?
  • Assuming that you have a good case for number of workers to be manually tuned by the user, would it be ok with you if I introduced -j/--jobs flag similar to how tools like make and ninja do it?

@ThadHouse
Copy link
Contributor Author

Limiting in code to a maximum of 60 would be fine. Thats actually what our project does. I just didn't know which you'd prefer.

I'm fine with making the flags jobs instead of workers as well. Not set on any name. But I don't have any compelling reason not to just limit the number of runners and be done with it.

@BlankSpruce
Copy link
Owner

I prefer to not introduce new flags or options unless they accomplish something user might be interested in to have a concious choice. I'll research the topic further and probably today, as in CET zone, I'll release a new version with that change.

@ThadHouse
Copy link
Contributor Author

ThadHouse commented Jan 11, 2024

@BlankSpruce it needs to be 60, not 61. You’re allowed 63 handles, and mp itself takes 3 of them. 61 will still fail. The issue says 61, but I think one more has been added since then which drops it to 60.

python/cpython#89240
Any use of mp.Pool with a number of workers greater than 60 fails
on Windows machines.

Twofold solution is introduced:
- add option for users to explicitly change number of workers
through -w/--workers, this is similar to solution used in black
- add limitation in the code for Windows machines

Co-Authored-By: Blank Spruce <32396809+BlankSpruce@users.noreply.github.com>
@BlankSpruce
Copy link
Owner

BlankSpruce commented Jan 11, 2024

Indeed, python bug tracker also mentions it here:
https://bugs.python.org/issue26903
as well as black's solution uses 60:
https://github.com/psf/black/blob/main/src/black/concurrency.py#L88

@BlankSpruce BlankSpruce merged commit 5f14d48 into BlankSpruce:master Jan 11, 2024
9 checks passed
@BlankSpruce
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants