-
Notifications
You must be signed in to change notification settings - Fork 321
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
Merged
Merged
Changes from 1 commit
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
| 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 a virtual address window | ||
| sized from device memory (roughly 1x 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: | ||
|
|
||
| ```python | ||
| POOL_SIZE = 2097152 # 2 MiB | ||
|
juenglin marked this conversation as resolved.
Outdated
|
||
|
|
||
| mr = DeviceMemoryResource(dev, DeviceMemoryResourceOptions(max_size=POOL_SIZE)) | ||
| ``` | ||
|
|
||
| Use a larger value only if a test genuinely requires it. | ||
|
|
||
| ### 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. | ||
|
|
||
| ### Enforcement | ||
|
|
||
| `test_mempool_hygiene.py` statically scans this directory and fails on | ||
| `DeviceMemoryResourceOptions` / `PinnedMemoryResourceOptions` constructions | ||
| that omit `max_size`. 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,90 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Static guard against uncapped memory pools in the test suite. | ||
|
|
||
| A pool created without ``max_size`` reserves an address-space window sized from | ||
| device memory rather than from what the test allocates, and the whole suite | ||
| shares one process. Enough of those reservations exhaust the address space and | ||
| the rest of the session fails with ``CUDA_ERROR_OUT_OF_MEMORY`` on a device with | ||
| free physical memory (issue #2381). See AGENTS.md in this directory. | ||
|
|
||
| This check is static rather than runtime so that it also covers pools created | ||
| by tests that are skipped on the current platform. | ||
| """ | ||
|
|
||
| import ast | ||
| import pathlib | ||
|
|
||
| import pytest | ||
|
|
||
| TESTS_ROOT = pathlib.Path(__file__).parent | ||
|
|
||
| # 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: pathlib.Path) -> list[str]: | ||
| 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.relative_to(TESTS_ROOT).as_posix()}:{node.lineno}: {name} without max_size") | ||
| return found | ||
|
|
||
|
|
||
| @pytest.mark.agent_authored(model="claude-opus-5") | ||
| def test_no_uncapped_memory_pools(): | ||
| violations = sorted(v for path in TESTS_ROOT.rglob("*.py") for v in _violations_in(path)) | ||
| assert not violations, ( | ||
| "Memory pools created by tests must set max_size (use POOL_SIZE = 2097152).\n" | ||
| f"Annotate a deliberate exception with a '# {OPT_OUT_MARKER}: <reason>' comment.\n" | ||
| "See cuda_core/tests/AGENTS.md.\n" + "\n".join(violations) | ||
| ) |
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
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.