RFR(S): JDK-8019845 NPG: Memory leak during class redefinition

[frederic parain]

Thank you Mikael, Karen, Jon, Coleen and Dan for
your reviews.

Fred

On 26/07/2013 15:58, Daniel D. Daugherty wrote:
> Fred,
>
> Thanks for the wonderful answers. I'm good with this change.
>
> Sounds like the allocate() size and deallocate() size have to
> match. The Metaspace folks should consider adding an assert()
> or a guarantee() for this...
>
> Dan
>
>
> On 7/26/13 4:12 AM, frederic parain wrote:
>> Dan,
>>
>> My answers are in-lined below.
>>
>> On 26/07/2013 00:21, Daniel D. Daugherty wrote:
>>> On 7/25/13 9:15 AM, frederic parain wrote:
>>>> Greetings,
>>>>
>>>> Please review this small fix:
>>>>
>>>> open webrev: http://cr.openjdk.java.net/~fparain/8019845/webrev.00/
>>>> bug link: http://bugs.sun.com/view_bug.do?bug_id=8019845
>>>
>>> src/share/vm/memory/metaspace.cpp
>>>      OK, I see how deallocate() size has to match the allocate() size
>>>      if we have to deallocate() exactly what was allocated.
>>>
>>>      But I'm a bit confused about how we managed to lose so much memory
>>>      since the delta between word_size and raw_word_size should be very
>>>      small. Unless the size mis-match caused us to lose the whole block.
>>
>>        In case of a size mismatch, the whole block is lost.
>>        For the particular case of RedefineBigClass.sh, most of the leak
>>        comes from the allocation of a 6618 words long block. Each time
>>        the big class is redefined, such a block is needed. But those
>>        blocks are returned as 6617 words long blocks. So for each
>>        redefinition, the old block is put in the free list (and not
>>        recycled or freed) and a new block is allocated. The test
>>        redefined the big class 1000 times, so with the leak, it
>>        requires a total memory space of 6618 * 1001 words just for this
>>        particular MetaspaceObject. Without the leak, only 6618 * 2 words
>>        are required.
>>
>>>      I'm really confused about why this only reproduced on Linux X64 and
>>>      not on Linux X86.
>>
>>        The difference from the raw_word_size and the word_size comes
>>        from the alignment constraints, which are not the same on 32bits
>>        and 64bits.
>>
>>>      With a size parameter on a deallocate() call, that implies that the
>>>      allocate/deallocate subsystem supports deallocating a different
>>> size
>>>      than what was originally allocated. Something like:
>>>
>>>      - allocate 2MB as a worst case
>>>      - read data in the buffer
>>>      - done reading data and only used 1MB so
>>>        deallocate 1MB of the buffer
>>
>>        No, I didn't see any support for partial deallocation. The
>>        MetaspaceObject is deallocated as a whole.
>>
>>>      If the allocate/deallocate subsystem doesn't support partial
>>> deallocates
>>>      then why does deallocate() have a size parameter? If you always
>>> have to
>>>      deallocate() exactly what you allocate()'ed, then deallocate() only
>>> needs
>>>      the pointer originally returned by allocate() and doesn't need the
>>> size
>>>      parameter.
>>
>> I'm not familiar enough with Metaspace to answer this questions. I'll
>> let the NPG experts respond to this one.
>>
>> Thanks,
>>
>> Fred
>>
>>>>
>>>> The bug title is "NPG: Memory leak during class redefinition" but
>>>> the leak is in the metaspace code.
>>>>
>>>> To store metadata, the VM allocates memory chunks using
>>>> SpaceManager::allocate(). In this method, a "raw_word_size"
>>>> is computed from the MetaspaceObject size and some
>>>> alignment constraints to determine the size of the memory
>>>> chunk needed. A look up is performed in the free list to
>>>> find a chunk of this size, if none is found, a new chunk
>>>> is allocated.
>>>>
>>>> When a memory chunk is not used anymore, for instance when
>>>> a class is redefined and the old metadata are discarded, it
>>>> is returned to the free list.
>>>>
>>>> The problem is that the memory chunk is returned to the free
>>>> list using its MetaspaceObject size without considering alignment
>>>> constraints. If the "raw_word_size" computed in
>>>> SpaceManager::allocate() is different from the MetaspaceObject
>>>> size, look ups in the free list cannot find returned chunks
>>>> because of the size difference, causing the leak.
>>>>
>>>> The fix is to return memory chunks using the same size
>>>> computation as the one used in SpaceManager::allocate().
>>>>
>>>> Tested with:
>>>>   RedefineBigClass.sh output (checking the memory leak
>>>>   doesn't show up anymore)
>>>>   JPRT
>>>>   vm.quick.testlist
>>>>   vm.runtime.testlist
>>>>   vm.parallel_class_loading.testlist
>>>>
>>>> Thanks,
>>>>
>>>> Fred
>>>>
>>>
>>
>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com