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

[Mikael Gerdin]

On 2013-03-11 19:33, Jon Masamitsu wrote:
> 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.

Thanks for the review Jon. I agree with you that we should revisit this 
at some point.

I need another review on this, any takers?

/Mikael

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