Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
GH-34971: [Format] Add non-CPU version of C Data Interface #34972
GH-34971: [Format] Add non-CPU version of C Data Interface #34972
Changes from 15 commits
3f9519d
63a65dc
fc16391
26c9aa9
e85d307
13e94a5
826390e
09e5ee6
564ce58
e74c286
1eb7ee9
05f70f7
43ce6c9
2c9a7a6
4f636a8
4e1d1f6
2b24a10
451d3a3
e34746f
b359239
4845e19
fa8aab0
e068bc3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please help me understand the
void* sync_event
pointer a bit better in the context of CUDA/C++? Is this a function pointer that is expected to be called with acudaStream_t
parameter provided by the application? Would there be a benefit from storing acudaStream_t
inArrowDeviceArray
(in thereserved
bytes or elsewhere)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case of CUDA, this would be a pointer to a
cudaEvent_t
. It would be the producer's responsibility to create the event and callcudaEventRecord
to ensure the relevant work in the stream(s) are captured in the event. The consumer can then callcudaStreamWaitEvent
which is typically a device-side more efficient stream synchronization mechanism thancudaStreamSynchronize
(if they need to wait for host code they can still usecudaEventSynchronize
instead).If both sides are on the same stream, then the
cudaStreamWaitEvent
call should have negligible overhead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, in the context of CUDA/C++ the
void* sync_event
would be expected to be acudaEvent_t*
that the producer created and will trigger based on when the data is available.A consumer would then do something like:
As per the previous discussions, most frameworks for cuda don't actually make their internal streams easily externally available so we aren't expecting a
cudaStream_t
to get passed. In the future if usage deems it necessary, we could absolutely leverage the reserved bytes to add a stream/queue pointer or something. But the initial pass here is intended to pass an event via thevoid* sync_event
that a queue can wait on from the producer and then just operate on the data from there.