RFR: tests for JDK-8028720 Make the Min/MaxHeapFreeRatio flags manageable

Igor Ignatyev igor.ignatyev at oracle.com
Wed Mar 5 09:12:28 UTC 2014


Hi Andrey,

0. In all your tests you have:
>   27  * @key gc
it's redundant, since the tests are already in gc directory. please 
remove it.
>   28  * @bug JDK-8028391
according to convention, you have to use '8028391' instead of 'JDK-8028391'.
$ grep -R "@bug" ./hotspot/test/ | wc -l
434
$ grep -R "@bug" ./hotspot/test/ | grep -c JDK
0

1. VMOptionValueChecker has checkInvalidValue and checkValid, please 
normalize them:
   checkInvalidValue/checkValidValue
   or
   checkInvalid/checkValid

2. I don't like that 'VMOptionValueChecker.getIntWritableValue' method 
do two things, could you please replace it w/ two methods:
VMOptionValueChecker.getIntValue(name)
VMOptionValueChecker.checkIsWriteable(name)

3. You use management packages, but not all JDK/JRE bundles have it, so 
you have to add your tests in some group (needs_compact3, I think). LevP 
should have more information on it.

Lev, could you please help us here? com.sun.management.* and 
java.lang.management.* are available only in compact3, aren't they?

4. your tests look similar, can you create a super class for both tests? 
we will be able to reuse it in other tests for writable flags

> class TestDynIntVMFlags  {
> String name;
> int originalValue;
> TestDynIntVMFlags(name) {
>   name = name;
>   int originalValue = getIntValue(name)
>   println(name + " = " + originalValue);
> }
>
> void test() {
>       checkIsWriteable(name);
>       checkInvalidValue(name, "" + Integer.MIN_VALUE);
>       checkInvalidValue(name, "" + Integer.MAX_VALUE);
>       checkInvalidValue(name, "-10");
>       checkInvalidValue(name, "190");
>       checkValid(name, "0");
>       extraTest();
> }
> void extraTest() { }
> }
> //// TestDynMinHeapFreeRatio.java
> TestDynMinHeapFreeRatio extends TestDynIntVMFlags {
> private TestDynMinHeapFreeRatio() { super("MinHeapFreeRatio"); }
>   String MaxFreeRatioFlagName = "MaxHeapFreeRatio";
>   void main() {
>     new TestDynMinHeapFreeRatio().test();
>   }
>   void extraTest() {
>     int maxHeapFreeValue = getIntValue(MaxFreeRatioFlagName);
>     checkInvalidValue(name, "" + (maxHeapFreeValue + 1));
>     checkValid(name, "" + (maxHeapFreeValue));
>   }
> }//// TestDynMaxHeapFreeRatio.java
> TestDynMaxHeapFreeRatio extends TestDynIntVMFlags {
> TestDynMaxHeapFreeRatio() { super("MaxHeapFreeRatio"); }
>   String MaxFreeRatioFlagName = "MinHeapFreeRatio";
>   void main() {
>     new TestDynMaxHeapFreeRatio().test();
>   }
>   void extraTest() {
>     int minHeapFreeValue = getIntValue(MinFreeRatioFlagName);
>     checkInvalidValue(name, "" + (minHeapFreeValue - 1));
>     checkValid(name, "" + (minHeapFreeValue));
>   }
> }

Thanks,
Igor

On 02/20/2014 10:09 PM, Thomas Schatzl wrote:
> Hi,
>
> On Thu, 2014-02-20 at 15:15 +0400, Andrey Zakharov wrote:
>> Any comments are welcome!
>> Thanks.
>>
>> On 14.02.2014 21:13, Jesper Wilhelmsson wrote:
>>> Looks good.
>>> /Jesper
>>>
>>> Andrey Zakharov skrev 14/2/14 16:33:
>>>> Simple tests to check that MaxHeapFreeRatio and MinHeapFreeRatio
>>>> flags are
>>>> writable and managable in proper ways.
>>>> webrev: http://cr.openjdk.java.net/~jwilhelm/8028391/webrev.test.02/
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8028720
>>>>
>>>> Thanks.
>>
>
>    some typos: "managable" -> "manageable"
>
> Otherwise looks good.
>
> Thomas
>
>



More information about the hotspot-gc-dev mailing list