Excessive allocations for faces construction

[amrueda]

While working with very large meshes, @mchavezmodena and I identified a serious memory blow-up occurring during face construction.

We traced the problem to the allocation of self%faces in multiple mesh-reading routines, for example:
https://github.com/loganoz/horses3d-gpu/blob/b4a26d1a68dd6dfe70b1179f97b224829c4a86ff/Solver/src/libs/mesh/Read_HDF5Mesh_HOPR.f90#L361
https://github.com/loganoz/horses3d-gpu/blob/b4a26d1a68dd6dfe70b1179f97b224829c4a86ff/Solver/src/libs/mesh/Read_HDF5Mesh_HOPR.f90#L762
https://github.com/loganoz/horses3d-gpu/blob/b4a26d1a68dd6dfe70b1179f97b224829c4a86ff/Solver/src/libs/mesh/Read_SpecMesh.f90#L262
(etc... it occurs in all mesh constructors!)

The case where we found it

We encountered the problem in a large mesh with numberOfFaces = 47607081 that was crashing with

ALLOCATE: 718295638128 bytes requested

In other words, it was trying to allocate 718GB of storage for an array with "only" ~47.6M entries. This is surprising because:

1. The Face derived type has only some (<30) integer/logical scalars + no large data allocated at construction time
   https://github.com/loganoz/horses3d-gpu/blob/b4a26d1a68dd6dfe70b1179f97b224829c4a86ff/Solver/src/libs/mesh/FaceClass.f90#L63
2. The nested types (MappedGeometryFace, FaceStorage_t) only contain allocatable arrays, which are unallocated at this stage.
3. A rough estimate suggests that one Face object should only require a few hundred bytes.

... So the numbers did not add up.

Cause of the issue

Many Fortran compilers must store allocatable component descriptors inside every element of an array of derived types. These descriptors include lower/upper bounds, rank, type info, vtable/type-binding pointers, and other bookkeeping fields. Therefore, the compiler can explode the required size if you have arrays of derived types with allocatable components. And this is exactly what's happening here.

[amrueda]

A fix has been proposed here: #33

[Jerryntk]

Hello Andres, nice catch. I think in the first stage of the mesh reader we don not even need to allocate the faces or some parts of it so this definitely solves a few different issues. Has anyone tried running the code with the proposed fixes using gpus? My only concern is whether we need to attach the pointer while we perform the deep copy for the faces. This is a weakness of OpenACC. Also, the pointer will introduce some overhead in the computation and we need to quantify that.

To take it a step further, another improvement would be to allocate a big chunk of memory for each array in storage (e.g. Q) for all elements and then point from each elements(eid) % storage % Q to that array. Right now one of the main reasons for preprocessing to be very slow is that we allocate many small arrays (N^3*NCONS) within each element and face storage. We do not intend to do dynamic p-adaptation in the gpu soon, so the large array would not be a problem to anything.

If the code has been tested on GPU and passes the tests on GPU then the commit should be fine.

[loganoz]

Sorry for the late reply. This week has been weird at the school with many bank holidays.
Thank you @Jerryntk for taking a look.

This modification has been tested in GPU by @mchavezmodena. In fact, with this modification we can run the big CRM mesh, that was not possible before. However, I do not think we have done any kind of profiling yet.
@Dhueper was also concerned about the use of pointers, may be he can explain in more detail to evaluate the risks posed by the fix.

[Dhueper]

@Jerryntk last week I solved a memory leak issue in horses related to the use of pointers. When you send a pointer as an input argument to a subroutine, and if that pointer is a derived type with allocatable components inside, the compiler (depending on the version and which compiler you use) may create a deep copy of the pointer instead of using a reference. That deep copy is automatically destroyed at the end of the subroutine when the variable goes out of the scope, but all the internal allocatable components are not. Therefore, you have a memory leak.
Although the proposed fix #33 solves the problem right now, it requires converting the Face Storage and Face Geometry to pointers, which are used many times inside horses. Even if we make sure it is not creating a memory leak right now, in the near future someone who is not aware of this issue will use those pointers in a different subroutine and we will have that memory leak again.
As new students are coming almost every month (or every year at least), sooner or later pointers will create problems.
I recommend avoiding pointers as much as possible. Everything would be simpler if we remove every pointer inside horses in my opinion.
Anyway, using pointers has advantages and inconveniences and I think @loganoz is the one who should make a decision on this point.