RFR 8134995(M): [REDO] GC: implement ranges (optionally constraints) for those flags that have them missing

[Jon Masamitsu]

On 10/01/2015 12:10 PM, sangheon.kim wrote:
> Hi Jon,
>
> On 10/01/2015 11:57 AM, Jon Masamitsu wrote:
>> Sangheon,
>>
>> What was the failure that prompted this change?
> test/gc/g1/mixedgc/TestLogging.java
> And as I answered to Kim, this test is wrote correctly.
>
> Current constraint function is trying to verify below:
> MaxGCPauseMillis >= GCPauseIntervalMillis
>
> But if one of them is using default value, this comparison is not 
> valid as default value means set it ergonomically.
> This is why I added FLAG_IS_CMDLINE().

OK.  I understand better now.  Perhaps you could add a comment
such as this to the top of the file.

// Some flags that have default values that indicate that the
// JVM should automatically determine an appropriate value
// for that flag.  In those cases it is only appropriate for the
// constraint checking to be done if the user has specified the
// value(s) of the flag(s) on the command line.  In the constraint
// checking functions,  FLAG_IS_CMDLINE() is used to check if
// the flag has been set by the user and so should be checked.

Jon

>
> Sangheon
>
>
>> I
>> don't understand why the check for FLAG_IS_CMDLINE()
>> is needed.
>>
>> Jon
>>
>> On 10/01/2015 09:24 AM, sangheon.kim wrote:
>>> Hi all,
>>>
>>> During this code review, newly added gc test found a problem of 1 gc 
>>> constraint function(MaxGCPauseMillisConstraintFunc).
>>>
>>> So I updated it and related function 
>>> (GCPauseIntervalMillisConstraintFunc).
>>> - Added checking for MaxGCPauseMillis and GCPauseIntervalMillis 
>>> whether these are set via commandline or not to proceed the 
>>> constraint function logic.
>>>
>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.03
>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.03_to_02/
>>>
>>> * Testing: JPRT and RBT(to manually test runtime/CommandLine for 
>>> embedded)
>>>
>>> Thanks,
>>> Sangheon
>>>
>>>
>>> On 09/25/2015 03:52 PM, sangheon.kim wrote:
>>>> Hi Gerard,
>>>>
>>>> I found that I missed your comment from webrev.01.
>>>> Here's a new one which includes yours for TestOptionsWithRanges.java.
>>>>
>>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.02/
>>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.02_to_01
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>>
>>>> On 09/14/2015 07:23 AM, gerard ziemski wrote:
>>>>> Thank you. I have no more comments - reviewed.
>>>>>
>>>>>
>>>>> cheers
>>>>>
>>>>>
>>>>> On 09/12/2015 03:38 AM, sangheon.kim wrote:
>>>>>> Hi Gerard,
>>>>>>
>>>>>> On 09/11/2015 12:24 PM, sangheon.kim wrote:
>>>>>>> Hi Gerard,
>>>>>>>
>>>>>>> Thank you for looking at this.
>>>>>>>
>>>>>>> On 09/11/2015 11:13 AM, gerard ziemski wrote:
>>>>>>>> hi Sangheon,
>>>>>>>>
>>>>>>>> #1 
>>>>>>>> test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
>>>>>>>>
>>>>>>>>
>>>>>>>> Please change the comment to:
>>>>>>>>
>>>>>>>> +        /*
>>>>>>>> +         * Exclude below options as their maximum value would 
>>>>>>>> consume too much memory
>>>>>>>> +         * and would affect other tests that run in parallel.
>>>>>>>> +         */
>>>>>>> Okay, I will fix as you suggested.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> #2 What tests did you run? Did you run 
>>>>>>>> test/runtime/CommandLine/OptionsValidation on all platforms 
>>>>>>>> (including embedded)?
>>>>>>> No.
>>>>>>> I ran tests under test/runtime/CommandLine/OptionsValidation 
>>>>>>> (especially TestOptionsWithRanges.java) for all platforms
>>>>>>> except embedded.
>>>>>>> Let me back after testing on embedded.
>>>>>> I ran for embedded (linux-arm64, linux-armvh, linux-armvfpsflt, 
>>>>>> linux-armvfphflt, linux-armsflt) and all of them PASSED
>>>>>> for test/runtime/CommandLine/OptionsValidation.
>>>>>>
>>>>>> Thanks,
>>>>>> Sangheon
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Sangheon
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> cheers
>>>>>>>>
>>>>>>>>
>>>>>>>> On 09/10/2015 07:01 PM, sangheon.kim wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> Please review this patch for command-line validation for GC 
>>>>>>>>> flags.
>>>>>>>>> This REDO patch is adding ranges and implementing constraint 
>>>>>>>>> functions for GC flags.
>>>>>>>>>
>>>>>>>>> Original CR of JDK-8078555 was backout as it made a test 
>>>>>>>>> failure from 'TestOptionsWithRanges.java'.
>>>>>>>>> And also there were some discussion of OOM handling.
>>>>>>>>>
>>>>>>>>> Most parts are same as JDK-8078555 except below:
>>>>>>>>> 1. Changed 'range' for some flags.
>>>>>>>>> 2. Excluded 3 flags for TestOptionsWithRanges.java test. These 
>>>>>>>>> flags make this test unstable as it tries to allocate
>>>>>>>>> huge amount of memory.
>>>>>>>>>
>>>>>>>>> And below are the suggestion note for JDK-8078555:
>>>>>>>>> 1. Exponential notation for 'double' type variable parse: 
>>>>>>>>> Previously there were some discussion for maximum value for
>>>>>>>>> double type flags from code review of JDK-8059557 and 
>>>>>>>>> JDK-8112746. And Kim and I decided not to add upper limit unless
>>>>>>>>> there are problems with DBL_MAX. And as 255 is the maximum 
>>>>>>>>> length that can be passed via command-line, we introduced
>>>>>>>>> exponential notation to avoid this limit. ( arguments.cpp )
>>>>>>>>> 2. These GC flags ranges are not ideal ranges but ranges which 
>>>>>>>>> don't make problem with current source code.
>>>>>>>>>      If one flag makes some problem but hard to find good 
>>>>>>>>> range, I added some ranges.
>>>>>>>>> 3. There are some constraint functions to avoid overflow.
>>>>>>>>> 4. Test applications are changed: as some of them assumed to 
>>>>>>>>> be ParallelGC or to check it's output messages.
>>>>>>>>> 5. Includes cleanup of JDK-8133565: GC -2nd followup to 
>>>>>>>>> JDK-8059557.
>>>>>>>>>
>>>>>>>>> CR:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8134995
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.00/
>>>>>>>>> http://cr.openjdk.java.net/~sangheki/8134995/webrev.00_to_8078555
>>>>>>>>>
>>>>>>>>> Testing:
>>>>>>>>> JPRT, UTE(vm.quick-pcl) and 
>>>>>>>>> test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java. 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Sangheon
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>
>>
>