From 176f1be7ca23f7830448eb66ebd1c369e2a61af2 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Sat, 25 Jul 2026 20:39:47 +0530 Subject: [PATCH] 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. --- graphify/extract.py | 9 +++++++ tests/test_extract.py | 57 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/graphify/extract.py b/graphify/extract.py index bbfa301cc..522403bde 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -4320,6 +4320,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) diff --git a/tests/test_extract.py b/tests/test_extract.py index be633c0fd..64b922f66 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -1488,6 +1488,63 @@ def submit(self, *a, **kw): 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) # ---------------------------------------------------------------------------