Fix #2049: Expose Buffer._size to Python#2068
Conversation
github-actions
Bot
added
the
cuda.core
|
| cdef public: | ||
| # Python code in _memory/_virtual_memory_resource.py needs to update | ||
| # this value, though it is technically private. | ||
| size_t _size |
There was a problem hiding this comment.
This seems odd. Didn't we already expose Buffer.size?
https://nvidia.github.io/cuda-python/cuda-core/latest/generated/cuda.core.Buffer.html#cuda.core.Buffer.size
There was a problem hiding this comment.
We could make that property settable (it's currently readonly), but I assumed we didn't want to make settabilility part of the public API -- seems kinda dangerous. So I exposed _size as a _-prefixed member instead.
|
Do we know why this did not trip in the CI? #2049 (comment) |
My agent's assessment: vmm_mr.allocate(2 * 1024 * 1024) reserves only that 2 MB range; nothing guarantees the next VA page is unreserved. In practice the CUDA driver's cuMemAddressReserve with a fixedAddr hint typically returns CUDA_SUCCESS but with a different address (the hint is advisory). That trips the new_ptr != int(buf.handle) + aligned_prev_size branch, the reservation is freed, and execution falls through to _grow_allocation_slow_path. The test author already noticed this — the comment at test_memory.py:955-956 literally says "Because of the slow path, the pointer may change … we can assert that a new pointer was assigned". So the test was written knowing only the slow path runs. |
Andy-Jost
left a comment
Andy-Jost
left a comment
There was a problem hiding this comment.
LGTM!
I don't particularly like the VVM interface forcing Buffer to be mutable but that's not in scope for this PR.
Should we create a new issue to follow-up for that? I'm not as familiar with this code so I don't know what that would look like... |
3 participants
_grow_allocation_fast_pathwas not being tested in CI, and static type-checking discovered an attribute error lurking within it.This new test is entirely agent-written and I don't personally understand this part of the system, so please give it extra scrutiny.
The bugfix itself is simple, but also could be removed when/if
_virtual_memory_resource.pyis Cythonized, since it couldcimportBuffer and get access to_sizethat way.