Skip to content

Commit

Permalink
Delay worker count determination
Browse files Browse the repository at this point in the history
os.cpu_count() can return None (sounds like a super arcane edge case
though) so the type annotation for the `workers` parameter of
`black.main` is wrong. This *could* technically cause a runtime
TypeError since it'd trip one of mypyc's runtime type checks so we
might as well fix it.

Reading the documentation (and cross-checking with the source code),
you are actually allowed to pass None as `max_workers` to
`concurrent.futures.ProcessPoolExecutor`. If it is None, the pool
initializer will simply call os.cpu_count() [^1] (defaulting to 1 if it
returns None [^2]). It'll even round down the worker count to a level
that's safe for Windows.

... so theoretically we don't even need to call os.cpu_count()
ourselves, but our Windows limit is 60 (unlike the stdlib's 61) and I'd
prefer not accidentally reintroducing a crash on machines with many,
many CPU cores.

[^1]: https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.ProcessPoolExecutor
[^2]: https://github.com/python/cpython/blob/a372a7d65320396d44e8beb976e3a6c382963d4e/Lib/concurrent/futures/process.py#L600
  • Loading branch information
ichard26 committed Aug 27, 2022
1 parent afed2c0 commit c0cc19b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 16 deletions.
14 changes: 3 additions & 11 deletions src/black/__init__.py
@@ -1,6 +1,5 @@
import io
import json
import os
import platform
import re
import sys
Expand Down Expand Up @@ -28,11 +27,6 @@
Union,
)

if sys.version_info >= (3, 8):
from typing import Final
else:
from typing_extensions import Final

import click
from click.core import ParameterSource
from mypy_extensions import mypyc_attr
Expand Down Expand Up @@ -92,7 +86,6 @@
from blib2to3.pytree import Leaf, Node

COMPILED = Path(__file__).suffix in (".pyd", ".so")
DEFAULT_WORKERS: Final = os.cpu_count()

# types
FileContent = str
Expand Down Expand Up @@ -371,9 +364,8 @@ def validate_regex(
"-W",
"--workers",
type=click.IntRange(min=1),
default=DEFAULT_WORKERS,
show_default=True,
help="Number of parallel workers",
default=None,
help="Number of parallel workers [default: number of CPUs in the system]",
)
@click.option(
"-q",
Expand Down Expand Up @@ -448,7 +440,7 @@ def main( # noqa: C901
extend_exclude: Optional[Pattern[str]],
force_exclude: Optional[Pattern[str]],
stdin_filename: Optional[str],
workers: int,
workers: Optional[int],
src: Tuple[str, ...],
config: Optional[str],
) -> None:
Expand Down
11 changes: 6 additions & 5 deletions src/black/concurrency.py
Expand Up @@ -6,6 +6,7 @@

import asyncio
import logging
import os
import signal
import sys
from concurrent.futures import Executor, ProcessPoolExecutor, ThreadPoolExecutor
Expand All @@ -15,7 +16,7 @@

from mypy_extensions import mypyc_attr

from black import DEFAULT_WORKERS, WriteBack, format_file_in_place
from black import WriteBack, format_file_in_place
from black.cache import Cache, filter_cached, read_cache, write_cache
from black.mode import Mode
from black.output import err
Expand Down Expand Up @@ -87,13 +88,13 @@ def reformat_many(
maybe_install_uvloop()

executor: Executor
worker_count = workers if workers is not None else DEFAULT_WORKERS
if workers is None:
workers = os.cpu_count() or 1
if sys.platform == "win32":
# Work around https://bugs.python.org/issue26903
assert worker_count is not None
worker_count = min(worker_count, 60)
workers = min(workers, 60)
try:
executor = ProcessPoolExecutor(max_workers=worker_count)
executor = ProcessPoolExecutor(max_workers=workers)
except (ImportError, NotImplementedError, OSError):
# we arrive here if the underlying system does not support multi-processing
# like in AWS Lambda or Termux, in which case we gracefully fallback to
Expand Down

0 comments on commit c0cc19b

Please sign in to comment.