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

Mikael Gerdin mikael.gerdin at oracle.com
Thu Aug 29 05:30:27 PDT 2013


Jon,

Updated webrev at:
http://cr.openjdk.java.net/~mgerdin/8009561/webrev.3

I also noticed that in the code you mentioned:

  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   }

that the ">" check is incorrect. It should in fact be a ">=" since a 
block of min_size() is fine as well.

/Mikael

On 2013-08-29 11:11, Mikael Gerdin wrote:
> 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
>>


More information about the hotspot-gc-dev mailing list