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

[Jon Masamitsu]

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.

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