Request for review: 8020530 Non heap memory size calculated incorrectly

[Vladimir Kempik]

Hello everyone, do you think this patch could be applicable to jdk7 and 
jdk6 as well ?

There is little difference between jdk7 and jdk8 here, two asserts were 
removed from jdk8. asserts shouldn't be triggered in product build and 
patch wouldn't do any harm anyway.

   // if any one of the memory pool has undefined init_size or max_size,
   // set it to -1
   if (has_undefined_init_size) {
     total_init = (size_t)-1;
   }
   if (has_undefined_max_size) {
     total_max = (size_t)-1;
   }

   // In our current implementation, we make sure that all non-heap
   // pools have defined init and max sizes. Heap pools do not matter,
   // as we never use total_init and total_max for them.
   assert(heap || !has_undefined_init_size, "Undefined init size");
   assert(heap || !has_undefined_max_size,  "Undefined max size");

   MemoryUsage usage((heap ? InitialHeapSize : total_init),
                     total_used,
                     total_committed,
                     (heap ? Universe::heap()->max_capacity() : total_max));

Thanks Vladimir.

On 21.08.2013 18:37, Coleen Phillimore wrote:
>
> Looks good, thanks!
> Coleen
>
> On 8/21/2013 6:40 AM, Vladimir Kempik wrote:
>> Thanks everyone for comments.
>>
>> Here is updated webrev 
>> http://cr.openjdk.java.net/~vkempik/8020530/webrev.01/
>>
>> Vladimir.
>> On 21.08.2013 1:52, Mandy Chung wrote:
>>>
>>> On 8/20/2013 5:38 AM, Staffan Larsen wrote:
>>>> That doesn't make a lot of sense to me. Why would a pool have 
>>>> undefined values?
>>>> The Metaspace pool has no max value (unless you specify 
>>>> -XX:MaxMetaspaceSize=), thus undefined.
>>>>
>>>>> If a subset of pools have undefined values why report completely 
>>>>> fallacious values of -1?
>>>> The javadoc for MemoryUsage says getMax() returns -1 if the maximum 
>>>> memory size is undefined.
>>>
>>> Yes the spec allows implementation of memory pools with undefined 
>>> max.  "used" and "committed" must have a value and the "committed" 
>>> memory is guaranteed to be available for the VM to use.  "max" will 
>>> give an idea of the upper bound how much memory can be allocated 
>>> from it; however, there is no guarantee that amount of memory is 
>>> available for the VM.
>>>
>>>>> It also isn't clear how this relates to the "committed" value in 
>>>>> the failure. What gets reported now?
>>>> I guess there can still be a committed value even if we don't have 
>>>> a max value for how much we might commit in the future: used <= 
>>>> committed <= max.
>>>
>>> The MemoryUsage constructor throws IAE if committed > max if max is 
>>> defined.  Perhaps it would be better if max should be Long.MAX_VALUE 
>>> if undefined (a different issue than this bug).
>>>
>>> Mandy
>>>
>>>> /Staffan
>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>
>>
>