From: Richard Si <63936253+ichard26@users.noreply.github.com> Date: Sat, 13 Aug 2022 17:46:52 +0000 (-0400) Subject: Delay worker count determination X-Git-Url: https://git.madduck.net/etc/vim.git/commitdiff_plain/c0cc19b5b3371842d696875897bebefebd5e1596?hp=afed2c01903465f9a486ac481a66aa3413cc1b01 Delay worker count determination 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 --- diff --git a/src/black/__init__.py b/src/black/__init__.py index 117dc83..86a0b63 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -1,6 +1,5 @@ import io import json -import os import platform import re import sys @@ -28,11 +27,6 @@ from typing import ( 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 @@ -92,7 +86,6 @@ from blib2to3.pgen2 import token from blib2to3.pytree import Leaf, Node COMPILED = Path(__file__).suffix in (".pyd", ".so") -DEFAULT_WORKERS: Final = os.cpu_count() # types FileContent = str @@ -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", @@ -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: diff --git a/src/black/concurrency.py b/src/black/concurrency.py index d77ea40..bdc368d 100644 --- a/src/black/concurrency.py +++ b/src/black/concurrency.py @@ -6,6 +6,7 @@ NOTE: this module is only imported if we need to format several files at once. import asyncio import logging +import os import signal import sys from concurrent.futures import Executor, ProcessPoolExecutor, ThreadPoolExecutor @@ -15,7 +16,7 @@ from typing import Any, Iterable, Optional, Set 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 @@ -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