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

[sangheon.kim]

Hi Jon, Gerard and Derek,

Here's the new webrev which includes:
1. @requires vm.gc==xx for 4 test cases. (Derek)
2. Change INITIAL_RANGES_SIZE and INITIAL_CONSTRAINTS_SIZE to current 
actual size. (Gerard)
3. Range of G1UpdateBufferSize is 'NOT_LP64(32*M) LP64_ONLY(1*G)'. (Jon)
4. 'amount' to 'count' at 
G1YoungSurvRateNumRegionsSummaryConstraintFunc. (Jon)
5. Add more explanation for CheckMaxHeapSizeAndSoftRefLRUPolicyMSPerMB. 
(Jon)

http://cr.openjdk.java.net/~sangheki/8078555/webrev.04
http://cr.openjdk.java.net/~sangheki/8078555/webrev.04_to_03

Thanks,
Sangheon


On 08/26/2015 03:30 PM, sangheon.kim wrote:
>
>
> On 08/26/2015 02:56 PM, Jon Masamitsu wrote:
>>
>>
>> On 8/26/2015 1:50 PM, sangheon.kim wrote:
>>> Hi Jon,
>>>
>>> On 08/26/2015 01:34 PM, Jon Masamitsu wrote:
>>>> http://cr.openjdk.java.net/~sangheki/8078555/webrev.03/src/share/vm/gc/g1/g1_globals.hpp.frames.html 
>>>>
>>>>
>>>> How was the 1*G maximum chosen?
>>>>
>>>>  114   product(size_t, G1UpdateBufferSize, 
>>>> 256,                                  \
>>>>  115           "Size of an update 
>>>> buffer")                                       \
>>>> 116 range(1, 1*G)
>>>>
>>>>
>>>> Maybe this was already discussed?
>>> You have eagles eye! :)
>>> Personally I asked to Thomas and his suggestion was 'I think 
>>> something in the MB range should be "enough" '.
>>>
>>> I know that 1G is quite different from MB range but I wanted to find 
>>> not making problem range.
>>> One point that would make problem is 
>>> 'G1CollectedHeap::pending_card_num()'.
>>> '(buffer_size * buffer_num + extra_cards)' should not overflow and I 
>>> think 1G would be safe enough.
>>
>> 1G seems too large on a 32bit system.  I don't know how large buffer_num
>> can get but can image it  can get large in some instances.
>>
>> Having said that this is not necessarily something that can be fixed at
>> flag validation time.  This may be one of those cases where the 
>> calculated
>> numbers are too dynamic to catch early.   If you can make the check
>> different on a 32bit system, 32M might be a better number on a
>> 32bit system.  Leave it 1G on a 64bit system.
> Okay, I will change as you mentioned.
> range(1, NOT_LP64(32*M) LP64_ONLY(1*G))
>
> This will be included at next webrev.
>
> Thanks,
> Sangheon
>
>
>>
>> Jon
>>
>>> But I know that it is too large as a buffer size. :(
>>> Any suggestion for this?
>>>
>>> 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.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>