-
Notifications
You must be signed in to change notification settings - Fork 34
Rework host memory allocation mechanics #1964
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
base: develop
Are you sure you want to change the base?
Changes from all commits
8f6820a
4dc85f6
11f118a
abc8eb4
7566b92
cf9a523
5db9bd4
f88cc30
43ee2c5
20e4ab1
13bfcda
8a0a4d2
1d39dcd
887ef6b
c68478b
e641dc0
c8abea7
89dd157
b28af9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,15 +157,24 @@ inline void setDefaultAllocator(umpire::resource::MemoryResourceType resource_ty | |
| #endif | ||
|
|
||
| /*! | ||
| * \brief Sets the default memory allocator to use. | ||
| * \param [in] allocId the Umpire allocator id | ||
| * \brief Sets the default memory allocator for the Umpire ResourceManager. | ||
| * \param [in] allocId the Axom allocator id | ||
| * | ||
| * \note When Axom is compiled with Umpire and \a allocId is | ||
| * axom::MALLOC_ALLOCATOR_ID, this function sets Umpire's default | ||
| * allocator to its Host resource. | ||
| * \note This function has no effect when Axom is not compiled with Umpire. | ||
| */ | ||
| inline void setDefaultAllocator(int allocId) | ||
| { | ||
| #ifdef AXOM_USE_UMPIRE | ||
| umpire::ResourceManager& rm = umpire::ResourceManager::getInstance(); | ||
| if(allocId == MALLOC_ALLOCATOR_ID) | ||
| { | ||
| rm.setDefaultAllocator(rm.getAllocator(umpire::resource::Host)); | ||
| return; | ||
| } | ||
|
|
||
| umpire::Allocator allocator = rm.getAllocator(allocId); | ||
| rm.setDefaultAllocator(allocator); | ||
| #else | ||
|
|
@@ -188,6 +197,27 @@ inline int getDefaultAllocatorID() | |
| #endif | ||
| } | ||
|
|
||
| namespace detail | ||
| { | ||
| /*! | ||
| * \brief Returns the ID of the default host allocator. | ||
| * | ||
| * \note This is distinct from the current default allocator returned by | ||
| * axom::getDefaultAllocatorID(), which tracks Umpire's default allocator | ||
| * when Axom is configured with Umpire. | ||
| * | ||
| * \return ID of the default host allocator. | ||
| */ | ||
| inline int getDefaultHostAllocatorID() | ||
| { | ||
| #if defined(AXOM_DEFAULT_HOST_ALLOCATOR_USES_UMPIRE_HOST) | ||
| return getUmpireResourceAllocatorID(umpire::resource::Host); | ||
| #else | ||
| return MALLOC_ALLOCATOR_ID; | ||
| #endif | ||
| } | ||
| } // namespace detail | ||
|
|
||
| /*! | ||
| * \brief Get the allocator id from which data has been allocated. | ||
| * \return Allocator id. If Umpire doesn't have an allocator for the | ||
|
|
@@ -241,7 +271,7 @@ int getSharedMemoryAllocatorID(std::size_t minSegmentSize = 0); | |
| * \brief Allocates a chunk of memory of type T. | ||
| * | ||
| * \param [in] n the number of elements to allocate. | ||
| * \param [in] allocID the Umpire allocator to use (optional) | ||
| * \param [in] allocID the Axom/Umpire allocator to use (optional) | ||
| * | ||
| * \tparam T the type of pointer returned. | ||
| * | ||
|
|
@@ -260,7 +290,7 @@ inline T* allocate(std::size_t n, int allocID = getDefaultAllocatorID()) noexcep | |
| * | ||
| * \param [in] n the number of elements to allocate. | ||
| * \param [in] name allocation name (must be non-empty for shared memory allocators) | ||
| * \param [in] allocID the Umpire allocator to use (optional) | ||
| * \param [in] allocID the Axom/Umpire allocator to use (optional) | ||
| * | ||
| * \return pointer to the new allocation or a nullptr if allocation failed. | ||
| */ | ||
|
|
@@ -360,6 +390,11 @@ inline T* allocate(std::size_t n, int allocID) noexcept | |
| { | ||
| const std::size_t numbytes = n * sizeof(T); | ||
|
|
||
| if(allocID == MALLOC_ALLOCATOR_ID) | ||
| { | ||
| return static_cast<T*>(std::malloc(numbytes)); | ||
| } | ||
|
|
||
| #ifdef AXOM_USE_UMPIRE | ||
| if(umpire::ResourceManager& rm = umpire::ResourceManager::getInstance(); rm.isAllocator(allocID)) | ||
| { | ||
|
|
@@ -368,11 +403,6 @@ inline T* allocate(std::size_t n, int allocID) noexcept | |
| } | ||
| #endif | ||
|
|
||
| if(allocID == MALLOC_ALLOCATOR_ID) | ||
| { | ||
| return static_cast<T*>(std::malloc(numbytes)); | ||
| } | ||
|
|
||
| std::cerr << "Unrecognized allocator id " << allocID << std::endl; | ||
| axom::utilities::processAbort(); | ||
|
|
||
|
|
@@ -384,6 +414,12 @@ inline T* allocate(std::size_t n, const std::string& name, int allocID) noexcept | |
| { | ||
| const std::size_t numbytes = n * sizeof(T); | ||
|
|
||
| if(allocID == MALLOC_ALLOCATOR_ID) | ||
| { | ||
| AXOM_UNUSED_VAR(name); | ||
| return static_cast<T*>(std::malloc(numbytes)); | ||
| } | ||
|
|
||
| #ifdef AXOM_USE_UMPIRE | ||
| if(umpire::ResourceManager& rm = umpire::ResourceManager::getInstance(); rm.isAllocator(allocID)) | ||
| { | ||
|
|
@@ -393,12 +429,6 @@ inline T* allocate(std::size_t n, const std::string& name, int allocID) noexcept | |
| } | ||
| #endif | ||
|
|
||
| if(allocID == MALLOC_ALLOCATOR_ID) | ||
| { | ||
| AXOM_UNUSED_VAR(name); | ||
| return static_cast<T*>(std::malloc(numbytes)); | ||
| } | ||
|
|
||
| std::cerr << "Unrecognized allocator id " << allocID << std::endl; | ||
| axom::utilities::processAbort(); | ||
|
|
||
|
|
@@ -547,13 +577,12 @@ inline void fill(void* dst, std::size_t n, const T& value) noexcept | |
| doHostFill = false; | ||
|
|
||
| // Device memory: fill on host, then copy to device | ||
| const auto num_bytes = n * sizeof(T); | ||
| T* src = allocate<T>(num_bytes, rm.getDefaultAllocator().getId()); | ||
| T* src = allocate<T>(n, axom::detail::getDefaultHostAllocatorID()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. It's not here but in |
||
| for(std::size_t i = 0; i < n; ++i) | ||
| { | ||
| src[i] = value; | ||
| } | ||
| rm.copy(dst, src, num_bytes); | ||
| axom::copy(dst, src, n * sizeof(T)); | ||
| deallocate<T>(src); | ||
| } | ||
| } | ||
|
|
||
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.
Shouldn't
setDefaultAllocatoralso be influenced byAXOM_DEFAULT_HOST_ALLOCATOR_USES_UMPIRE_HOST?It seems that if we're in an Umpire build, we can never get back to the malloc allocator as the default.
E.g., consider the following in a build with Umpire:
Uh oh!
There was an error while loading. Please reload this page.
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.
This method is only setting the default allocator in the Umpire ResourceManager. As long as all Axom usage honors the Axom default (Malloc or Host) when appropriate this should be OK.
Do we need to handle the case where a user calls
setDefaultAllocator()with something that is neither Malloc or Host, but is still valid on the host, such as Pinned when Umpire is enabled? As I understand it, that is the main purpose of this method.I think only the method documentation needs to be clarified, which I did.
Does that make sense now?
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.
If that's the case, perhaps it should be renamed
setUmpireDefaultAllocator?(and be a no-op in non-umpire configs)
This is the current doxygen brief for the
setDefaultAllocatorUntil now, we've been using it as Axom's default allocator
(which points to Umpire when Axom is configured against Umpire).
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 modified the doxygen brief comment to indicate that it sets the default allocator in the Umpire ResourceManager, which it did before my changes IIRC. It is a no-op in non-umpire configs.