Review request (S) JDK-8004241 NPG: Metaspace occupies more memory than specified by -XX:MaxMetaspaceSize option

[Jon Masamitsu]

Mikael,

This change looks good.   I looked over your replies
and am OK with them.  At some point we may need to
limit the space in the "last" Virtualspace so that the total
reserved is at MaxMetaspaceSize, but that is a
different change.

Thanks.

Jon


On 03/11/13 03:11, Mikael Gerdin wrote:
> Jon,
>
> On 2013-03-07 21:57, Jon Masamitsu wrote:
>>
>>
>> On 03/07/13 00:54, Mikael Gerdin wrote:
>>> Hi
>>>
>>>
>>> When deciding when to reserve more metaspace memory we erroneously
>>> looked only at the "capacity" of the metaspace insted of the reserved
>>> space (which is what we ask this function when expanding).
>>
>> Using MetaspaceAux::reserved_in_bytes() means that we
>> could return false here
>>
>> 1105   if (!FLAG_IS_DEFAULT(MaxMetaspaceSize)&&
>> 1106       MetaspaceAux::reserved_in_bytes()>= MaxMetaspaceSize) {
>> 1107     return false;
>> 1108   }
>>
>> when most of the space reserved in one or two  VirtualSpace's is unused.
>> With the current value of parameters, that could almost be 512kb.
>
> Yes.
>
> I guess the question is:
> Should MaxMetaspaceSize limit the amount of virtual address space 
> reserved for metaspaces or should it limit the amount of committed pages?
>
> To be consistent with how the Java heap is handled my opinion is that 
> we should limit the amount of reserved memory.
>
> I any case the current version is incorrect since 
> MetaspaceGC::should_expand was queried to determine if we should 
> reserve more virtual address space or not and the size check was 
> against the amount of committed memory.
>
> /Mikael
>
>>
>> Jon
>>
>>>
>>> Additionally, we didn't check MaxMetaspaceSize against the sum of
>>> reserved(Class) + reserved(NonClass) which caused us to use more than
>>> MaxMetaspaceSize even when it was set.
>>>
>>> Bug:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004241
>>> (not yet available at the time of writing this mail)
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mgerdin/8004241/webrev.0
>>>
>>> Testing:
>>> JPRT with -XX:MaxMetaspaceSize set for all tests to exercise the code
>>> path.