RFR 8059557: Test set for "Validate JVM Command-Line Flag Arguments"
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Wed May 27 16:57:12 UTC 2015
Hello David,
Thank you for reviewing the code! Please see my comments inline.
On 27.05.2015 4:18, David Holmes wrote:
> Hi Dmitry,
>
> Just browsing through ...
>
> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOptionsUtils.java
>
>
> 60 if (name.startsWith("CMS")) {
> 61 option.addPrepend("-XX:+UseConcMarkSweepGC");
> 62 }
>
> Not all CMS related options start with CMS. Is this just looking for
> the particular subset of options that do start with CMS? Is starting
> with CMS or starting with G1 significant with regards to the type of
> option?
The intention for this function and addTypeDependency is following: It
is good if option is used somehow during start-up because this can help
to catch possible problems(crash of VM,hangs) when option have min/max
values or other valid values(withing range). Thus I add several
dependencies. This approach are quite rough, so I add
"-XX:+UseConcMarkSweepGC" for options which start with "CMS" and
"-XX:+UseG1GC" for options which start with "G1".
>
> 64 switch (name) {
> 65 case "MinHeapFreeRatio":
> 66 option.addPrepend("-XX:MaxHeapFreeRatio=100");
> 67 break;
> 68 case "MaxHeapFreeRatio":
> 69 option.addPrepend("-XX:MinHeapFreeRatio=0");
> 70 break;
>
> It isn't at all clear why these options have to have special
> treatment, or how the prepended option relates to the option under test?
The intention the same as above: these options have constraints and to
avoid violating of constraint during testing valid values I add these
dependencies. This is only to help to catch possible problems like
crash. VM can exit with error for valid value(in range) and this not
considered as fail, because it can not continue for other reasons(e.g.
not enough memory). So, adding these dependencies is needed to bypass
constraints check and successfully start up VM.
>
> 106 static private void addTypeDependency(JVMOption option, String
> type) {
> 107 if (type.contains("C1") || type.contains("C2")) {
> 108 /* Run in compiler mode for compiler flags */
> 109 option.addPrepend("-Xcomp");
> 110 }
> 111 }
>
> Don't you need to ensure -client or -server to deal with C1 and C2
> options?
The intention the same as above: it is good if option is used during
start up and thus I ran VM in compiler mode for compiler options. I not
add explicit "client" or "server" because I verify options with the same
VM arguments as test. But actually "client" or "server" can be added
explicitly.
>
> ---
>
> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/IntJVMOption.java
>
>
> 58 * Is this value is signed or unsigned
> 63 * Is this value is 64 bit unsigned
>
> delete 'is'
Fixed.
>
> I'm confused by the MIN_LONG/MAX_LONG etc variables. Are these
> supposed to be representing C-types? We don't use C "long" as a rule.
Probably name of these variables seems confusing. MIN_LONG/MAX_LONG are
needed to hold minumum/maximum value for intx HotSpot type. For 32 bit
Java it equals to min/max for 32 bit long and for 64 bit Java it equals
to min/max for 64 bit long. This is needed to determine that option have
minumum/maximum allowed value for it's type and not pass value equal to
"min-1" or "max+1".
>
> Also unclear why you need to use BigInteger here instead of just Java
> long for everything ??
My first approach was to use Java long, but unfortunately Java long is
not enough to hold values with type uintx and similar types on 64 bit
systems. On 64 bit systems uintx can have value greater than
9223372036854775807 and therefore I use BigInteger.
>
> ---
>
> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/DoubleJVMOption.java
>
>
> 31 private final double MAX_DOUBLE = 18446744073709551616.000;
>
> where does this magic number come from? (And why 3 zeroes after the
> decimal point ??)
>
It was from one double option which have range, but this construction
seems not correct now. Thank you for pointing this. I will rewrite this
logic.
Regards,
Dmitry
> Thanks,
> David
>
> On 22/05/2015 6:57 AM, Dmitry Dmitriev wrote:
>> Hello all,
>>
>> Recently I correct several typos, so here a new webrev for tests:
>> http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.02/
>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.02/>
>>
>> Thanks,
>> Dmitry
>>
>> On 18.05.2015 18:48, Dmitry Dmitriev wrote:
>>> Hello all,
>>>
>>> Please review test set for verifying functionality implemented by JEP
>>> 245 "Validate JVM Command-Line Flag Arguments"(JDK-8059557). Review
>>> request for this JEP can be found there:
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-May/018539.html
>>>
>>> I create 3 tests for verifying options with ranges. The tests mostly
>>> rely on common/optionsvalidation/JVMOptionsUtils.java. Class in this
>>> file contains functions to get options with ranges as list(by parsing
>>> new option "-XX:+PrintFlagsRanges" output), run command line test for
>>> list of options and other. The actual test code contained in
>>> common/optionsvalidation/JVMOption.java file - testCommandLine(),
>>> testDynamic(), testJcmd() and testAttach() methods.
>>> common/optionsvalidation/IntJVMOption.java and
>>> common/optionsvalidation/DoubleJVMOption.java source files contain
>>> classes derived from JVMOption class for integer and double JVM
>>> options correspondingly.
>>>
>>> Here are description of the tests:
>>> 1)
>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java
>>>
>>>
>>>
>>> This test get all options with ranges by parsing output of new option
>>> "-XX:+PrintFlagsRanges" and verify these options by starting Java and
>>> passing options in command line with valid and invalid values.
>>> Currently it verifies about 106 options which have ranges.
>>> Invalid values are values which out-of-range. In test used values
>>> "min-1" and "max+1".In this case Java should always exit with code 1
>>> and print error message about out-of-range value(with one exception,
>>> if option is unsigned and passing negative value, then out-of-range
>>> error message is not printed because error occurred earlier).
>>> Valid values are values in range, e.g. min&max and also several
>>> additional values. In this case Java should successfully exit(exit
>>> code 0) or exit with error code 1 for other reasons(low memory with
>>> certain option value etc.). In any case for values in range Java
>>> should not print messages about out of range value.
>>> In any case Java should not crash.
>>> This test excluded from JPRT because it takes long time to execute and
>>> also fails - some options with value in valid range cause Java to
>>> crash(bugs are submitted).
>>>
>>> 2)
>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java
>>>
>>>
>>>
>>> This test get all writeable options with ranges by parsing output of
>>> new option "-XX:+PrintFlagsRanges" and verify these options by
>>> dynamically changing it's values to the valid and invalid values. Used
>>> 3 methods for that: DynamicVMOption isValidValue and isInvalidValue
>>> methods, Jcmd and by attach method. Currently 3 writeable options with
>>> ranges are verified by this test.
>>> This test pass in JPRT.
>>>
>>> 3)
>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestJcmdOutput.java
>>>
>>> This test verified output of Jcmd when out-of-range value is set to
>>> the writeable option or value violates option constraint. Also this
>>> test verify that jcmd not write error message to the target process.
>>> This test pass in JPRT.
>>>
>>>
>>> I am not write special tests for constraints for this JEP because
>>> there are exist test for that(e.g.
>>> test/runtime/CompressedOops/ObjectAlignment.java for
>>> ObjectAlignmentInBytes or
>>> hotspot/test/gc/arguments/TestHeapFreeRatio.java for
>>> MinHeapFreeRatio/MaxHeapFreeRatio).
>>>
>>> Webrev: http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.00/>
>>>
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8059557
>>>
>>> Thanks,
>>> Dmitry
>>>
>>
More information about the hotspot-dev
mailing list