Request for review - 7197557

[Jon Masamitsu]

Ramki,

Thanks for taking at look.

On 9/17/2012 3:53 PM, Srinivas Ramakrishna wrote:
> Hi Jon --
>
> While I am not familiar with all of the details of the new metaspaces
> implementation,
> from my high level knowledge of how it works, the shape of the code changes
> here to address the bug looks good to me.
> (Although it would have been nice if one could have read the stack retrace
> of the JVM when the deadlock occurred, i
> think such information was not in the public part of the bug report visible
> on bugs.sun.com.)

This is the top of the stack trace of the thread doing the GC.  It is 
actually a live lock where
the GC has already entered the infinite loop that keeps trying to do a 
GC (checks the
results of the gc_prologue) until the GC succeeds.  We needed to check 
for an active
GC_locker.  A thread trying to release a JNI critical section was 
blocking on a
safepoint.

=>[1] GC_locker::is_active_internal(), line 88 in "gcLocker.hpp"
   [2] GC_locker::is_active_and_needs_gc(), line 104 in "gcLocker.hpp"
   [3] VM_GC_Operation::skip_operation(this = 0xfffffd7fec9d5358), line 
92 in "vm
GCOperations.cpp"
   [4] VM_GC_Operation::doit_prologue(this = 0xfffffd7fec9d5358), line 
111 in "vm
GCOperations.cpp"
   [5] VMThread::execute(op = 0xfffffd7fec9d5358), line 587 in 
"vmThread.cpp"
   [6] CollectorPolicy::satisfy_failed_metadata_allocation(this = 
0x432558, loade
r_data = 0x52a62f8, word_size = 0x3a99U, mdtype = NonClassType), line 
765 in "co
llectorPolicy.cpp"
   [7] Metaspace::allocate(loader_data = 0x52a62f8, word_size = 0x3a99U, 
read_onl
y = true, mdtype = NonClassType, __the_thread__ = 0x348d800), line 2953 
in "meta
space.cpp"
   [8] Array<unsigned short>::operator new(size = 0x8U, loader_data = 
0x52a62f8,
length = 0xea60, read_only = true, __the_thread__ = 0x348d800), line 322 
in "arr
ay.hpp"
   [9] MetadataFactory::new_array<unsigned short>(loader_data = 
0x52a62f8, length
  = 0xea60, __the_thread__ = 0x348d800), line 38 in "metadataFactory.hpp"

> A somewhat orthogonal question:
> Could you tell me if there is any a-priori limit that the JVM sets on the
> c-heap space used for the metadata?

No limit.  The space is actually not C-heap space.  It is allocated 
using Virtualspaces which
means mmap'ed space on unix systems.  You can set a limit on the command 
line with
MaxMetaspaceSize which is analogous to MaxPermSize.

> If yes, can that limit be changed from the command-line? If there is no
> such a-priori limit, could you shed any light
> on a comparison of the memory footprint between the pre-NPG world and the
> new post-NPG world for
> some benchmarks that exercise class load/unload etc.?

We do induce GC to do class unloading.  We have a high water mark (HWM) 
for class metadata
used.  When the used class metadata hits the HWM, we induce a GC.  The 
initial value of the
HWM is 12Mbytes to 16Mbytes depending on the platform  The HWM may be 
increased after the
GC depending on how much free metadata space there is.  Think 
MinFreeRation / MaxFreeRatio
kind of policy.

For small benchmarks that don't do extensive class loading, the 
footprint may look
less because we don't reserve the space for the perm gen.  In general it 
is comparable.
We do loose some space to fragmentation and need to do more tuning in that
area.

Jon
> thanks!
> -- ramki
>
> On Mon, Sep 17, 2012 at 3:18 PM, Jon Masamitsu<jon.masamitsu at oracle.com>wrote:
>
>> I have one review (Thanks JohnCu).  The change is straight forward
>> but it is code that has problems in the past (GC_locker code) so any
>> other reviews would be welcome.
>>
>> Jon
>>
>>
>> On 09/16/12 20:09, Jon Masamitsu wrote:
>>
>>> NPG: nsk/sysdict/vm/stress/chain/**chain004 hangs intermittently
>>>
>>> The code that was doing a GC to find dead class loaders so that metadata
>>> could be freed does not correctly account for GC_locker activity (i.e.,
>>> use
>>> of JNI critical sections which stall GC).  Added code to recognize if
>>> a GC_locker is active and expand the Metaspace and allocate out of the
>>> expanded area.  If the expansion cannot provide metadata, wait for
>>> the GC_locker to inactivate so that a GC can be done.
>>>
>>> http://cr.openjdk.java.net/~**jmasa/7197557/webrev.00/<http://cr.openjdk.java.net/%7Ejmasa/7197557/webrev.00/>
>>>
>>> Note that there are three places in the different GC's that need to
>>> account for GC_locker activity.  I am investigating whether this
>>> code can be unified.
>>>