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

Support malloc/free when configured with Umpire #1507

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

gunney1
Copy link
Contributor

@gunney1 gunney1 commented Feb 21, 2025

Summary

  • This PR adds support for malloc/free memory when Axom is configured with Umpire.
  • It does the following:
    • Introduces Malloc memory space and its associated MALLOC_ALLOCATOR_ID. These use malloc, free and realloc.
    • Fixes memory_management.hpp crashes caused by assuming all pointers are from Umpire.
    • Clarifies that memory_management.hpp assumes that non-Umpire memory is from malloc and realloc.
    • Test allocating and deallocating memory in the Malloc memory space.
    • Test reallocating memory between Malloc, Host and all device memory spaces supported.
    • Changes axom:: getAllocatorIDFromPointer to return MALLOC_ALLOCATOR_ID instead of DYNAMIC_ALLOCATOR_ID. The latter doesn't refer to a specific allocator but the current Umpire default allocator, which can change.
    • Remove DYNAMIC_ALLOCATOR_ID, whose role was taken over by MALLOC_ALLOCATOR_ID.

New test uses all available memory spaces and reallocates
memory between them.  The test crashes because we assume
memory is from Umpire even when it's Dynamic.
This fixes (1) Dynamic memory space allocation/reallocation and
(2) reallocations betwen Dynamic memory space and Umpire spaces.
@gunney1 gunney1 added bug Something isn't working Core Issues related to Axom's 'core' component GPU Issues related to GPU development labels Feb 21, 2025
@gunney1 gunney1 self-assigned this Feb 21, 2025
Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might need to discuss this further since the proposed change might lead to unexpected behaviors.

Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gunney1 and I discussed this today, and he will rework this PR.

It seems that he has a user request to support malloc allocations through Axom's memory_management API even when Axom is configured with Umpire.

I recommended that this new use case should use an allocator ID that is not valid with umpire (e.g. a negative number other than -1, which we currently use as our INVALID_ALLOCATOR_ID).

He will also remove/rename the DYNAMIC_ALLOCATOR_ID global variable, which was only recently added since the name is confusing and it uses a potentially valid Umpire allocator ID (0)

@gunney1 gunney1 changed the title Fix Dynamic memory space when configured with Umpire Add Malloc memory space Mar 18, 2025
@gunney1 gunney1 changed the title Add Malloc memory space Support malloc/free when configured with Umpire Mar 18, 2025
@gunney1 gunney1 requested a review from kennyweiss March 21, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Core Issues related to Axom's 'core' component GPU Issues related to GPU development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants