RFR 8134995(M): [REDO] GC: implement ranges (optionally constraints) for those flags that have them missing
sangheon.kim
sangheon.kim at oracle.com
Thu Oct 1 19:10:41 UTC 2015
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().
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
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>
>
More information about the hotspot-dev
mailing list