Request for review (s) - 7198873

[Mikael Gerdin]

Jon,

On 2012-09-26 18:44, Jon Masamitsu wrote:
> Mikael,
>
> Do you want a default value (less than max uint) for MaxMetaspaceSize
> before
> this change goes back?

While I see a value in setting some sort of "sensible" value for 
MaxMetaspaceSize this is not really the exact issue I was worried about.

Consider this scenario:

* Person runs a Java app server/container with +UseConcMarkSweepGC (but 
without +CMSClassUnloadingEnabled).
* Some app is repeatedly deployed and un-deployed from the instance, 
every time an app is un-deployed its class loader and classes are made 
unreachable.

In this case CMS would not opt to unload classes until MaxMetaspaceSize 
is hit or the process/system runs out of memory.

Maybe there could be some sort of threshold for when to trigger a full 
collection (which includes class unloading) without that threshold value 
being a limit on the amount of memory available for meta data?

I know this is a complicated policy issue, and I agree that it would be 
good if we could enable +CMSClassUnloadingEnabled and get rid of the 
issue altogether. The question is whether or not performance testing 
resources are available to evaluate that.

/Mikael

>
> Jon
>
> On 9/25/2012 11:09 AM, Mikael Gerdin wrote:
>> On 2012-09-25 18:23, Jon Masamitsu wrote:
>>> Mikael,
>>>
>>> Thanks for the review.
>>>
>>> The expand_and_allocate() does not do a GC.  It expands
>>> the Metaspace and does an allocation from the expanded
>>> space.  Only if that fails does the CMS case fall through to
>>> the full GC.
>>
>> Yes.
>>
>>>
>>> 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'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.
>>
>> /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
>>>>
>>

-- 
Mikael Gerdin
Java SE VM SQE Stockholm