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

[Jon Masamitsu]

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").

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?

More later.

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.
>>>>
>>>
>>
>