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.

Sign up for GitHub

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

Jump to bottom

gh-89240: Limit multiprocessing.Pool to 61 workers on Windows #102920

Closed
wants to merge 1 commit into from
Closed

gh-89240: Limit multiprocessing.Pool to 61 workers on Windows #102920

wants to merge 1 commit into from

Conversation

aisk
Copy link
Member

@aisk aisk commented Mar 22, 2023 •
edited by bedevere-bot

@bedevere-bot bedevere-bot mentioned this pull request Mar 22, 2023
Open
@erlend-aasland erlend-aasland changed the title gh-89240: limit multiprocessing.Pool to 61 workers on windows gh-89240: Limit multiprocessing.Pool to 61 workers on Windows Jan 6, 2024
hauntsaninja
hauntsaninja reviewed Jan 7, 2024
View reviewed changes
Copy link
Contributor

@hauntsaninja hauntsaninja left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me, compare to https://github.com/python/cpython/pull/13132/files. Although that PR complains 63 is the max and the overhead is 2 objects.

@aisk
Copy link
Member Author

aisk commented Jan 7, 2024

It looks like #107873 can totally resolve the original issue if it has been merged, rather than pointing out the limitations in document and error messages like this PR.
So I think it's better to wait until #107873 is merged and close the current PR, or if it's not, then we can restart the review process.

@zooba
Copy link
Member

zooba commented Jan 8, 2024

My PR is waiting on reviews and/or testing. If a couple of people want to confirm that it works, then we can merge it.

Otherwise, this PR looks fine to me.

@aisk
Copy link
Member Author

aisk commented Feb 25, 2024

Since #107873 has been merged, this issue can be closed now. Still, thank you to everyone who reviewed this!

@aisk aisk closed this Feb 25, 2024
@aisk aisk deleted the limit-pool-size branch February 25, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hauntsaninja hauntsaninja hauntsaninja left review comments

@eryksun eryksun Awaiting requested review from eryksun

@corona10 corona10 Awaiting requested review from corona10

@zooba zooba Awaiting requested review from zooba

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aisk @zooba @hauntsaninja @bedevere-bot