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

[C++] Create registry for Devices to map DeviceType to MemoryManager in C Device Data import #40698

Closed
jorisvandenbossche opened this issue Mar 21, 2024 · 1 comment

Comments

@jorisvandenbossche
Copy link
Member

Describe the enhancement requested

Follow-up on #39980 (comment)

Right now, the user of ImportDeviceArray or ImportDeviceRecordBatch needs to provide a DeviceMemoryMapper mapping the device type and id to a MemoryManager. We provide a default implementation of that mapper that just knows about the default CPU memory manager (and there is another implementation in arrow::cuda, but you need to explicitly pass that to the import function)

To make this easier, the suggestion was to create a registry such that default device mappers can be added separately.

Component(s)

C++

jorisvandenbossche added a commit to jorisvandenbossche/arrow that referenced this issue Mar 21, 2024
jorisvandenbossche added a commit that referenced this issue Mar 27, 2024
…ryManager in C Device Data import (#40699)

### Rationale for this change

Follow-up on #39980 (comment)

Right now, the user of `ImportDeviceArray` or `ImportDeviceRecordBatch` needs to provide a `DeviceMemoryMapper` mapping the device type and id to a MemoryManager. We provide a default implementation of that mapper that just knows about the default CPU memory manager (and there is another implementation in `arrow::cuda`, but you need to explicitly pass that to the import function)

To make this easier, this PR adds a registry such that default device mappers can be added separately.

### What changes are included in this PR?

This PR adds two new public functions to register device types (`RegisterDeviceMemoryManager`) and retrieve the mapper from the registry (`GetDeviceMemoryManager`).

Further, it provides a `RegisterCUDADevice` to optionally register the CUDA devices (by default only CPU device is registered).

### Are these changes tested?

### Are there any user-facing changes?

* GitHub Issue: #40698

Lead-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@jorisvandenbossche jorisvandenbossche added this to the 16.0.0 milestone Mar 27, 2024
@jorisvandenbossche
Copy link
Member Author

Issue resolved by pull request 40699
#40699

vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…o MemoryManager in C Device Data import (apache#40699)

### Rationale for this change

Follow-up on apache#39980 (comment)

Right now, the user of `ImportDeviceArray` or `ImportDeviceRecordBatch` needs to provide a `DeviceMemoryMapper` mapping the device type and id to a MemoryManager. We provide a default implementation of that mapper that just knows about the default CPU memory manager (and there is another implementation in `arrow::cuda`, but you need to explicitly pass that to the import function)

To make this easier, this PR adds a registry such that default device mappers can be added separately.

### What changes are included in this PR?

This PR adds two new public functions to register device types (`RegisterDeviceMemoryManager`) and retrieve the mapper from the registry (`GetDeviceMemoryManager`).

Further, it provides a `RegisterCUDADevice` to optionally register the CUDA devices (by default only CPU device is registered).

### Are these changes tested?

### Are there any user-facing changes?

* GitHub Issue: apache#40698

Lead-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant