Request for review (s) - 7198873

[Mikael Gerdin]

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