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

[Daniel D. Daugherty]

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