jorisvandenbossche commented on code in PR #42221:
URL: https://github.com/apache/arrow/pull/42221#discussion_r1700040015
##########
python/pyarrow/tests/test_cuda.py:
##########
@@ -534,6 +534,25 @@ def put(*args, **kwargs):
put(position=position, nbytes=nbytes)
+def test_buffer_device():
+ buf = cuda.new_host_buffer(10)
+ assert buf.device_type == pa.DeviceAllocationType.CUDA_HOST
+ assert isinstance(buf.device, pa.Device)
+ assert isinstance(buf.memory_manager, pa.MemoryManager)
+ assert buf.is_cpu
+ assert buf.device.is_cpu
+ assert buf.device == pa.default_cpu_memory_manager().device
Review Comment:
> If a CUDA_HOST buffer is reachable from the CPU using its address, then it
should probably have the CPU _device_,
A CudaHostBuffer is definitely reachable from the CPU, as we have this kind
of code in our tests (viewing that buffer as a numpy array):
```python
buf = cuda.new_host_buffer(size)
arr = np.frombuffer(buf, dtype=np.uint8)
# ... manipulate arr
```
> but which memory manager it should have is an open question. Presumably
the memory manager that's able to deallocate it?
"The memory allocated by this function (cuMemHostAlloc) must be freed with
cuMemFreeHost()" (from the NVIDIA docs), and the
`CudaHostBuffer::~CudaHostBuffer()` deleter uses
`CudaDeviceManager::FreeHost()` (which will indeed call `cuMemFreeHost`).
But, it's not that the MemoryManager returned from
`Buffer::memory_manager()` is directly used for deallocating the buffer AFAIK
(so it might not matter that much in practice).
> By the way, how I envisioned this is that if a given memory area can be
accessed both from CPU and from GPU, then it can have different Buffer
instances pointing to it. This is what the `View` and `ViewOrCopy` APIs are
for: they should ideally not force-copy if a transparent view is possible. This
is also why it's better to use those APIs than to force-copy the contents when
you have a non-CPU Buffer.
Yes, thanks for the explanation. For this PR we are not copying/viewing,
just checking the attributes of the created host buffer. But in
https://github.com/apache/arrow/pull/42223 I am adding bindings for `CopyTo`,
and so that is a good reason I should certainly expose `ViewOrCopyTo` as well.
And that reminds me that in the PrettyPrinting non-CPU data PR, I used
`CopyTo` which could actually use `ViewOrCopyTo`. Opened a PR for this ->
https://github.com/apache/arrow/pull/43508
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
