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

[Stefan Karlsson]

On 9/19/13 7:27 PM, Coleen Phillimore wrote:
>
> Ok, then.  Looks good.

Thanks, Coleen.

StefanK
>
> Coleen
>
> On 09/19/2013 01:09 PM, Stefan Karlsson wrote:
>> On 9/19/13 5:22 PM, Coleen Phillimore wrote:
>>>
>>> Yes, I'm confused why you're fixing this now and not with the change 
>>> that really fixes it to be consistent.  Does it enable more tests to 
>>> run?
>>
>> The main reason is that I'm not the only one working on JDK-8024547, 
>> and pushing some of my patches out to the hotspot-gc will make it 
>> easier for the others to help out with some of the listed issues.
>>
>> StefanK
>>
>>> Thanks,
>>> Coleen
>>>
>>> On 09/19/2013 11:10 AM, 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;
>>>>
>>>> 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
>>>>
>>>
>>
>