RFR 8078555(M): GC: implement ranges (optionally constraints) for those flags that have them missing

[sangheon.kim at oracle.com]

Hi Jon,

> On Aug 26, 2015, at 8:10 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> http://cr.openjdk.java.net/~sangheki/8078555/webrev.03/src/share/vm/runtime/commandLineFlagConstraintsGC.cpp.frames.html
> 
> Minor point.
> 
> 303 Flag::Error G1YoungSurvRateNumRegionsSummaryConstraintFunc(intx value, bool verbose) {
> 304 if (!UseG1GC) return Flag::SUCCESS;
> 305
> 306 if (value > (intx)HeapRegionBounds::target_number()) {
> 307 CommandLineError::print(verbose,
> 308 "G1YoungSurvRateNumRegionsSummary (" INTX_FORMAT ") must be "
> 309 "less than or equal to region amount (" SIZE_FORMAT ")\n",
> 
> I think "less than or equal to region count" sounds better ("count" instead of "amount").
Okay. 
I will fix it as your suggestion. 

> 
> http://cr.openjdk.java.net/~sangheki/8078555/webrev.03/src/share/vm/gc/g1/g1_globals.hpp.frames.html
> 
> 261   develop(uintx, G1DummyRegionsPerGC, 0,                                    \
> 262           "The number of dummy regions G1 will allocate at the end of "     \
> 263           "each evacuation pause in order to artificially fill up the "     \
> 264           "heap and stress the marking implementation.")                    \
> 265 range(0, 1000)
> 
> Is 1000 large enough?  Have I already asked that?   Is that enough to fill a 100g heap?

Let me cancel adding a range here and handle from different CR. 

> More later.
Okay. 

Thanks,
Sangheon


> 
> Jon
> 
>> On 8/26/2015 11:51 AM, sangheon.kim wrote:
>> Hi Derek,
>> 
>> Thank you for looking at this!
>> 
>>> On 08/26/2015 10:59 AM, Derek White wrote:
>>> Hi Sangheon,
>>> 
>>> I haven't reviewed the actual ranges and constraints yet, but one thing you might want to consider:
>>> 
>>> For the test cases, you may want synchronize the GC specified to ProcessBuilder with the "@requires gc=" tags. This prevents the test harness from running G1 tests when the test harness is trying to run CMS test, etc, and also avoids potential confusing test failures.
>>> 
>>> @requires vm.gc=="G1" | vm.gc=="null"
>>> (or specify Parallel GC as needed).
>> Thank you for the explanation.
>> I didn't know about this.
>> 
>> So it seems like vm.gc=="null" means not specifying vm option.
>> 
>>> 
>>> This is for:
>>> TestG1ConcMarkStepDurationMillis.java (G1)
>>> TestObjectTenuringFlags.java (Parallel)
>>> TestInitialTenuringThreshold.java (Parallel)
>>> TestG1HeapRegionSize.java (G1)
>> Okay, I will add these tags at next webrev.
>> 
>> Thanks,
>> Sangheon
>> 
>> 
>>> 
>>> - Derek
>>> 
>>>> On 8/24/15 5:33 PM, sangheon.kim wrote:
>>>> Hi Kim,
>>>> 
>>>>> On 08/24/2015 02:16 PM, Kim Barrett wrote:
>>>>>> On Aug 24, 2015, at 3:06 PM, sangheon.kim <sangheon.kim at oracle.com> wrote:
>>>>>> Hi Kim,
>>>>>> 
>>>>>> Here's webrev.03 which includes your comment for MarkStackSize constraint function.
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~sangheki/8078555/webrev.03
>>>>>> http://cr.openjdk.java.net/~sangheki/8078555/webrev.03_to_02/
>>>>>> 
>>>>>> And all your comments will be managed by https://bugs.openjdk.java.net/browse/JDK-8134348 .
>>>>> If the value of MarkStackSizeMax were changed later, there's nothing
>>>>> to verify MarkStackSize is still smaller.  [This is related to my
>>>>> earlier comment about constraints between options being tested twice
>>>>> and failures reported twice.]  Do we care in this case?
>>>> If your concern is something like
>>>> -XX:MarkStackSize=128 -XX:MarkStackSizeMax=100.
>>>> Yes, in this case the order is important as ranges and constraint functions are verified by its order.
>>>> MarkStackSizeMax will be verified first(its range) and MarkStackSize will be compared with verified MarkStackSizeMax.
>>>> 
>>>> And as I said your original concern is current limitation.
>>>> If we set CMSOldPLABMin and CMSOldPLABMax together with invalid values (e.g. CMSOldPLABMin=100, CMSOldPLABMax=50),
>>>> they will print out 2 failure messages.
>>>> 
>>>> Thanks,
>>>> Sangheon
>>>> 
>>>>> 
>>>>> Other than that, looks good.
>