Request for review (s) - 7198873

[Jon Masamitsu]

On 9/25/2012 2:32 PM, Srinivas Ramakrishna wrote:
> I think that it's definitely time to set cms class unloading to be true by default :-)
> In my recent experience the impact of that on remark pauses has been very small... Of course I haven't run w/NPG so don't know how that changes the perf eqn ... Worth checking .. May be already done, Jon et al.?

I'll run something and see but maybe not this week.

> However, even then we don't want CMS or G1 to do a full gc because meatspace could've expanded but was reluctant. I agree w/Jon that making the default max size for metaspace large reasonable value based on say the heap size max, might be appropriate.

You might be right that something related to the max size of the heap 
would be
appropriate but I'm not sure it's worth having the value vary from platform
to platform (just because I have to ask how much physical memory was
on the system).

Jon
> --Ramki
>
> ysr1729
>
> On Sep 25, 2012, at 13:11, Jon Masamitsu<jon.masamitsu at oracle.com>  wrote:
>
>>
>> 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