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

[Coleen Phillimore]

Ok, then.  Looks good.
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
>>>
>>
>