Skip to content
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

Add IPCHandle class for inter-process communication #8245

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Devdoot57
Copy link

@Devdoot57 Devdoot57 commented Mar 15, 2024

This pull request adds a new IPCHandle class to facilitate inter-process communication (IPC) for sharing Cupy arrays between processes (#8029). The IPCHandle class provides methods for creating IPC handles and managing shared memory, allowing for efficient data transfer between processes.
The .get_ipc_handle() method returns an instance of IPCHandle, which can be passed to other processes (e.g. via multiprocessing). On the destination process, use the .get() method to create a new ndarray object that shares the allocation from the original process.

# On source process
import cupy
arr = cupy.ones((3,3))
arr_ipc = arr.get_ipc_handle()

# On destination process
arr = arr_ipc.get()

Copy link
Member

@kmaehashi kmaehashi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Devdoot57!

self.arr_ptr = ipcOpenMemHandle(self.handle)
mem = UnownedMemory(self.arr_ptr, self.size, owner=self)
memptr = MemoryPointer(mem, offset=0)
arr = ndarray(shape=self.shape, dtype=self.dtype,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possilbe to handle ndarray-subclassing? The source might be an instance of user-defined subclass of cupy.ndarray.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so you want get() to return an instance of whatever class (ndarray or its subclass) was used while creating the handle?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of that, but on second thought, I think it's difficult to support it without modifying pickling behavior in the base class (ndarray). Maybe adding a warning in get_ipc_handle() if it is a subclass instance would be sufficient for now 🙂

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so for now one can still get a handle for a subclass, but open() will always return an instance of ndarray. I'll push the requested changes

from multiprocessing import get_start_method


class IPCHandle:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name it IPCMemoryHandle to distinguish it from IPC event handles.

Comment on lines 2090 to 2091
handle = ipcGetMemHandle(self.data.ptr)
return IPCHandle(self.shape, self.size, self.dtype, self.strides, handle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
handle = ipcGetMemHandle(self.data.ptr)
return IPCHandle(self.shape, self.size, self.dtype, self.strides, handle)
return IPCMemoryHandle(self)

I think it's better to retain the reference to self in the handle instance. Otherwise a = cupy.arange(10).get_ipc_handle() won't work as the backing ndarray will be freed immediately.

https://docs.nvidia.com/cuda/archive/9.0/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9

Calling cudaFree on an exported memory region before calling cudaIpcCloseMemHandle in the importing context will result in undefined behavior.

It is also better to document that users are responsible for keeping the IPC instance alive in the source process for the lifetime of the corresponding array in the destination process.

class IPCHandle:
"""
A serializable class representing an Inter-process Communication (IPC)
handle for sharing Cupy arrays between processes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

Suggested change
handle for sharing Cupy arrays between processes.
handle for sharing CuPy arrays between processes.

Comment on lines 37 to 38
if self.arr_ptr != -1:
ipcCloseMemHandle(self.arr_ptr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if self.arr_ptr != -1:
ipcCloseMemHandle(self.arr_ptr)
self.close()

Comment on lines 60 to 62
if get_start_method() == "fork":
raise RuntimeError(
"The handle cannot be spawned on a forked process.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check might be inaccurate, because it is not guaranteed that the process that created an IPC handle is the same one as the fork parent of the current process.

Suggested change
if get_start_method() == "fork":
raise RuntimeError(
"The handle cannot be spawned on a forked process.")

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. But, should I add an additional check because it throws cudaErrorInitializationError anyway?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds nice to show hint message for users in that case!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about putting this file as cupy/cuda/_ipc_handle.py? I think this one is not a part of core.

ipcCloseMemHandle(self.arr_ptr)
self.arr_ptr = -1

def get(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think def open(self) was better, considering close() is provided as an opposing concept 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants