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

[Mikael Gerdin]

Jon,

On 2013-08-16 15:59, Jon Masamitsu wrote:
> 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?

I think that would reduce the usefulness of the free list. It's 
unavoidable to waste a few words here and there due to alignment and 
such. If the allocation request is close enough in size I think we 
should take the chance to reuse the memory from the free list.

>
> 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   }

Sure, I'll fix that.

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

It was more of a hunch. 64k in the free list per 
Metaspace/CLD/ClassLoader seemed excessive. I can't say that I've done 
any scientific experiments to show that 4k is better but I think it's 
good enough.

/Mikael

>
> 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
>