-
Notifications
You must be signed in to change notification settings - Fork 23
Add experimental ManagedArrayPointer #350
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
Conversation
robinson96
left a comment
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.
LGTM
| across multiple backends. It has pointer semantics, meaning that copies are shallow, | ||
| which allows this object to be passed by value to a CUDA or HIP kernel. When copy | ||
| constructed, it queries the array manager to update the cached size and pointer | ||
| from the array manager. The array must be explicitly freed from only one of the |
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.
Is there a way for a user to check whether the data referenced by a ManagedArrayPointer has been freed; e.g., by another pointer object pointing to the same data? If not, it may be worth considering. I can see how that would be useful.
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.
Not without a lot of extra tracking. My plan was to add another class called ManagedArraySharedPointer that would act like std::shared_ptr and avoid the need to explicitly free.
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.
Yeah. I figured that would be the case. You solution sounds like it should do the trick.
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.
Looks good to me. One comment to consider.
Also, you may want to rework the copyright statements in the files like I did in RAJA. Then, updating the copyright year only requires touching 2 files instead of all of them, which kinda pollutes the commit history of each file.
Here's the change: llnl/RAJA#1976
ManagedArrayPointer is the experimental replacement for ManagedArray. All the logic for handling various memory types is delegated to a ManagerType template parameter.
This class provides a uniform interface for working with different types of memory across multiple backends. It has pointer semantics, meaning that copies are shallow, which allows this object to be passed by value to a CUDA or HIP kernel. When copy constructed, it queries the array manager to update the cached size and pointer from the array manager. The array must be explicitly freed from only one of the shallow copies.