RFR 8134995(M): [REDO] GC: implement ranges (optionally constraints) for those flags that have them missing
Jon Masamitsu
jon.masamitsu at oracle.com
Thu Oct 1 18:57:21 UTC 2015
Sangheon,
What was the failure that prompted this change? 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