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

[Mikael Gerdin]

Stefan,

thanks for looking at this.

On 2013-09-10 14:03, Stefan Karlsson wrote:
> On 09/10/2013 10:25 AM, Mikael Gerdin wrote:
>> Hi,
>>
>> can I have a second review for this change?
>>
>> The latest webrev is at:
>> http://cr.openjdk.java.net/~mgerdin/8009561/webrev.3/
>
> http://cr.openjdk.java.net/~mgerdin/8009561/webrev.3/src/share/vm/memory/metaspace.cpp.frames.html
>
>
> This should be using >= instead of >:

Yes, fixed.

>
> 2330 void SpaceManager::retire_current_chunk() {
> 2331   if (current_chunk() != NULL) {
> 2332     size_t remaining_words = current_chunk()->free_word_size();
> 2333     if (remaining_words > TreeChunk<Metablock,
> FreeList>::min_size()) {
> 2334
> block_freelists()->return_block(current_chunk()->allocate(remaining_words),
> remaining_words);
> 2335       inc_used_metrics(remaining_words);
> 2336     }
> 2337   }
> 2338 }
>
> otherwise this looks good to me.
>
> If you're going to edit this patch, maybe you could split this line into
> two and capitalize the start of the sentence?
>
>   228   // only allocate and split from freelist if the size of the
> allocation is at least 1/4th the size of the available block
>   229   const static int WasteMultiplier = 4;

Done.

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

>
>
> thanks,
> StefanK
>
>>
>>
>> /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
>