Implement MPL_ALLOCATE for MPI buffers#109
Conversation
There was a problem hiding this comment.
Pull request overview
Draft implementation of an MPL allocator abstraction with multiple backends (Fortran allocate/deallocate, internal buddy allocator pool, optional pluto-based resources), plus build-system integration via fypp and a new allocator test suite.
Changes:
- Added
MPL_ALLOCATOR_MODAPI + submodule implementation with selectable backends (FORTRAN / BUDDY_ALLOC / PLUTO-based). - Introduced internal growable buddy allocator pool (C++ wrapper + Fortran bindings) and vendored
buddy_allocv1.3.1. - Added allocator tests and CMake plumbing (fypp preprocessing, test registration, optional pluto linkage).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/mpl_allocator/test_mpl_allocator.F90 | Adds an end-to-end test driver covering init/reserve/allocate/deallocate flows and backend selection. |
| tests/mpl_allocator/CMakeLists.txt | Registers allocator tests under CTest with multiple resource/default combinations. |
| tests/CMakeLists.txt | Hooks the new mpl_allocator test subdirectory into the test build. |
| src/fiat/mpl/mpl_allocator_mod.fypp | Introduces the public allocator API (init/final/reserve/query/allocate/deallocate/resource). |
| src/fiat/mpl/mpl_allocator_smod.fypp | Implements allocator backend selection and the allocate/deallocate procedures for multiple types/ranks. |
| src/fiat/mpl/internal/mpl_allocator/fiat_buddy_mod.fypp | Defines the Fortran FIAT_BUDDY wrapper type interfacing to the buddy allocator pool. |
| src/fiat/mpl/internal/mpl_allocator/fiat_buddy_smod.fypp | Implements FIAT_BUDDY methods, including allocation/deallocation via C interop. |
| src/fiat/mpl/internal/mpl_allocator/fiat_buddy_mod.cc | Implements the growable linked-list buddy allocator pool on top of buddy_alloc. |
| src/fiat/mpl/internal/mpl_allocator/buddy_alloc-v1.3.1/manifest | Records upstream provenance for vendored buddy_alloc. |
| src/fiat/mpl/internal/mpl_allocator/buddy_alloc-v1.3.1/buddy_alloc.h | Vendored third-party allocator header (buddy_alloc v1.3.1). |
| src/fiat/mpl/internal/mpl_allocator/buddy_alloc-v1.3.1/LICENSE.md | Adds third-party license for buddy_alloc. |
| src/fiat/mpl/internal/mpl_allocator/buddy_alloc | Provides the include path indirection to the vendored buddy_alloc version directory. |
| src/fiat/CMakeLists.txt | Adds fypp preprocessing outputs and buddy allocator sources to FIAT library; links pluto when enabled. |
| cmake/fiat_preprocess_fypp.cmake | Adds fypp discovery (including FetchContent fallback) and a helper to preprocess .fypp to .F90. |
| cmake/fiat_macros.cmake | Includes the new fypp preprocessing CMake module. |
| cmake/fiat-import.cmake.in | Exports pluto enablement and adds pluto dependency resolution for consumers. |
| README.md | Documents third-party buddy_alloc license attribution. |
| CMakeLists.txt | Enables fypp discovery and adds optional pluto feature configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
1063f31 to
c9dc5eb
Compare
|
@pmarguinaud please review when you can. @DJDavies2 I'm sure you will have some compilation issues to report? 🫣 |
|
Hello Willem, A linked list of buffers is not needed; we just need a single heap managed memory buffer that we allocate when the model starts with a size large enough (similar to MPL_MBX). This allocation will just reserve a set of virtual addresses, no physical memory. Also I do not see the point of the "VARIABLE_NAME" argument. Philippe |
|
Hi Philippe thank you for your initial review. The API is such that it would be possible to swap the buddy_alloc with another memory_resource like provided by pluto.
When using another memory_resource such as pluto, it is possible in debug/tracing mode to see which variables are allocated by this associated LABEL. Do you foresee any problem with this?
OK great, so following will give you the same effect, with only a single node in the linked list when SIZE is chosen appropriately: CALL MPL_ALLOCATOR_INIT(SIZE) ! Buffer gets created with given sizeor CALL MPL_ALLOCATOR_INIT() ! No buffer was yet created
CALL MPL_ALLOCATOR_RESERVE(SIZE) ! Buffer gets created with given sizeAnd in case SIZE was not enough, a second node (buffer) will be created which reserves extra virtual memory. In MPL_MBX_SIZE=1000000
CL_MBX_SIZE=' '
CALL GET_ENVIRONMENT_VARIABLE('MPL_MBX_SIZE',CL_MBX_SIZE)
IF (CL_MBX_SIZE /= ' ') THEN
READ(CL_MBX_SIZE,*) MPL_MBX_SIZE
ENDIFDo you suggest to make a similar environment variable for this allocator with a default value of 2GB? I suppose MPL_ALLOCATOR_INIT can read this in, to the effect of CALL MPL_ALLOCATOR_INIT(SIZE=${MPL_ALLOCATOR_SIZE})With the view of GPU-aware communication, do you envision we will need dedicated MPL_HOST_ALLOCATOR and MPL_DEVICE_ALLOCATOR in the future? Willem |
|
Having multiple nodes does not make sense to me. It will just fragment the memory. On the GPU there is no virtual memory, so we will have to be careful with the size of the buffer we pre-allocate and tune it; but it is one out of many other constraints that we have with GPUs. |
|
Thanks for your feedback. I have added tests that verify that following fails with an ABOR1 message without any changes to the environment; so the absolute default: CALL MPL_ALLOCATOR_INIT(SIZE=<MPL_ALLOCATOR_SIZE>)
CALL MPL_ALLOCATE(...) ! try allocating more than availableAlso tested is setting environment variable Also following is supported: Which will by default need to allow for growing. export MPL_ALLOCATOR_SIZE=1GB # Note the use of units
export MPL_ALLOCATOR_GROW=0 # No growing allowedI think this provides enough flexibility to satisfy all use cases:
Thanks once more for your feedback to come to this solution. |
|
I would suggest an additional allocator called "LINEAR" where we allocate/reallocate two buffers, one for sends and one for receives, until we reach steady state such that all subsequent uses require less than the size of the send/receive buffers. This allows very simple book keeping and reduces the numbers of buffer registrations and cache entries in the NIC. Since the API allows us to choose any allocation strategy, one could choose between linear, buddy_alloc or any other implementation depending on their use case. |
I do have one thing to report that I have noticed. When I do incremental compiles using this branch, I get compile failures like this: 547 | CALL MPL_PLUTO_ALLOCATOR%ALLOCATE(LABEL, ARRAY, LBOUNDS, UBOUNDS) Looking at the cmake output it appears that for clean builds I get this: -- Could NOT find pluto (missing: pluto_DIR) whereas for an incremental build I get this: -- fiat FOUND pluto: /home/users/david.davies/cylc-run/mo-bundle-415/share/make_mo_base__spice_gnu/atlas/pluto (found version "0.46.0") The only change is a simple code change in fckit so I don't know why it is re-doing cmake at all. In any case there is no change that should affect whether pluto is detected or not. |
|
Thanks @DJDavies2 that is probably only in a bundle configuration where atlas with pluto comes later than fiat in the order. I can fix this, thanks for reporting. |
…and optional pluto backend Co-authored-by: Philippe Marguinaud <philippe.marguinaud@meteo.fr>
29ce37e to
2c9c373
Compare
marsdeno
left a comment
There was a problem hiding this comment.
Very nice development!
The test suite is particularly nice
|
I do think it might be worth calling |
aad1bd1 to
3d013f0
Compare
This is a proposal for issue #91
MPI_Alloc_memandMPI_Alloc_freeallocation. This is the default if pluto feature is enabled.The allocator needs to be set up:
CALL MPL_ALLOCATOR_INIT()This will initialise with a default backend "MPI_POOL" (if pluto feature is enabled), else "BUDDY_ALLOC". No memory is allocated at this point! An optional RESOURCE argument can be passed if needed. If not passed, the RESOURCE can be chosen as well via environment variable "MPL_RESOURCE":
Via hardcoding, coming from some configuration file (namelist) we could also pass argument directly:
To allocate / deallocate
Current proposal intialises the memory pool upon first call to
MPL_ALLOCATEto a size large enough to cater for the required allocation greater than 256Mb, and grows each time with a factor of 4, so quickly reaches large sizes: 256MB, 1GB, 4GB, ..CALL MPL_ALLOCATOR_RESERVE( SIZE )Initialises and tries to allocate + deallocate SIZE bytes in the pool. This essentially grows the memory pool to cater for envisioned allocations of SIZE.
With
MPL_ALLOCATOR_RESERVEone can immediately reserve what's needed, but perhaps it is not going to be necessary, and we can rely on justMPL_ALLOCATOR_INITwithout extra steps.There are query functions:
MPL_ALLOCATOR_INITIALIZED(): Check ifMPL_ALLOCATOR_INIT()was calledMPL_ALLOCATOR_CAPACITY(): the total size the pool can contain. When growing is required, this will increase.MPL_ALLOCATOR_ALLOCATED(): the current amount of bytes allocated using the MPL_ALLOCATOR.Finally, we will need to end with
MPL_ALLOCATOR_FINAL()More thoughts: Should we
CALL MPL_ALLOCATOR_INITandCALL MPL_ALLOCATOR_FINALwithin MPL_INIT and MPL_END?