Thread safety for `MemoryManager`

[sebastiangrimberg]

Thread safety for `MemoryManager`

[v-dobrev]

Yes, I think you have the right idea.

One thing to keep in mind is to avoid doing Memory operations in the OMP parallel regions that force the host pointer to be registered with the memory-manager -- basically the registration is most probably not thread-safe. For example, if you create a Vector and make it reference another Vector with Vector::MakeRef then this will call Memory::MakeAlias which will call the method to register the "base" pointer with the memory manager (in cases like yours when the globally-set "device" memory type is actual device memory type like cuda or hip). I'm not 100% sure that this will absolutely cause a problem but it might.

One feature that should help is that all device allocations are lazy -- the actual device memory allocation does not happen before you actually request the device pointer with a call like Vector::Read or Vector::ReadWrite.

[v-dobrev]

Yes, we have not really considered thread safety in the Memory and MemoryManager classes -- basically allocations are never done inside mfem::forall* calls, so none of the MFEM backends need it.

I guess you have a coarser grain parallelism via OpenMP on CPU in mind to need thread safety. Since this is just on CPU, isn't the standard host allocator (memory type HOST) sufficient? That should work fine in threaded environment as you pointed out. If you need to have a build where this mode of execution is supported along with other modes like cuda with Umpire allocators -- that should be possible -- you can configure the memory manager with what memory types to use at the beginning of the program.

[sebastiangrimberg]

That's exactly right about the use case I have in mind. I can be assured that I won't ever call Memory::Read or Write inside a parallel region with the device memory class, but I do want to be able to call it with the host class in some coarse-grained cases. One use case is to be able to call FiniteElementSpace::GetElementDofs or Mesh::GetElementTransformation inside an OpenMP parallel region, used to set up a data structure which will eventually (outside of the OpenMP loop) be used on the GPU.

This is helpful suggestion, yes I think HOST is always sufficient for the host. Would the correct way to achieve this be to call, before configuring the Device, Device::SetMemoryTypes(MemoryType::HOST, device_mem_type) (where device_mem_type could be any memory type, including DEVICE, MANAGED, DEVICE_DEBUG, etc.)? This should work, as long as I don't have ":uvm" in my device string or set Backend::DEBUG_DEVICE, which would cause Device::UpdateMemoryTypeAndClass to override my setting of the host_mem_type to something not thread safe.

Do I have the right idea? Thanks a lot for your help!