FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-26903: Limit ProcessPoolExecutor to 61 workers on Windows by brianquinlan · Pull Request #13132 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-26903: Limit ProcessPoolExecutor to 61 workers on Windows #13132

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

Merged
zooba merged 3 commits into from
May 8, 2019
Conversation 9 Commits 3 Checks 0 Files changed
Merged

bpo-26903: Limit ProcessPoolExecutor to 61 workers on Windows #13132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5536b03
bpo-26903: Limit ProcessPoolExecutor to 61 works on Windows
brianquinlan May 6, 2019
b513bba
📜🤖 Added by blurb_it.
blurb-it[bot] May 6, 2019
97c1084
Update Lib/test/test_concurrent_futures.py
zooba May 6, 2019
File filter

Filter by extension

Filter by extension .py  (2) .rst  (2) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
4 changes: 4 additions & 0 deletions Doc/library/concurrent.futures.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ to a :class:`ProcessPoolExecutor` will result in deadlock.
given, it will default to the number of processors on the machine.
If *max_workers* is lower or equal to ``0``, then a :exc:`ValueError`
will be raised.
On Windows, *max_workers* must be equal or lower than ``61``. If it is not
then :exc:`ValueError` will be raised. If *max_workers* is ``None``, then
the default chosen will be at most ``61``, even if more processors are
available.
*mp_context* can be a multiprocessing context or None. It will be used to
launch the workers. If *mp_context* is ``None`` or not given, the default
multiprocessing context is used.
Expand Down
14 changes: 14 additions & 0 deletions Lib/concurrent/futures/process.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import weakref
from functools import partial
import itertools
import sys
import traceback

# Workers are created as daemon threads and processes. This is done to allow the
Expand Down Expand Up @@ -109,6 +110,12 @@ def _python_exit():
EXTRA_QUEUED_CALLS = 1


# On Windows, WaitForMultipleObjects is used to wait for processes to finish.
# It can wait on, at most, 63 objects. There is an overhead of two objects:
# - the result queue reader
# - the thread wakeup reader
_MAX_WINDOWS_WORKERS = 63 - 2

# Hack to embed stringification of remote traceback in local traceback

class _RemoteTraceback(Exception):
Expand Down Expand Up @@ -505,9 +512,16 @@ def __init__(self, max_workers=None, mp_context=None,

if max_workers is None:
self._max_workers = os.cpu_count() or 1
if sys.platform == 'win32':
self._max_workers = min(_MAX_WINDOWS_WORKERS,
self._max_workers)
else:
if max_workers <= 0:
raise ValueError("max_workers must be greater than 0")
elif (sys.platform == 'win32' and
max_workers > _MAX_WINDOWS_WORKERS):
raise ValueError(

JelleZijlstra May 7, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should we consider automatically changing max_workers to 61 instead? That would make for a nicer API for people using max_workers=os.cpu_count(), and it would make code like what was in Black before https://github.com/python/black/pull/838/files work unchanged.

On the other hand, it may be unexpected that we silently use fewer workers than the user asked for.

brianquinlan May 7, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I thought silently using fewer processes could be surprising. I also think that it's pretty unusual in the standard library to silently fix bad argument values.

zooba May 8, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like raising when too many workers are requested, and doing it quietly when it was "automatic".
f"max_workers must be <= {_MAX_WINDOWS_WORKERS}")

self._max_workers = max_workers

Expand Down
7 changes: 7 additions & 0 deletions Lib/test/test_concurrent_futures.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,13 @@ def test_default_workers(self):


class ProcessPoolExecutorTest(ExecutorTest):

@unittest.skipUnless(sys.platform=='win32', 'Windows-only process limit')
def test_max_workers_too_large(self):
with self.assertRaisesRegex(ValueError,
"max_workers must be <= 61"):
futures.ProcessPoolExecutor(max_workers=62)

def test_killed_child(self):
# When a child process is abruptly terminated, the whole pool gets
# "broken".
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2019-05-06-19-17-04.bpo-26903.4payXb.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Limit `max_workers` in `ProcessPoolExecutor` to 61 to work around a WaitForMultipleObjects limitation.
Back | FazBrowse Home | New Git URL