Skip to content

Commit 2dd9138

Browse files
committed
address review comments
1 parent bf712bf commit 2dd9138

File tree

6 files changed

+15
-13
lines changed

6 files changed

+15
-13
lines changed

cuda_core/cuda/core/experimental/_memory.pyx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from __future__ import annotations
66

77
cimport cpython
8+
from libc.limits cimport ULLONG_MAX
89
from libc.stdint cimport uintptr_t, intptr_t
910
from libc.string cimport memset, memcpy
1011

@@ -95,13 +96,15 @@ cdef class Buffer:
9596
The stream object to use for asynchronous deallocation. If None,
9697
the behavior depends on the underlying memory resource.
9798
"""
99+
cdef _cyMemoryResource cy_mr
98100
if self._ptr and self._mr is not None:
99101
if isinstance(self._mr, _cyMemoryResource):
100102
# FIXME
101103
if stream is None:
102104
stream = Stream.__new__(Stream)
103105
(<cyStream>(stream))._handle = <cydriver.CUstream>(0)
104-
(<_cyMemoryResource>(self._mr))._deallocate(self._ptr, self._size, <cyStream>stream)
106+
cy_mr = <_cyMemoryResource>(self._mr)
107+
cy_mr._deallocate(self._ptr, self._size, <cyStream>stream)
105108
else:
106109
self._mr.deallocate(self._ptr, self._size, stream)
107110
self._ptr = 0
@@ -123,7 +126,7 @@ cdef class Buffer:
123126
return self._ptr
124127
else:
125128
# contract: Buffer is closed
126-
return None
129+
return 0
127130

128131
@property
129132
def size(self) -> int:
@@ -673,7 +676,7 @@ cdef class DeviceMemoryResource(_cyMemoryResource, MemoryResource):
673676
DeviceMemoryResourceOptions, options, "DeviceMemoryResource options", keep_none=True
674677
)
675678
cdef cydriver.cuuint64_t current_threshold
676-
cdef cydriver.cuuint64_t max_threshold = 0xFFFFFFFFFFFFFFFF
679+
cdef cydriver.cuuint64_t max_threshold = ULLONG_MAX
677680
cdef cydriver.CUmemPoolProps properties
678681

679682
if opts is None:

cuda_core/cuda/core/experimental/_stream.pyx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ cdef class Stream:
113113
self._handle = <cydriver.CUstream>(NULL)
114114
self._owner = None
115115
self._builtin = False
116-
self._nonblocking = -1 # delayed
117-
self._priority = INT32_MIN # delayed
118-
self._device_id = cydriver.CU_DEVICE_INVALID # delayed
119-
self._ctx_handle = CU_CONTEXT_INVALID # delayed
116+
self._nonblocking = -1 # lazy init'd
117+
self._priority = INT32_MIN # lazy init'd
118+
self._device_id = cydriver.CU_DEVICE_INVALID # lazy init'd
119+
self._ctx_handle = CU_CONTEXT_INVALID # lazy init'd
120120

121121
def __init__(self, *args, **kwargs):
122122
raise RuntimeError(

cuda_core/docs/source/release/0.X.Y-notes.rst

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ Breaking Changes
2222
- **CUDA 11 support dropped**: CUDA 11 support is no longer tested and it may or may not work with cuda.bindings and CTK 11.x. Users are encouraged to migrate to CUDA 12.x or 13.x.
2323
- Support for ``cuda-bindings`` (and ``cuda-python``) < 12.6.2 is dropped. Internally, ``cuda.core`` now always requires the `new binding module layout <https://nvidia.github.io/cuda-python/cuda-bindings/latest/release/12.6.1-notes.html#cuda-namespace-cleanup-with-a-new-module-layout>`_. As per the ``cuda-bindings`` `support policy <https://nvidia.github.io/cuda-python/cuda-bindings/latest/support.html>`_), CUDA 12 users are encouraged to use the latest ``cuda-bindings`` 12.9.x, which is backward-compatible with all CUDA Toolkit 12.y.
2424
- **LaunchConfig grid parameter interpretation**: When :attr:`LaunchConfig.cluster` is specified, the :attr:`LaunchConfig.grid` parameter now correctly represents the number of clusters instead of blocks. Previously, the grid parameter was incorrectly interpreted as blocks, causing a mismatch with the expected C++ behavior. This change ensures that ``LaunchConfig(grid=4, cluster=2, block=32)`` correctly produces 4 clusters × 2 blocks/cluster = 8 total blocks, matching the C++ equivalent ``cudax::make_hierarchy(cudax::grid_dims(4), cudax::cluster_dims(2), cudax::block_dims(32))``.
25-
- When :class:`Buffer` is closed, :attr:`Buffer.handle` is now set to ``None``. It was previously set to ``0`` by accident.
2625

2726

2827
New features

cuda_core/examples/memory_ops.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@
128128
cp.cuda.Stream.null.use() # reset CuPy's current stream to the null stream
129129

130130
# Verify buffers are properly closed
131-
assert device_buffer.handle is None, "Device buffer should be closed"
132-
assert pinned_buffer.handle is None, "Pinned buffer should be closed"
133-
assert new_device_buffer.handle is None, "New device buffer should be closed"
131+
assert device_buffer.handle == 0, "Device buffer should be closed"
132+
assert pinned_buffer.handle == 0, "Pinned buffer should be closed"
133+
assert new_device_buffer.handle == 0, "New device buffer should be closed"
134134

135135
print("Memory management example completed!")

cuda_core/tests/test_launcher.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,4 +370,4 @@ def test_launch_with_buffers_allocated_by_memory_resource(init_cuda, memory_reso
370370
cp.cuda.Stream.null.use() # reset CuPy's current stream to the null stream
371371

372372
# Verify buffer is properly closed
373-
assert buffer.handle is None, f"{name} buffer should be closed"
373+
assert buffer.handle == 0, f"{name} buffer should be closed"

cuda_core/tests/test_memory.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ def test_buffer_copy_from():
223223
def buffer_close(dummy_mr: MemoryResource):
224224
buffer = dummy_mr.allocate(size=1024)
225225
buffer.close()
226-
assert buffer.handle is None
226+
assert buffer.handle == 0
227227
assert buffer.memory_resource is None
228228

229229

0 commit comments

Comments
 (0)