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

Allow custom allocator to be injected in more places #690

Open
paleolimbot opened this issue Nov 25, 2024 · 3 comments
Open

Allow custom allocator to be injected in more places #690

paleolimbot opened this issue Nov 25, 2024 · 3 comments

Comments

@paleolimbot
Copy link
Member

paleolimbot commented Nov 25, 2024

When developing https://github.com/paleolimbot/duckdb-nanoarrow it was apparent that we need the ability to inject an ArrowBufferAllocator into the ArrowIpcArrayStreamReader for clients that need tight control over memory usage.

There is also at least one place in the device API where we assume a buffer allocator:

In those two examples specifically a lack of custom allocator isn't blocking any real-world case (in DuckDB we can also do a better job coordinating IO if we skip the ArrowIpcArrayStreamReader entirely and go through the lower-level ArrowIpcDecoder; in cudf they have their own code for copying arrays and don't use the device API); however, the option should still be there.

@paleolimbot
Copy link
Member Author

@vyasr @zeroshade I had some feedback that the new cudf Arrow converter doesn't allow a caller-specified allocator. I know you use this already on the GPU side, but does cudf have an CPU allocator abstraction for this somewhere that you can plug in?

@vyasr
Copy link
Contributor

vyasr commented Dec 2, 2024

No, not at the moment. Pre-nanoarrow our legacy arrow layer had some internal controls over how memory was allocated, but there was nothing publicly exposed and that didn't make it into the new implementations. As you noted, the new functions do support providing a device allocator in the form of an rmm memory resource. We could certainly consider adding something similar for host allocation if there are use cases.

@paleolimbot
Copy link
Member Author

Thanks for the note! I did have one complaint 🙂 (but I'll let them raise the feature request if it comes up for them).

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

No branches or pull requests

2 participants