-
Notifications
You must be signed in to change notification settings - Fork 317
Add check and agent guidance about uncapped mempools #2514
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
Open
juenglin
wants to merge
2
commits into
NVIDIA:main
Choose a base branch
from
juenglin:more-oom-mitigations
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Check that tests do not create uncapped CUDA memory pools. | ||
|
|
||
| A pool created without ``max_size`` reserves an address-space window sized from | ||
| installed device memory rather than from what the test allocates, and the whole | ||
| cuda_core suite shares one process. Enough of those reservations exhaust the | ||
| address space, after which the rest of the session fails with | ||
| CUDA_ERROR_OUT_OF_MEMORY on a device with free physical memory. | ||
|
|
||
| See cuda_core/tests/AGENTS.md for the rule this enforces. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import ast | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[2] | ||
| DEFAULT_TREE = ROOT / "cuda_core" / "tests" | ||
|
|
||
| # Managed pools cannot be right-sized: cuMemPoolCreate requires maxSize == 0 for | ||
| # managed pools, so ManagedMemoryResourceOptions has no max_size to set. | ||
| CAPPABLE_OPTIONS = frozenset({"DeviceMemoryResourceOptions", "PinnedMemoryResourceOptions"}) | ||
| CAPPABLE_RESOURCES = frozenset({"DeviceMemoryResource", "PinnedMemoryResource"}) | ||
|
|
||
| OPT_OUT_MARKER = "uncapped-pool-ok" | ||
|
|
||
|
|
||
| def _callee_name(node: ast.Call) -> str: | ||
| func = node.func | ||
| if isinstance(func, ast.Attribute): | ||
| return func.attr | ||
| if isinstance(func, ast.Name): | ||
| return func.id | ||
| return "" | ||
|
|
||
|
|
||
| def _is_capped(node: ast.Call) -> bool: | ||
| # ``**kwargs`` (arg is None) may carry max_size; do not guess. | ||
| return any(kw.arg is None or kw.arg == "max_size" for kw in node.keywords) | ||
|
|
||
|
|
||
| def _dict_is_capped(node: ast.Dict) -> bool: | ||
| for key in node.keys: | ||
| if key is None: # ``**other`` inside the literal | ||
| return True | ||
| if isinstance(key, ast.Constant) and key.value == "max_size": | ||
| return True | ||
| return False | ||
|
|
||
|
|
||
| def _opted_out(lines: list[str], node: ast.AST) -> bool: | ||
| """True if the call, or the line above it, carries the opt-out marker.""" | ||
| start = max(node.lineno - 2, 0) # -1 for 0-based, -1 more for a preceding comment | ||
| end = getattr(node, "end_lineno", node.lineno) | ||
| return any(OPT_OUT_MARKER in line for line in lines[start:end]) | ||
|
|
||
|
|
||
| def violations_in(path: Path) -> list[str]: | ||
| """Return one message per uncapped pool construction in ``path``.""" | ||
| source = path.read_text(encoding="utf-8") | ||
| lines = source.splitlines() | ||
| found = [] | ||
| for node in ast.walk(ast.parse(source, filename=str(path))): | ||
| if not isinstance(node, ast.Call): | ||
| continue | ||
| name = _callee_name(node) | ||
| if name in CAPPABLE_OPTIONS: | ||
| uncapped = not _is_capped(node) | ||
| elif name in CAPPABLE_RESOURCES: | ||
| # The options may also be given as a dict literal. | ||
| dicts = [arg for arg in [*node.args, *(kw.value for kw in node.keywords)] if isinstance(arg, ast.Dict)] | ||
| uncapped = any(not _dict_is_capped(d) for d in dicts) | ||
| else: | ||
| continue | ||
| if uncapped and not _opted_out(lines, node): | ||
| found.append(f"{path.as_posix()}:{node.lineno}: {name} without max_size") | ||
| return found | ||
|
|
||
|
|
||
| def main(argv: list[str] | None = None) -> int: | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument( | ||
| "paths", | ||
| nargs="*", | ||
| type=Path, | ||
| help=f"Files to check. Defaults to every .py under {DEFAULT_TREE.relative_to(ROOT).as_posix()}.", | ||
| ) | ||
| args = parser.parse_args(argv) | ||
|
|
||
| paths = args.paths or sorted(DEFAULT_TREE.rglob("*.py")) | ||
| violations = sorted(v for path in paths if path.suffix == ".py" for v in violations_in(path)) | ||
| if not violations: | ||
| return 0 | ||
|
|
||
| print("error: memory pools created by tests must set max_size:", file=sys.stderr) | ||
| for violation in violations: | ||
| print(f" - {violation}", file=sys.stderr) | ||
| print( | ||
| f"Use the suite-wide POOL_SIZE from cuda_core/tests/helpers/constants.py, or annotate a\n" | ||
| f"deliberate exception with a '# {OPT_OUT_MARKER}: <reason>' comment.\n" | ||
| f"See cuda_core/tests/AGENTS.md.", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) | ||
| from check_mempool_hygiene import DEFAULT_TREE, main, violations_in | ||
|
|
||
|
|
||
| def write(tmp_path, source): | ||
| path = tmp_path / "test_sample.py" | ||
| path.write_text(source, encoding="utf-8") | ||
| return path | ||
|
|
||
|
|
||
| UNCAPPED = [ | ||
| pytest.param("DeviceMemoryResource(dev, DeviceMemoryResourceOptions(ipc_enabled=True))", id="options-kwarg"), | ||
| pytest.param("PinnedMemoryResource(PinnedMemoryResourceOptions())", id="options-empty"), | ||
| pytest.param('DeviceMemoryResource(dev, {"ipc_enabled": True})', id="options-dict"), | ||
| ] | ||
|
|
||
| CAPPED = [ | ||
| pytest.param("DeviceMemoryResource(dev, DeviceMemoryResourceOptions(max_size=POOL_SIZE))", id="capped-kwarg"), | ||
| pytest.param('DeviceMemoryResource(dev, {"max_size": POOL_SIZE})', id="capped-dict"), | ||
| pytest.param("DeviceMemoryResource(dev, DeviceMemoryResourceOptions(**opts))", id="opaque-kwargs"), | ||
| # No options at all wraps the device's default pool and reserves nothing, so | ||
| # capping it would convert a free wrapper into a new pool. | ||
| pytest.param("DeviceMemoryResource(dev)", id="default-pool-wrapper"), | ||
| # cuMemPoolCreate requires maxSize == 0 for managed pools, so these have no | ||
| # max_size to set. | ||
| pytest.param("ManagedMemoryResource(ManagedMemoryResourceOptions(preferred_location=0))", id="managed-exempt"), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| @pytest.mark.parametrize("source", UNCAPPED) | ||
| def test_uncapped_pool_is_reported(tmp_path, source): | ||
| assert violations_in(write(tmp_path, source)) | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| @pytest.mark.parametrize("source", CAPPED) | ||
| def test_acceptable_construction_is_not_reported(tmp_path, source): | ||
| assert violations_in(write(tmp_path, source)) == [] | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| @pytest.mark.parametrize("comment_line", [0, 1], ids=["marker-above", "marker-inline"]) | ||
| def test_marker_opts_a_call_out(tmp_path, comment_line): | ||
| # The escape hatch exists mainly for pytest.raises cases, where validation | ||
| # rejects the arguments before any pool is created. | ||
| call = "PinnedMemoryResource(PinnedMemoryResourceOptions())" | ||
| marker = "# uncapped-pool-ok: raises before the pool is created" | ||
| source = f"{marker}\n{call}" if comment_line == 0 else f"{call} {marker}" | ||
|
|
||
| assert violations_in(write(tmp_path, source)) == [] | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_reported_message_names_file_line_and_symbol(tmp_path): | ||
| path = write(tmp_path, "x = 1\nDeviceMemoryResource(dev, DeviceMemoryResourceOptions())\n") | ||
|
|
||
| (violation,) = violations_in(path) | ||
|
|
||
| assert violation.startswith(path.as_posix()) | ||
| assert ":2:" in violation | ||
| assert "DeviceMemoryResourceOptions without max_size" in violation | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_main_reports_failure_for_the_files_it_is_given(tmp_path, capsys): | ||
| path = write(tmp_path, "DeviceMemoryResource(dev, DeviceMemoryResourceOptions())") | ||
|
|
||
| assert main([str(path)]) == 1 | ||
| assert "must set max_size" in capsys.readouterr().err | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_main_ignores_non_python_files(tmp_path): | ||
| unrelated = tmp_path / "notes.txt" | ||
| unrelated.write_text("DeviceMemoryResourceOptions()", encoding="utf-8") | ||
|
|
||
| assert main([str(unrelated)]) == 0 | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_the_live_test_suite_is_clean(): | ||
| # Without a default the hook would only ever see changed files, so a | ||
| # violation could ride in on a rename or a merge. | ||
| assert DEFAULT_TREE.is_dir() | ||
| assert main([]) == 0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # cuda.core test suite | ||
|
|
||
| Package-wide conventions live in `../AGENTS.md`; repository-wide ones in | ||
| `../../AGENTS.md`. This file covers conventions specific to the tests. | ||
|
|
||
| ## Never create an uncapped memory pool | ||
|
|
||
| A memory pool created without `max_size` reserves virtual address space similar | ||
| in size to the installed physical device memory regardless of what the test | ||
| actually allocates. The reservation is charged to the process address space | ||
| even though it is not backed by physical memory, and it is not returned until | ||
| the pool is destroyed *and* the stream-ordered frees of its outstanding | ||
| allocations retire. The whole suite shares one process and one device, so these | ||
| reservations accumulate across tests. | ||
|
|
||
| When a test needs its own pool, use the suite-wide cap from | ||
| `helpers/constants.py`: | ||
|
|
||
| ```python | ||
| from helpers.constants import POOL_SIZE | ||
|
|
||
| mr = DeviceMemoryResource(dev, DeviceMemoryResourceOptions(max_size=POOL_SIZE)) | ||
| ``` | ||
|
|
||
| Use a larger value only if a test genuinely requires it, and prefer adding a | ||
| shared constant to `helpers/constants.py` over redefining one per module. | ||
|
|
||
| ### Passing no options is different from passing empty options | ||
|
|
||
| `DeviceMemoryResource(dev)` with no options does **not** create a pool. It | ||
| wraps the device's existing default mempool (`_mempool_owned` is false) and | ||
| costs no additional address space. Passing *any* options object creates a new | ||
| owned pool, and a new pool without `max_size` is uncapped: | ||
|
|
||
| ```python | ||
| DeviceMemoryResource(dev) # wraps default pool, free | ||
| DeviceMemoryResource(dev, DeviceMemoryResourceOptions()) # NEW uncapped pool, expensive | ||
| DeviceMemoryResource(dev, {"ipc_enabled": True}) # NEW uncapped pool, expensive | ||
| ``` | ||
|
|
||
| Do not add `max_size` to a call that currently passes no options: that | ||
| converts a free default-pool wrapper into a new pool and makes things worse. | ||
|
|
||
| ### Managed pools are exempt | ||
|
|
||
| `cuMemPoolCreate` requires `CUmemPoolProps.maxSize` to be zero for managed | ||
| pools, so `ManagedMemoryResourceOptions` has no `max_size` option. Managed | ||
| pools cannot be right-sized and are not checked. | ||
|
|
||
| ### Document exemptions | ||
|
|
||
| When a call is deliberately exempt -- most often because it sits inside | ||
| `pytest.raises` and no pool is ever created -- annotate it: | ||
|
|
||
| ```python | ||
| with pytest.raises(RuntimeError, match="IPC is not available"): | ||
| # uncapped-pool-ok: raises before the pool is created | ||
| DeviceMemoryResource(mempool_device, DeviceMemoryResourceOptions(ipc_enabled=True)) | ||
| ``` | ||
|
|
||
| ## Release resources at test boundaries | ||
|
|
||
| The `_init_cuda_context` fixture in `conftest.py` runs `gc.collect()` followed | ||
| by `cuCtxSynchronize()` before popping the context. Tests should not rely on | ||
| that as a substitute for cleaning up explicitly: prefer context managers for | ||
| resources whose lifetime fits a single scope, and keep pool lifetimes inside | ||
| the test that creates them. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Constants shared across the cuda_core test suite.""" | ||
|
|
||
| # Cap for memory pools created by tests. A pool created without an explicit | ||
| # max_size instead reserves a system-dependent window that scales with | ||
| # installed device memory -- hundreds of GiB on large-memory GPUs. The | ||
| # per-process virtual address budget is bounded (~1 TB on Windows MCDM), and a | ||
| # reservation is not returned until the pool is torn down and its | ||
| # stream-ordered frees retire, so oversized windows accumulate across a session | ||
| # and eventually starve later pool creations with CUDA_ERROR_OUT_OF_MEMORY | ||
| # (issue #2381). See AGENTS.md in the tests directory. | ||
| POOL_SIZE = 2097152 # 2 MiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.