Request for review (s) - 7198873

[Jon Masamitsu]

On 09/25/12 11:09, Mikael Gerdin wrote:
> ...
>>
>> The policy for CMS is
>>
>> 1) Hitting the HWM should start a concurrent collection if
>> CMS is doing class unloading.
>> 2) Always  expand the Metaspace and allocate from
>> the expanded space.
>> 3) If expanding the Metaspace does not provide any free
>> space, do a full GC to reclaim classloaders and class metadata
>> and then retry the allocation.
>
> But at what point does expanding the Metaspace not provide any free 
> space? Is there some sort of back-off so that we don't just go ahead 
> and allocate all available memory and then try to do a full gc when 
> we've filled up the address space?

I don't know what to do about this case.  We could change
CMSClassUnloadingEnabled to true by default so that
users would have to turn it off (at their own risk) if
they don't want to unload classes.  We could pick
some size for MaxMetaspaceSize (something appropriately
large) and again make the users change it (at their own
risk).  The latter isn't quite in line with the spirit of "removing
the need to turn perm gen" but maybe it is the lesser of
two evils.  I just don't know.

> I'm kind of concerned about the case with CMS without 
> CMSClassUnloadingEnabled. What I'm mainly worried about is slow leaks 
> and cases where an application loads a bunch of classes and then 
> releases the java level references to them but does not unload them 
> since we don't get to the expand_and_allocate returning null.

We're trading the VM shutting down for perm gen OOM for
the VM shutting down because out of address space.  I'm
beginning to think we should have a reasonably large
default value for MaxMetaspaceSize.


Jon
>
> /Mikael
>
>>
>> Jon
>>
>>
>> On 09/25/12 07:23, Mikael Gerdin wrote:
>>> Jon,
>>>
>>> On 2012-09-24 23:46, Jon Masamitsu wrote:
>>>> NPG: VM Does not unload classes with UseConcMarkSweepGC
>>>>
>>>> If CMS is not doing class unloading, don't start a concurrent
>>>> collection for classloader (and metadata) collection (since
>>>> it won't happen without class unloading).
>>>
>>> It looks like you still unconditionally call expand_and_allocate when
>>> running with CMS, no matter the value of CMSClassUnloadingEnabled.
>>> I think that the code:
>>>
>>>  213       // For CMS expand since the collection is going to be
>>> concurrent.
>>>  214       _result =
>>>  215 _loader_data->metaspace_non_null()->expand_and_allocate(_size,
>>> _mdtype);
>>>
>>> Should be inside the "if (CMSClassUnloadingEnabled)" and if running
>>> without it set then CMS users will have to take the hit of a stw full
>>> gc when running into the metadata threshold.
>>>
>>> /Mikael
>>>
>>>>
>>>> http://cr.openjdk.java.net/~jmasa/7198873/webrev.00/
>>>>
>>>> Also, refactored the code for readability and guarded extra
>>>> output with Verbose.
>>>>
>>>> Thanks.
>>>>
>>>> Jon
>>>
>