Change _Memory object's memory ownership model

The _Memory object acquires additional field void * _opaque_ptr.
This pointer is the pointer to std::shared_ptr<void> with a deleter
class to delete USM allocations. The pointer is opaque to retain C-API
of Py_MemoryObject.

If _opaque_ptr is NULL, refobj field is assumed to be responsible for
USM deallocation.

api Memory_GetOpaquePointer is added to access the opaque pointer.

A test in test_sycl_usm is modified to reflect factual changes in
the behavior of _Memory type.

Author: oleksandr-pavlyk

dpctl/_sycl_queue.pyx

@@ -342,7 +342,7 @@ cdef DPCTLSyclEventRef _memcpy_impl(
     cdef unsigned char[::1] dst_host_buf = None
 
     if isinstance(src, _Memory):
-        c_src_ptr = <void*>(<_Memory>src).memory_ptr
+        c_src_ptr = <void*>(<_Memory>src).get_data_ptr()
     elif _is_buffer(src):
         src_host_buf = src
         c_src_ptr = <void *>&src_host_buf[0]
@@ -354,7 +354,7 @@ cdef DPCTLSyclEventRef _memcpy_impl(
        )
 
     if isinstance(dst, _Memory):
-        c_dst_ptr = <void*>(<_Memory>dst).memory_ptr
+        c_dst_ptr = <void*>(<_Memory>dst).get_data_ptr()
     elif _is_buffer(dst):
         dst_host_buf = dst
         c_dst_ptr = <void *>&dst_host_buf[0]
@@ -1265,7 +1265,7 @@ cdef class SyclQueue(_SyclQueue):
         cdef DPCTLSyclEventRef ERef = NULL
 
         if isinstance(mem, _Memory):
-            ptr = <void*>(<_Memory>mem).memory_ptr
+            ptr = <void*>(<_Memory>mem).get_data_ptr()
         else:
             raise TypeError("Parameter `mem` should have type _Memory")
 
@@ -1285,7 +1285,7 @@ cdef class SyclQueue(_SyclQueue):
         cdef DPCTLSyclEventRef ERef = NULL
 
         if isinstance(mem, _Memory):
-            ptr = <void*>(<_Memory>mem).memory_ptr
+            ptr = <void*>(<_Memory>mem).get_data_ptr()
         else:
             raise TypeError("Parameter `mem` should have type _Memory")
 

dpctl/memory/CMakeLists.txt

@@ -1,7 +1,7 @@
 
-file(GLOB _cython_sources *.pyx)
-foreach(_cy_file ${_cython_sources})
-    get_filename_component(_trgt ${_cy_file} NAME_WLE)
-    build_dpctl_ext(${_trgt} ${_cy_file} "dpctl/memory")
-    target_link_libraries(DpctlCAPI INTERFACE ${_trgt}_headers)
-endforeach()
+set(_cy_file ${CMAKE_CURRENT_SOURCE_DIR}/_memory.pyx)
+get_filename_component(_trgt ${_cy_file} NAME_WLE)
+build_dpctl_ext(${_trgt} ${_cy_file} "dpctl/memory" SYCL)
+# _memory include _opaque_smart_ptr.hpp
+target_include_directories(${_trgt} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+target_link_libraries(DpctlCAPI INTERFACE ${_trgt}_headers)

dpctl/memory/_memory.pxd

@@ -33,7 +33,8 @@ cdef DPCTLSyclQueueRef get_queue_ref_from_ptr_and_syclobj(
 
 
 cdef public api class _Memory [object Py_MemoryObject, type Py_MemoryType]:
-    cdef DPCTLSyclUSMRef memory_ptr
+    cdef DPCTLSyclUSMRef _memory_ptr
+    cdef void* _opaque_ptr
     cdef Py_ssize_t nbytes
     cdef SyclQueue queue
     cdef object refobj
@@ -50,6 +51,8 @@ cdef public api class _Memory [object Py_MemoryObject, type Py_MemoryType]:
     cpdef memset(self, unsigned short val=*)
 
     cpdef bytes tobytes(self)
+    cdef DPCTLSyclUSMRef get_data_ptr(self)
+    cdef void * get_opaque_ptr(self)
 
     @staticmethod
     cdef SyclDevice get_pointer_device(

dpctl/memory/_memory.pyx

@@ -78,6 +78,12 @@ __all__ = [
 
 include "_sycl_usm_array_interface_utils.pxi"
 
+cdef extern from "_opaque_smart_ptr.hpp":
+    void * OpaqueSmartPtr_Make(void *, DPCTLSyclQueueRef) nogil
+    void * OpaqueSmartPtr_Copy(void *) nogil
+    void OpaqueSmartPtr_Delete(void *) nogil
+    void * OpaqueSmartPtr_Get(void *) nogil
+
 class USMAllocationError(Exception):
     """
     An exception raised when Universal Shared Memory (USM) allocation
@@ -152,7 +158,8 @@ cdef class _Memory:
         MemoryUSMShared, MemoryUSMDevice, MemoryUSMHost
     """
     cdef _cinit_empty(self):
-        self.memory_ptr = NULL
+        self._memory_ptr = NULL
+        self._opaque_ptr = NULL
         self.nbytes = 0
         self.queue = None
         self.refobj = None
@@ -198,7 +205,8 @@ cdef class _Memory:
                 )
 
             if (p):
-                self.memory_ptr = p
+                self._memory_ptr = p
+                self._opaque_ptr = OpaqueSmartPtr_Make(p, QRef)
                 self.nbytes = nbytes
                 self.queue = queue
             else:
@@ -214,18 +222,22 @@ cdef class _Memory:
         cdef _Memory other_mem
         if isinstance(other, _Memory):
             other_mem = <_Memory> other
-            self.memory_ptr = other_mem.memory_ptr
             self.nbytes = other_mem.nbytes
             self.queue = other_mem.queue
-            if other_mem.refobj is None:
-                self.refobj = other
+            if other_mem._opaque_ptr is NULL:
+                self._memory_ptr = other_mem._memory_ptr
+                self._opaque_ptr = NULL
+                self.refobj = other.reference_obj
             else:
-                self.refobj = other_mem.refobj
+                self._memory_ptr = other_mem._memory_ptr
+                self._opaque_ptr = OpaqueSmartPtr_Copy(other_mem._opaque_ptr)
+                self.refobj = None
         elif hasattr(other, '__sycl_usm_array_interface__'):
             other_iface = other.__sycl_usm_array_interface__
             if isinstance(other_iface, dict):
                 other_buf = _USMBufferData.from_sycl_usm_ary_iface(other_iface)
-                self.memory_ptr = other_buf.p
+                self._opaque_ptr = NULL
+                self._memory_ptr = <DPCTLSyclUSMRef>other_buf.p
                 self.nbytes = other_buf.nbytes
                 self.queue = other_buf.queue
                 self.refobj = other
@@ -241,23 +253,25 @@ cdef class _Memory:
             )
 
     def __dealloc__(self):
-        if (self.refobj is None):
-            if self.memory_ptr:
-                if (type(self.queue) is SyclQueue):
-                    DPCTLfree_with_queue(
-                        self.memory_ptr, self.queue.get_queue_ref()
-                    )
+        if not (self._opaque_ptr is NULL):
+            OpaqueSmartPtr_Delete(self._opaque_ptr)
         self._cinit_empty()
 
+    cdef DPCTLSyclUSMRef get_data_ptr(self):
+        return self._memory_ptr
+
+    cdef void* get_opaque_ptr(self):
+        return self._opaque_ptr
+
     cdef _getbuffer(self, Py_buffer *buffer, int flags):
         # memory_ptr is Ref which is pointer to SYCL type. For USM it is void*.
         cdef SyclContext ctx = self._context
         cdef _usm_type UsmTy = DPCTLUSM_GetPointerType(
-            self.memory_ptr, ctx.get_context_ref()
+            self._memory_ptr, ctx.get_context_ref()
         )
         if UsmTy == _usm_type._USM_DEVICE:
             raise ValueError("USM Device memory is not host accessible")
-        buffer.buf = <char*>self.memory_ptr
+        buffer.buf = <void *>self._memory_ptr
         buffer.format = 'B'                     # byte
         buffer.internal = NULL                  # see References
         buffer.itemsize = 1
@@ -285,7 +299,7 @@ cdef class _Memory:
         represented as Python integer.
         """
         def __get__(self):
-            return <size_t>(self.memory_ptr)
+            return <size_t>(self._memory_ptr)
 
     property _context:
         """:class:`dpctl.SyclContext` the USM pointer is bound to. """
@@ -333,7 +347,7 @@ cdef class _Memory:
             .format(
                 self.get_usm_type(),
                 self.nbytes,
-                hex(<object>(<size_t>self.memory_ptr))
+                hex(<object>(<size_t>self._memory_ptr))
             )
         )
 
@@ -377,7 +391,7 @@ cdef class _Memory:
         """
         def __get__(self):
             cdef dict iface = {
-                "data": (<size_t>(<void *>self.memory_ptr),
+                "data": (<size_t>(<void *>self._memory_ptr),
                          True),  # bool(self.writable)),
                 "shape": (self.nbytes,),
                 "strides": None,
@@ -402,18 +416,18 @@ cdef class _Memory:
         if syclobj is None:
             ctx = self._context
             return _Memory.get_pointer_type(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             ).decode("UTF-8")
         elif isinstance(syclobj, SyclContext):
             ctx = <SyclContext>(syclobj)
             return _Memory.get_pointer_type(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             ).decode("UTF-8")
         elif isinstance(syclobj, SyclQueue):
             q = <SyclQueue>(syclobj)
             ctx = q.get_sycl_context()
             return _Memory.get_pointer_type(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             ).decode("UTF-8")
         raise TypeError(
             "syclobj keyword can be either None, or an instance of "
@@ -435,18 +449,18 @@ cdef class _Memory:
         if syclobj is None:
             ctx = self._context
             return _Memory.get_pointer_type_enum(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             )
         elif isinstance(syclobj, SyclContext):
             ctx = <SyclContext>(syclobj)
             return _Memory.get_pointer_type_enum(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             )
         elif isinstance(syclobj, SyclQueue):
             q = <SyclQueue>(syclobj)
             ctx = q.get_sycl_context()
             return _Memory.get_pointer_type_enum(
-                self.memory_ptr, ctx
+                self._memory_ptr, ctx
             )
         raise TypeError(
             "syclobj keyword can be either None, or an instance of "
@@ -475,8 +489,8 @@ cdef class _Memory:
         # call kernel to copy from
         ERef = DPCTLQueue_Memcpy(
             self.queue.get_queue_ref(),
-            <void *>&host_buf[0],     # destination
-            <void *>self.memory_ptr,  # source
+            <void *>&host_buf[0],      # destination
+            <void *>self._memory_ptr,  # source
             <size_t>self.nbytes
         )
         with nogil: DPCTLEvent_Wait(ERef)
@@ -500,8 +514,8 @@ cdef class _Memory:
         # call kernel to copy from
         ERef = DPCTLQueue_Memcpy(
             self.queue.get_queue_ref(),
-            <void *>self.memory_ptr,  # destination
-            <void *>&host_buf[0],     # source
+            <void *>self._memory_ptr,  # destination
+            <void *>&host_buf[0],      # source
             <size_t>buf_len
         )
         with nogil: DPCTLEvent_Wait(ERef)
@@ -542,16 +556,16 @@ cdef class _Memory:
             if (same_contexts):
                 ERef = DPCTLQueue_Memcpy(
                     this_queue.get_queue_ref(),
-                    <void *>self.memory_ptr,
+                    <void *>self._memory_ptr,
                     <void *>src_buf.p,
                     <size_t>src_buf.nbytes
                 )
                 with nogil: DPCTLEvent_Wait(ERef)
                 DPCTLEvent_Delete(ERef)
             else:
                 copy_via_host(
-                    <void *>self.memory_ptr, this_queue,  # dest
-                    <void *>src_buf.p, src_queue,     # src
+                    <void *>self._memory_ptr, this_queue, # dest
+                    <void *>src_buf.p, src_queue,         # src
                     <size_t>src_buf.nbytes
                 )
         else:
@@ -565,7 +579,7 @@ cdef class _Memory:
 
         ERef = DPCTLQueue_Memset(
             self.queue.get_queue_ref(),
-            <void *>self.memory_ptr,  # destination
+            <void *>self._memory_ptr,  # destination
             <int> val,
             self.nbytes)
 
@@ -703,20 +717,29 @@ cdef class _Memory:
         res = _Memory.__new__(_Memory)
         _mem = <_Memory> res
         _mem._cinit_empty()
-        _mem.memory_ptr = USMRef
         _mem.nbytes = nbytes
         QRef_copy = DPCTLQueue_Copy(QRef)
         if QRef_copy is NULL:
             raise ValueError("Referenced queue could not be copied.")
         try:
-            _mem.queue = SyclQueue._create(QRef_copy)  # consumes the copy
+            # _create steals ownership of QRef_copy
+            _mem.queue = SyclQueue._create(QRef_copy)
         except dpctl.SyclQueueCreationError as sqce:
             raise ValueError(
                 "SyclQueue object could not be created from "
                 "copy of referenced queue"
             ) from sqce
-        _mem.refobj = memory_owner
-        return mem_ty(res)
+        if memory_owner is None:
+            _mem._memory_ptr = USMRef
+            # assume ownership of USM allocation via smart pointer
+            _mem._opaque_ptr = OpaqueSmartPtr_Make(<void *>USMRef, QRef)
+            _mem.refobj = None
+        else:
+            _mem._memory_ptr = USMRef
+            _mem._opaque_ptr = NULL
+            _mem.refobj = memory_owner
+        _out = mem_ty(<object>_mem)
+        return _out
 
 
 cdef class MemoryUSMShared(_Memory):
@@ -908,10 +931,13 @@ def as_usm_memory(obj):
             format(obj)
         )
 
+cdef api void * Memory_GetOpaquePointer(_Memory obj):
+    "Opaque pointer value"
+    return obj.get_opaque_ptr()
 
 cdef api DPCTLSyclUSMRef Memory_GetUsmPointer(_Memory obj):
     "Pointer of USM allocation"
-    return obj.memory_ptr
+    return obj.get_data_ptr()
 
 cdef api DPCTLSyclContextRef Memory_GetContextRef(_Memory obj):
     "Context reference to which USM allocation is bound"