RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC

[Igor Ignatyev]

+
>> 44-45,48-49:
>> According to section 9 in java code conventions[1]:
>>> The names of variables declared class constants and of ANSI constants
>>> should be all uppercase with words separated by underscores ("_").
>>> (ANSI constants should be avoided, for ease of debugging.)
>>
>> [1]
>> http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html
for 43-44


Igor

On 03/28/2014 05:48 PM, Igor Ignatyev wrote:
> Andrey,
>
>>> And as I said in 8037925:
>>>> You don't need to use enum here and string constant (Labels.*) in
>>>> TestDynShrinkHeap. I've mistakenly think that MemoryUsagePrinter uses
>>>> String argument to change its own behavior. Sorry for useless extra
>>>> work.
> Please remove class 'Labels', it's unnecessary and just decrease
> readability.
>
>>>>>   26  * @key gc
>>>> why do you need this?
>>> Hm, used as common practice.
> if you don't understand meaning of '@key gc', please don't use it.
>
> otherwise it looks good.
>
> Igor
>
> On 03/28/2014 05:37 PM, Andrey Zakharov wrote:
>> Hi, here is updated webrev:
>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.04/
>> Thanks.
>>
>> On 28.03.2014 02:29, Igor Ignatyev wrote:
>>> Andrey,
>>> you've placed the wrong link, the correct one is
>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.02/.
>>> And as I said in 8037925:
>>>> You don't need to use enum here and string constant (Labels.*) in
>>>> TestDynShrinkHeap. I've mistakenly think that MemoryUsagePrinter uses
>>>> String argument to change its own behavior. Sorry for useless extra
>>>> work.
>>>
>>> MemoryUsagePrinter:
>>>>   44         float freeratio = 1f - (float) memusage.getUsed() /
>>>> memusage.getCommitted();
>>> I'd prefer '1.0f', but it's only my fad, it's not necessary to change.
>>>
>>> TestDynShrinkHeap:
>>>>   96             StringBuilder strb = new StringBuilder("committed
>>>> heap size under pressure is not less than committed full heap size,
>>>> heap hasn't been shrunk?");
>>>>   97 strb.append(System.getProperty("line.separator"));
>>>>   98             strb.append(MinFreeRatioFlagName + " = " +
>>>> minHeapFreeValue);
>>>>   99 strb.append(System.getProperty("line.separator"));
>>>>  100             strb.append(MaxFreeRatioFlagName + " = " +
>>>> maxHeapFreeValue);
>>>>  101             throw new RuntimeException(strb.toString());
>>> it can be replaced by:
>>>  throw new RuntimeException(String.format("committed heap size under
>>> pressure is not less than committed full heap size, heap hasn't been
>>> shrunk?%n%s=%d%n%s=%n",MinFreeRatioFlagName,minHeapFreeValue,MaxFreeRatioFlagName,maxHeapFreeValue));
>>>
>>>
>>> but it's also not necessary.
>>>
>>> Thanks
>>> Igor
>>>
>>> On 03/27/2014 04:42 PM, Andrey Zakharov wrote:
>>>> Here is updated webrev with string constants:
>>>> http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.02/
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> On 26.03.2014 20:02, Andrey Zakharov wrote:
>>>>> Test to check that ParallelGC respect dynamic change of MaxFreeRatio
>>>>> and shrinks heap.
>>>>> webrev: http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037924
>>>>>
>>>>> Thanks.
>>>>>
>>>>
>>