-
Notifications
You must be signed in to change notification settings - Fork 274
build: default to a parallel extension build in setup.py #565
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,12 @@ def build_extension(self, ext: CMakeExtension) -> None: | |
| if hasattr(self, "parallel") and self.parallel: | ||
| # CMake 3.12+ only. | ||
| build_args += [f"-j{self.parallel}"] | ||
| else: | ||
| # Without an explicit -j, `cmake --build` compiles one TU at a | ||
| # time, so pip installs built the extension serially. Cap at 8: | ||
| # the extension has ~6 TUs, each peaking at 1-2 GB of compiler | ||
| # memory, so higher values buy nothing and only add pressure. | ||
| build_args += [f"-j{min(os.cpu_count() or 1, 8)}"] | ||
|
Comment on lines
+96
to
+101
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
if rg -n 'if hasattr\(self, "parallel"\) and self\.parallel:' setup.py; then
echo "The truthiness check still treats explicit -j 0 as unset." >&2
exit 1
fi
rg -n --glob '*test*.py' 'parallel\s*=\s*0|build_ext.*-j.*0' . || {
echo "Add regression coverage for explicit -j 0." >&2
exit 1
}Repository: NVIDIA/cudnn-frontend Length of output: 280 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- setup.py relevant section ---'
sed -n '70,115p' setup.py
printf '%s\n' '--- project configuration ---'
for f in pyproject.toml setup.cfg; do
if [ -f "$f" ]; then
echo "[$f]"
sed -n '1,180p' "$f"
fi
done
printf '%s\n' '--- parallel-related code and tests ---'
rg -n --glob '*.py' --glob '*.toml' --glob '*.cfg' \
'self\.parallel|parallel\s*=|build_ext|cmake --build|CMAKE_BUILD_PARALLEL_LEVEL|-j[[:space:]]*0' .
printf '%s\n' '--- test files ---'
git ls-files '*test*.py' '*tests*' | sed -n '1,160p'Repository: NVIDIA/cudnn-frontend Length of output: 15213 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import inspect
from setuptools.command.build_ext import build_ext
print("--- setuptools build_ext option definition ---")
print(inspect.getsource(build_ext.user_options))
print("--- setuptools build_ext parallel handling ---")
source = inspect.getsource(build_ext)
for i, line in enumerate(source.splitlines(), 1):
if "parallel" in line or "ThreadPool" in line or "build_extensions" in line:
start = max(1, i - 4)
end = min(len(source.splitlines()), i + 8)
print(f"\n[source lines {start}-{end}]")
print("\n".join(f"{n}: {source.splitlines()[n-1]}" for n in range(start, end + 1)))
PY
printf '%s\n' '--- installed tool versions ---'
cmake --version 2>/dev/null | head -n 1 || true
ninja --version 2>/dev/null || true
printf '%s\n' '--- setup.py generator and build command context ---'
sed -n '1,110p' setup.pyRepository: NVIDIA/cudnn-frontend Length of output: 282 🌐 Web query:
💡 Result: There is no official "0" flag or specific "0" semantic defined for the cmake --build command [1][2][3]. When invoking cmake --build, the command acts as a wrapper that abstracts the native build tool's interface [1][4]. If you are seeing a reference to 0 in this context, it is likely one of the following: 1. Return Code: Upon regular successful termination, the cmake executable (and the underlying build tool) returns the exit code 0 [1][5]. 2. Positional Argument Confusion: The cmake --build command requires the binary directory as the first argument [1][2]. If a script or user provides 0 in that position, it may be interpreted as a literal directory name named "0" or result in an error if that directory does not exist. 3. Build Tool Misinterpretation: Some users may confuse shell return values with command-line arguments. To pass specific options to the Ninja build tool through CMake, you must use the double-dash (--) separator [1][4][3]. Any options placed after the -- are passed directly to the underlying generator (e.g., Ninja) [1][3]. For example: cmake --build. -- -v This command instructs CMake to build the project in the current directory and passes the -v (verbose) flag directly to the Ninja generator [6]. There is no official command-line option "0" for Ninja or CMake build operations [1][4]. Citations:
🌐 Web query:
💡 Result: The command 'setuptools build_ext --parallel 0 build_extensions source' contains a misunderstanding of how the 'build_ext' command works in 'setuptools'. 1. Command Structure: The 'build_ext' command accepts a '--parallel' (or '-j') option to specify the number of parallel jobs for building C/C++ extensions [1][2][3]. It does not take a 'source' argument at the end, nor does it recognize a 'build_extensions' argument in that position [4][3]. 2. Parallel Flag Behavior: The '--parallel' option expects an integer representing the number of parallel workers [1][3]. If set to '0', it typically does not disable parallelism; rather, the underlying logic often defaults to serial execution if the value is not a positive integer or if it falls back to a serial implementation [4][1][3]. If you want to enable parallel building, you should pass a positive integer (e.g., --parallel 4) or omit the number to let 'setuptools' attempt to use the number of available CPUs [4][1][3]. 3. How Parallelization Works: 'setuptools' parallelizes the build process at the extension level, meaning it builds separate extension modules concurrently, not individual source files within a single extension (unlike a Makefile) [5]. If you are trying to build extensions during a package installation (e.g., via 'pip'), you cannot always pass flags directly to 'build_ext' [6][7]. Instead, common workarounds include: - Setting the 'DIST_EXTRA_CONFIG' environment variable pointing to a file containing '[build_ext] parallel = 8' [6][7]. - Using environment variables if supported by specific configurations [6][2]. - Using 'setup.cfg' to permanently set 'parallel = N' under the '[build_ext]' section [6]. In summary, 'build_ext --parallel [N]' is the correct syntax for direct 'setup.py' calls [7][2], but 'source' and 'build_extensions' are not valid arguments for this command [4][3]. Citations:
🏁 Script executed: #!/usr/bin/env bash
set -u
printf '%s\n' '--- installed tool versions ---'
cmake --version 2>/dev/null | head -n 1 || true
ninja --version 2>/dev/null || true
printf '%s\n' '--- setuptools source references ---'
curl -fsSL \
https://raw.githubusercontent.com/pypa/setuptools/v64.0.0/setuptools/_distutils/command/build_ext.py \
| grep -n -A18 -B8 -E 'parallel|build_extensions' || true
printf '%s\n' '--- CMake documentation references ---'
curl -fsSL \
https://raw.githubusercontent.com/Kitware/CMake/master/Help/manual/cmake.1.rst \
| grep -n -A12 -B8 -E 'parallel|--parallel|-j' | head -n 100 || trueRepository: NVIDIA/cudnn-frontend Length of output: 10564 🌐 Web query:
💡 Result: The --parallel option (also represented as -j) for the cmake --build command is used to specify the maximum number of concurrent processes for the build [1]. According to the official documentation, if the argument for the number of jobs is omitted, the native build tool uses its own default [1]. While the documentation notes that a value of 1 can be used to limit the build to a single job, it does not explicitly define the behavior of passing 0 as an argument [1]. Typically, setting a parallel job count to 0 in build systems can result in an error, be ignored, or default to a specific behavior like using the number of available CPU cores, but this specific behavior is not documented for the CMake build command [1]. Citations: 🌐 Web query:
💡 Result: In the Ninja build system, you do not need to explicitly specify a number of parallel jobs to achieve parallel execution. Ninja is designed to run builds in parallel by default, automatically determining the number of concurrent jobs based on the number of CPUs available on your system [1][2]. If you wish to override this default behavior and specify a fixed number of parallel jobs, you can use the -j flag [1][2]. For example, running the command ninja -j 20 will instruct Ninja to execute up to 20 build commands in parallel [1][2]. Additionally, Ninja supports the GNU Make jobserver client protocol (as of version 1.13), which allows it to coordinate parallelism with other build systems or jobserver pools [1][2]. Regardless of any custom pool configurations defined within your build files, Ninja will never exceed the parallelism level set by the default auto-detection or the explicit limit provided by the -j flag [1][2]. Citations: 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
source = Path("setup.py").read_text()
assert 'if hasattr(self, "parallel") and self.parallel:' in source
assert 'build_args += [f"-j{min(os.cpu_count() or 1, 8)}"]' in source
assert 'cmake_args += ["-GNinja"' in source
assert 'ninja==1.11.1.1' in Path("pyproject.toml").read_text()
def current_build_args(parallel, cpu_count=16):
if parallel:
return [f"-j{parallel}"]
return [f"-j{min(cpu_count or 1, 8)}"]
def proposed_build_args(parallel, cpu_count=16):
if parallel is not None:
return [f"-j{parallel}"] if parallel else []
return [f"-j{min(cpu_count or 1, 8)}"]
print("current self.parallel=0:", current_build_args(0))
print("proposed elif ... is None, self.parallel=0:", proposed_build_args(0))
print("serial CMake setting documented by CMake: -j1")
print("current self.parallel=None:", current_build_args(None))
print("proposed self.parallel=None:", proposed_build_args(None))
test_files = [
p for p in Path(".").rglob("*.py")
if "test" in p.name.lower() or "tests" in p.parts
]
matches = []
for path in test_files:
text = path.read_text(errors="replace")
if "parallel" in text or "build_ext" in text:
matches.append(str(path))
print("test files mentioning parallel/build_ext:", matches)
PYRepository: NVIDIA/cudnn-frontend Length of output: 247 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
setup = Path("setup.py").read_text()
pyproject = Path("pyproject.toml").read_text()
assert 'if hasattr(self, "parallel") and self.parallel:' in setup
assert 'build_args += [f"-j{min(os.cpu_count() or 1, 8)}"]' in setup
assert '"-GNinja"' in setup
assert "ninja==1.11.1.1" in pyproject
def current_build_args(parallel, cpu_count=16):
if parallel:
return [f"-j{parallel}"]
return [f"-j{min(cpu_count or 1, 8)}"]
def proposed_build_args(parallel, cpu_count=16):
if parallel is not None:
return [f"-j{parallel}"] if parallel else []
return [f"-j{min(cpu_count or 1, 8)}"]
print("current self.parallel=0:", current_build_args(0))
print("proposed elif ... is None, self.parallel=0:", proposed_build_args(0))
print("serial CMake setting: -j1")
print("current self.parallel=None:", current_build_args(None))
print("proposed self.parallel=None:", proposed_build_args(None))
matches = []
for path in Path(".").rglob("*.py"):
if "test" not in path.name.lower() and "tests" not in path.parts:
continue
text = path.read_text(errors="replace")
if "parallel" in text or "build_ext" in text:
matches.append(str(path))
print("test files mentioning parallel/build_ext:", matches)
PYRepository: NVIDIA/cudnn-frontend Length of output: 426 Preserve serial behavior for explicit
🤖 Prompt for AI Agents |
||
|
|
||
| build_temp = Path(self.build_temp) / ext.name | ||
| if not build_temp.exists(): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: NVIDIA/cudnn-frontend
Length of output: 2894
🏁 Script executed:
Repository: NVIDIA/cudnn-frontend
Length of output: 12526
🏁 Script executed:
Repository: NVIDIA/cudnn-frontend
Length of output: 1023
🌐 Web query:
CMake cmake --build omitted --parallel default native build tool Ninja default parallelism documentation💡 Result:
When using the cmake --build command, omitting the --parallel (or -j) flag instructs CMake to delegate parallelization decisions to the underlying native build tool [1][2]. The build tool then uses its own internal default behavior for determining the number of concurrent jobs [1][3]. In the case of the Ninja generator, Ninja defaults to running jobs in parallel based on the number of available CPU cores [4]. Specifically, Ninja historically defaults to N+2 parallel jobs, where N is the number of available processors [4]. If you wish to override this default behavior without relying on the native build tool's internal logic, you have two primary options: 1. Command-line flag: You can use the --parallel (or -j ) option with cmake --build to explicitly set the maximum number of concurrent processes [1][5]. 2. Environment variable: You can set the CMAKE_BUILD_PARALLEL_LEVEL environment variable [1][6]. If this variable is defined, CMake uses its value to specify the parallel level for the underlying build tool when the --parallel flag is not provided on the command line [6][3]. If the variable is defined but empty, CMake reverts to using the native build tool's default [6]. It is important to note that cmake --build is designed to be a cross-platform wrapper; when you omit --parallel, you are essentially telling CMake not to interfere with the build tool's default parallelism [1][3]. While Ninja's default is typically to use all available cores (plus extra for I/O waiting), other tools (like legacy Makefiles) may default to a single job unless configured otherwise, which is why the --parallel flag or CMAKE_BUILD_PARALLEL_LEVEL variable is often used to ensure consistent, predictable parallel performance across different environments [4][7].
Citations:
🏁 Script executed:
Repository: NVIDIA/cudnn-frontend
Length of output: 1073
Correct the Ninja concurrency rationale.
CMake delegates omitted
-jto the native build tool, and Ninja runs jobs in parallel by default. This fallback explicitly passes-jmin(os.cpu_count() or 1, 8)whenCMAKE_BUILD_PARALLEL_LEVELandself.parallelare unset. Update the comment and add focused coverage for this concurrency cap.🤖 Prompt for AI Agents