Memory pool aborts on zero allocation request, returns NULL for < minimum

[ibaned]

Moving this out of #786 for @kyungjoo-kim.
He wrote:

there is a bug in memory pool. The following is the error message. The code requested 9480 and the pool clearly has enough space but it returns a NULL pointer.

requested = 9480
pool_size(4194304) superblock_size(1048576)
Superblock[ 0 / 4 ] { block_size(65536) block_count( 3 / 16 )
Superblock[ 1 / 4 ] { block_size(131072) block_count( 1 / 8 )
Superblock[ 2 / 4 ] { block_size(32768) block_count( 1 / 32 )
Superblock[ 3 / 4 ] { block_size(262144) block_count( 2 / 4 )
>> Error in file /home/kyukim/Work/lib/trilinos/temp/packages/shylu/tacho/refactor/src/TachoExp_TaskFunctor_CholSupernodes.hpp, line 84, error 1 
   bufmemory pool allocation fails

[ibaned]

@kyungjoo-kim I think what is happening here is your are asking for an allocation that is smaller than the minimum allocation size that you gave to the memory pool constructor, which is not allowed. Can you confirm what is the minimum allocation size passed to the constructor?

[ibaned]

@kyungjoo-kim also wrote in #786

Another bug... When it has 0 allocation, it aborts (it should return a NULL).

before allocate 0
Kokkos MemoryPool allocation request exceeded specified maximum allocation size
Program received signal SIGABRT, Aborted.

I believe this is the same problem, unless you passed in zero as the minimum allocation size to the memory pool constructor.

[hcedwar]

• Should return NULL for 0 allocation size - is a bug.
• Should search for superblock with smallest available block size greater than or equal to requested allocations size instead of a choosing a prescribed block size - is an enhancement.

[kyungjoo-kim]

@ibaned the minimum block size that I set in the constructor is "1". If it is modified, it is modified in kokkos. The minimum allocation size does not matter as far as I know. If the requested memory size is small, it should return it from the smallest block size.

[ibaned]

Hmm okay I thought previously it did matter and was a hard limit (seems like this is true due to the bug report), but as @hcedwar pointed out we can move to a more relaxed behavior where it chooses the smallest block size. We also need to document the behavior more clearly.

[kyungjoo-kim]

So, this means that I can bypass the allocation failure of smallest block size with

const auto bufsize = max(poolmin_block_size(), request);

Correct ?

[ibaned]

I think thats true... until we change the code to do this internally.

[hcedwar]

Unfortunately no. If superblocks are all used by larger-than-minimum block size then the allocation will currently fail due to not searching.

[ibaned]

That sounds like a bug as well... worst case it could prevent any allocations smaller than the maximum value.

[kyungjoo-kim]

I tested and it does not work. I query the min block size and it remains "1".

requested = 80688 , min block size = 1
pool_size(4194304) superblock_size(1048576)
Superblock[ 0 / 4 ] { block_size(524288) block_count( 1 / 2 )
Superblock[ 1 / 4 ] { block_size(65536) block_count( 2 / 16 )
Superblock[ 2 / 4 ] { block_size(32768) block_count( 3 / 32 )
Superblock[ 3 / 4 ] { block_size(262144) block_count( 1 / 4 )

[hcedwar]

OK - let's call both of these cases functionality bugs that require enhancing implementation to fix.

[kyungjoo-kim]

deallocate(buf, bufsize); where buf is NULL and bufsize is zero. This should do no-op but it also triggers abort.

[kyungjoo-kim]

Another feature request.

I have a situation that I repeatedly solve a problem with the same symbolic structure. I roughly allocate memory pool with loose estimation of tasks and memory requirement.

Can I get some useful information from scheduler or memory pool ? so that in the next iteration, I can set more tight (optimal) bound of my memory pool.

[hcedwar]

@kyungjoo-kim
Currently on the 'task-dag' branch: The zero size and NULL bug fixed. Using a larger block size when the desired block size is unavailable enhancement is done.

  MemoryPool<...>::usage_statistics tmp ;
  MemoryPool<...>::get_usage_statistics( tmp );

Will fill the tmp data structure with allocation usage statistics.

[kyungjoo-kim]

Thanks. I will try and test 'task-dag' branch.

[kyungjoo-kim]

soon we also need to merge the branch to master and snapshotted to trilinos so that sierra can see the code fix.