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
ARROW-3451: [C++/Python] pyarrow and numba CUDA interop #2732
ARROW-3451: [C++/Python] pyarrow and numba CUDA interop #2732
Changes from 14 commits
f16148f
b629401
cf3aebe
5284ad0
4e20aa9
50aebe6
cba9bcf
ef69f35
a42ffd1
3a1e564
1107aad
1478909
0920105
cbe7338
740e474
1bf7856
f576474
bdc00ce
42fc232
c1b5717
f27796a
cc87f3b
a6d5376
cfb1e74
c546ce5
415cbd6
f9e69b2
1b0c19e
3e99126
767df42
39d3f69
fc5f505
8457e00
b6f4ede
b503b0c
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.
I admit I don't understand the difference between "get shared context" and "create shared context".
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.
"Create shared context" will create CudaContext with a context handle that is owned by another library (or created by another library).
"Get shared context" does "Create shared context" if the
CudaContext
for the given device is not created yet, and caches theCudaContext
instance. Otherwise, it will return the cached instance.This follows the same logic as in
GetContext
andCreateNewContext
methods.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.
Are all those variations actually useful? Isn't
GetSharedContext
enough?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.
The difference between the values returned by
GetSharedContext
andGetContext
is that in the latter case the created context is owned by arrow context manager (and that is allowed to destroy the context inClose
method).For the shared case, the context wrapped by
CudaContext
is assumed to be managed by another library.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.
But why do we need both
GetSharedContext
andCreateSharedContext
?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.
GetSharedContext
is what users should use in their code.CreateSharedContext
does not need to be exposed to users. Similarly,CreateNewContext
does not need to be exposed as well, IMHO.However, whoever designed the
CudaContext
, must have something in mind when makingCreateNewContext
public. There is alsoCudaContext::Close
method that is currently not used anywhere under Arrow ...So, I don't know the answer. I am fine with making
CreateSharedContext
private.