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++] Add functionality to MemoryManager for copying a slice of a buffer #39858

Closed
zeroshade opened this issue Jan 30, 2024 · 2 comments
Closed

Comments

@zeroshade
Copy link
Member

Describe the enhancement requested

Currently MemoryManager objects define functionality to Copy or View entire buffers. Occasionally there is the need to only copy a single value or slice from a buffer (see #39770 (comment)). It's overkill to do a bunch of whole Buffer operations and manually slicing just to copy 4 or 8 bytes.

Instead we can add new methods to the MemoryManager interface such as CopyBufferSlice etc.

Component(s)

C++

@alanstoate
Copy link
Contributor

take

pitrou pushed a commit that referenced this issue May 23, 2024
#41477)

### Rationale for this change
Currently ```MemoryManager``` objects define functionality to Copy or View entire buffers. Occasionally there is the need to only copy a single value or slice from a buffer to a piece of CPU memory (see #39770 (comment)). It's overkill to do a bunch of whole Buffer operations and manually slicing just to copy 4 or 8 bytes.

### What changes are included in this PR?
Add the ```MemoryManager::CopyBufferSliceToCPU``` function, which initially attempts to use memcpy for the specified slice. If this is not possible, it defaults to copying the entire buffer and then viewing/copying the slice.

Update ```ArrayImporter::ImportStringValuesBuffer``` to use this function.

### Are these changes tested?

```ArrayImporter::ImportStringValuesBuffer```  is tested as a part of  ```arrow-c-bridge-test```
* GitHub Issue: #39858

Lead-authored-by: Alan Stoate <alan.stoate@gmail.com>
Co-authored-by: Mac Lilly <maclilly45@gmail.com>
Co-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 17.0.0 milestone May 23, 2024
@pitrou
Copy link
Member

pitrou commented May 23, 2024

Issue resolved by pull request 41477
#41477

@pitrou pitrou closed this as completed May 23, 2024
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
…pointer (apache#41477)

### Rationale for this change
Currently ```MemoryManager``` objects define functionality to Copy or View entire buffers. Occasionally there is the need to only copy a single value or slice from a buffer to a piece of CPU memory (see apache#39770 (comment)). It's overkill to do a bunch of whole Buffer operations and manually slicing just to copy 4 or 8 bytes.

### What changes are included in this PR?
Add the ```MemoryManager::CopyBufferSliceToCPU``` function, which initially attempts to use memcpy for the specified slice. If this is not possible, it defaults to copying the entire buffer and then viewing/copying the slice.

Update ```ArrayImporter::ImportStringValuesBuffer``` to use this function.

### Are these changes tested?

```ArrayImporter::ImportStringValuesBuffer```  is tested as a part of  ```arrow-c-bridge-test```
* GitHub Issue: apache#39858

Lead-authored-by: Alan Stoate <alan.stoate@gmail.com>
Co-authored-by: Mac Lilly <maclilly45@gmail.com>
Co-authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
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

4 participants