Review request: 8025059: Metspace::should_expand mixes bytes and words in check against MaxMetaspaceSize

[Stefan Karlsson]

On 9/19/13 10:55 PM, Bengt Rutisson wrote:
> On 9/19/13 7:01 PM, Stefan Karlsson wrote:
>> On 9/19/13 5:10 PM, Bengt Rutisson wrote:
>>>
>>> Hi Stefan,
>>>
>>> Given that we just want to get the byte/word conversion right I 
>>> think this change is fine. The bug JDK-8024547 contains a long list 
>>> of improvements so it is a bit difficult to tell, but the asymmetry 
>>> with using reserved memory for NonClassType and allocated memory for 
>>> ClassType will go away with some of the changes for JDK-8024547, right?
>>>
>>> 1345     size_t nonclass_allocated = 
>>> MetaspaceAux::reserved_bytes(Metaspace::NonClassType);
>>> 1346     size_t class_allocated    = 
>>> MetaspaceAux::allocated_capacity_bytes(Metaspace::ClassType);
>>> 1347     size_t real_allocated     = nonclass_allocated + 
>>> real_allocated;
>>
>> Yes. The current code we're working on calls 
>> MetaspaceAux::committed_bytes() directly.
>
> OK. Sounds good. Then I'm fine with this change.

Thanks, Bengt.

StefanK
>
> Bengt
>
>
>>
>> StefanK
>>>
>>> Thanks,
>>> Bengt
>>>
>>>
>>> On 9/19/13 4:59 PM, Stefan Karlsson wrote:
>>>> http://cr.openjdk.java.net/~stefank/8025059/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8025059
>>>>
>>>> The following code in Metaspace::should_expand:
>>>>
>>>> if (!FLAG_IS_DEFAULT(MaxMetaspaceSize)) {
>>>>   size_t real_allocated = Metaspace::space_list()->reserved_words() +
>>>> MetaspaceAux::allocated_capacity_bytes(Metaspace::ClassType);
>>>>   if (real_allocated >= MaxMetaspaceSize) {
>>>>     return false;
>>>>   }
>>>>
>>>> doesn't convert Metaspace::space_list()->reserved_words() to bytes, 
>>>> as it should.
>>>>
>>>> Note that when JDK-8024547 gets fixed, this check will be rewritten 
>>>> to limit the amount of committed memory in both the Class and 
>>>> NonClass metaspaces. See:
>>>> https://bugs.openjdk.java.net/browse/JDK-8024547: MaxMetaspaceSize 
>>>> should limit the committed memory used by the metaspaces
>>>>
>>>> Testing: I've run our internal testing that sets the MaxMetaspaceSize.
>>>>
>>>> thanks,
>>>> StefanK
>>>
>>
>