Request for review: JDK-8009561 NPG: Metaspace fragmentation when retiring a Metachunk

[Jon Masamitsu]

Mikael,

1) Should we not split the block if the remainder is not larger than
the minimum size for the dictionary?  Just return the block to the
dictionary and fail the allocation?

2) Could you reuse "unused" like this.

  825    size_t unused = block_size - word_size;
  824   if (unused > TreeChunk<Metablock, FreeList>::min_size()) {
  826     return_block(new_block + word_size, unused);
  827   }


3) Your change to reduce the threshold is a good one.  Was this motivated
by some behavior you observed.

Thanks.

Jon


On 8/14/2013 5:59 AM, Mikael Gerdin wrote:
> Hi,
>
> After some discussions and some benchmarking I have a new version of 
> the change:
>
> http://cr.openjdk.java.net/~mgerdin/8009561/webrev.1/
>
> The idea is to use FreeBlockDictionary<Metablock>::atLeast but 
> limiting the splitting of large blocks by refusing the allocation if 
> the block returned from the freelist is 4x larger than the allocation 
> request.
>
> I also reduced the freelist allocation threshold since 64k seems too 
> large for what is in effect a per-ClassLoaderData freelist.
>
> /Mikael
>
> On 2013-06-05 16:04, Mikael Gerdin wrote:
>> Hi,
>>
>> Can I have some reviews of this small fix to the Metaspace memory
>> allocation path.
>>
>> Problem:
>> When a Metaspace allocation request cannot be satisfied by the current
>> chunk the chunk is retired and a new chunk is requested. This causes
>> whatever is left in the chunk to be effectively leaked.
>>
>> Suggested fix:
>> Put the remaining memory in each chunk on the Metablock freelist so it
>> can be used to satisfy future allocations.
>>
>> Possible addition:
>> When allocating from the block free list, use
>> FreeBlockDictionary<Metablock>::atLeast instead of
>> FreeBlockDictionary<Metablock>::exactly and split the Metablock if it's
>> large enough.
>>
>> One might argue that this increases the fragmentation of the memory on
>> the block free list but I think that we primarily want to use the block
>> free list for small allocations and allocate from chunks for large
>> allocations.
>>
>> Webrev:
>> Only fix:
>> http://cr.openjdk.java.net/~mgerdin/8009561/webrev.0/
>>
>> Incremental webrev for splitting blocks:
>> http://cr.openjdk.java.net/~mgerdin/8009561/webrev.0%2b/
>>
>> Bug links:
>> https://jbs.oracle.com/bugs/browse/JDK-8009561
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8009561
>>
>> Thanks
>> /Mikael