Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

CoreNEURON memory allocation routines are confusing #806

Open
olupton opened this issue Apr 26, 2022 · 0 comments · May be fixed by #872
Open

CoreNEURON memory allocation routines are confusing #806

olupton opened this issue Apr 26, 2022 · 0 comments · May be fixed by #872
Assignees

Comments

@olupton
Copy link
Contributor

olupton commented Apr 26, 2022

Describe the issue
The current codebase uses a wide range of different memory [de]allocation routines, starting from a mix of new/delete and malloc/free and extending (via other system calls like posix_memalign) to various homespun methods like coreneuron::[de]allocate_unified:

/** @brief Allocate unified memory in GPU builds iff GPU enabled, otherwise new
*/
void* allocate_unified(std::size_t num_bytes);
/** @brief Deallocate memory allocated by `allocate_unified`.
*/
void deallocate_unified(void* ptr, std::size_t num_bytes);

alloc_memory, calloc_memory, free_memory:
/** @brief Allocate unified memory in GPU builds iff GPU enabled, otherwise new
*/
void* allocate_unified(std::size_t num_bytes);
/** @brief Deallocate memory allocated by `allocate_unified`.
*/
void deallocate_unified(void* ptr, std::size_t num_bytes);

and helper structs like MemoryManaged:
class MemoryManaged {
public:
void* operator new(size_t len) {
void* ptr;
cudaMallocManaged(&ptr, len);
cudaDeviceSynchronize();
return ptr;
}
void* operator new[](size_t len) {
void* ptr;
cudaMallocManaged(&ptr, len);
cudaDeviceSynchronize();
return ptr;
}
void operator delete(void* ptr) {
cudaDeviceSynchronize();
cudaFree(ptr);
}
void operator delete[](void* ptr) {
cudaDeviceSynchronize();
cudaFree(ptr);
}
};

Some of these names are not very descriptive, and some of their behaviours change according to compile time and runtime options, which has led to bugs (see #594, for example) when (for example) we end up with mismatched allocations and deallocations.

This should be improved, with more descriptively named methods and better organisation that enforces consistent pairing of allocation and deallocation functions.

Discussion
Note that we need to be able to request a few different types of allocation. For example, in GPU builds, we may need to distinguish between:

  • Host-only memory.
  • Unified memory (accessible from both host and device) even when CORENRN_ENABLE_CUDA_UNIFIED_MEMORY=OFF, for things like Random123 state where we require unified memory (Use CUDA unified memory for Random123 state #595)
  • Unified memory if CORENRN_ENABLE_CUDA_UNIFIED_MEMORY=ON, otherwise host memory.

Additionally the last two should probably return host-only memory in GPU builds where the GPU was not enabled at runtime by passing --gpu or coreneuron.gpu = True.

Ideally we would handle these through a more uniform API that makes these distinctions clear.
Right now, {alloc,calloc,free}_memory and MemoryManaged provide point 3 (though without a test on --gpu), coreneuron::[de]allocate_unified provide point 2, and a mix of standard APIs provide point 1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants