mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 00:36:39 +00:00
fix: skip the process pool when only one worker is available (#2173)
`_extract_parallel` spawned a ProcessPoolExecutor whenever there were at least _PARALLEL_THRESHOLD (20) uncached files, even when the resolved worker count was 1. A one-worker pool buys no parallelism: it still pays a process spawn plus an IPC round trip per file, and it is the one residual case where the parent's rebuild watchdog (os._exit) can orphan a worker that is mid-task. The Windows post-commit hook exports GRAPHIFY_MAX_WORKERS=1, so this was the default there for any rebuild touching 20+ uncached files. Gate the pool on the resolved worker count -- after the GRAPHIFY_MAX_WORKERS override and the win32/floor clamps -- and return False when it is 1. That reuses the existing contract: the caller already falls back to `_extract_sequential` in-process when `_extract_parallel` returns False. Tests: no pool is constructed with GRAPHIFY_MAX_WORKERS=1 and 25 uncached files, and a multi-worker run still takes the pool path. Only item 2 of #2173 is addressed here. Item 1 (the `graphify watch` rebuild timeout) needs a maintainer decision first: `watch()` currently arms no timeout at all on any platform -- there is no signal.SIGALRM branch in graphify/watch.py to add an `else` to -- so applying the hook's shape means adding a watchdog that os._exit(1)s a long-running foreground watcher on a slow-but-healthy rebuild. That is a behaviour change rather than a Windows-compat fix, so it is left out of this PR.
This commit is contained in:
committed by
safishamsi
parent
4191b7dee3
commit
556e615cc6
@@ -4330,6 +4330,15 @@ def _extract_parallel(
|
||||
max_workers = min(max_workers, 61)
|
||||
max_workers = max(max_workers, 1)
|
||||
|
||||
# A one-worker pool buys no parallelism: it still pays process spawn plus an
|
||||
# IPC round trip per file, and it is the one residual case where the parent's
|
||||
# rebuild watchdog (os._exit) can orphan a worker that is mid-task. The
|
||||
# Windows post-commit hook exports GRAPHIFY_MAX_WORKERS=1, so this is the
|
||||
# default there. Hand the work back so the caller extracts sequentially in
|
||||
# this process instead (#2173).
|
||||
if max_workers == 1:
|
||||
return False
|
||||
|
||||
# root anchors hash keys / node ids / XAML boundary; cache_location is where
|
||||
# the cache dir is written (defaults to root when not decoupled) (#1774).
|
||||
root_str = str(root)
|
||||
|
||||
@@ -1488,6 +1488,63 @@ def test_extract_parallel_returns_false_on_broken_pool(tmp_path, monkeypatch, ca
|
||||
assert "__main__" in out, "warning must hint at the Windows __main__ guard idiom"
|
||||
|
||||
|
||||
def test_extract_parallel_skips_pool_when_max_workers_is_one(tmp_path, monkeypatch):
|
||||
"""#2173: a resolved worker count of 1 must not spawn a ProcessPoolExecutor.
|
||||
|
||||
The Windows post-commit hook exports GRAPHIFY_MAX_WORKERS=1, so before this the
|
||||
rebuild spawned a one-worker pool for >= _PARALLEL_THRESHOLD files: no
|
||||
parallelism, one process spawn plus an IPC round trip per file, and the only
|
||||
window where the parent's rebuild watchdog (os._exit) can orphan a worker
|
||||
mid-task. _extract_parallel must decline (return False) so the caller extracts
|
||||
sequentially in-process.
|
||||
"""
|
||||
import concurrent.futures
|
||||
from graphify import extract as extract_mod
|
||||
|
||||
spawned = {"count": 0}
|
||||
|
||||
def fake_pool(*args, **kwargs):
|
||||
spawned["count"] += 1
|
||||
raise AssertionError("ProcessPoolExecutor must not be constructed for 1 worker")
|
||||
|
||||
monkeypatch.setattr(concurrent.futures, "ProcessPoolExecutor", fake_pool)
|
||||
monkeypatch.setenv("GRAPHIFY_MAX_WORKERS", "1")
|
||||
|
||||
uncached = [(i, FIXTURES / "sample.py") for i in range(25)] # >= _PARALLEL_THRESHOLD
|
||||
per_file: list = [None] * len(uncached)
|
||||
|
||||
ok = extract_mod._extract_parallel(uncached, per_file, tmp_path, None, len(uncached))
|
||||
assert ok is False, "must hand the work back for sequential extraction"
|
||||
assert spawned["count"] == 0, "no pool may be spawned when max_workers resolves to 1"
|
||||
|
||||
|
||||
def test_extract_parallel_still_spawns_pool_for_multiple_workers(tmp_path, monkeypatch):
|
||||
"""Guard the #2173 skip: >1 worker must still take the pool path."""
|
||||
import concurrent.futures
|
||||
from graphify import extract as extract_mod
|
||||
|
||||
spawned = {"count": 0}
|
||||
|
||||
class FakePool:
|
||||
def __init__(self, *a, **kw):
|
||||
spawned["count"] += 1
|
||||
def __enter__(self):
|
||||
return self
|
||||
def __exit__(self, *a):
|
||||
return False
|
||||
def submit(self, *a, **kw):
|
||||
raise concurrent.futures.process.BrokenProcessPool("stop here")
|
||||
|
||||
monkeypatch.setattr(concurrent.futures, "ProcessPoolExecutor", FakePool)
|
||||
monkeypatch.setenv("GRAPHIFY_MAX_WORKERS", "4")
|
||||
|
||||
uncached = [(i, FIXTURES / "sample.py") for i in range(25)]
|
||||
per_file: list = [None] * len(uncached)
|
||||
|
||||
extract_mod._extract_parallel(uncached, per_file, tmp_path, None, len(uncached))
|
||||
assert spawned["count"] == 1, "multi-worker runs must still use the pool"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bash extractor tests (#866)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user