RFR(xs): 8153201: TestOptionsWithRanges fails with -XX:OldPLABSize=2147483648

[Bengt Rutisson]

Hi Sangheon,

On 2016-04-01 09:07, sangheon wrote:
> Hi Bengt,
>
> Thank you for looking at this.
>
> On 03/31/2016 10:21 PM, Bengt Rutisson wrote:
>>
>> Hi Sangheon,
>>
>> On 2016-04-01 02:05, sangheon wrote:
>>> Hi all,
>>>
>>> Could I have a couple of reviews for this tiny change for 
>>> OldPLABSize flag?
>>>
>>> We would face an assert complaining too large array size based on 
>>> OldPLABSize.
>>> And we already have a constraint for this flag checking same 
>>> value(ThreadLocalAllocBuffer::max_size) but the constraint function 
>>> is checking for CMS GC and G1 GC.
>>> This change is just adding parallel gc on that check routine to 
>>> limit both min and max.
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8153201
>>> Webrev: http://cr.openjdk.java.net/~sangheki/8153201/webrev.00
>>
>> Do we need to check for the GC at all? SerialGC does not use PLABs, 
>> but that just means that those who use SerialGC should not have 
>> OldPALBSize on their command lines. But if they do, why should it be 
>> ok to set it to an arbitrary value? So, I would suggest just removing 
>> the GC check all together.
> Range/constraint validation is only working on utilized flags and 
> there are 2 reasons.
> Firstly, sometimes there's no way to validate. For example, 
> ParGCStridesPerThread flag is only used at CMS GC. If other GCs want 
> to validate it, how can we validate it? Allow all values? That is same 
> as not checking.
> Secondly, original JEP JDK-8059557 describes as "Non-goal 1. We will 
> not validate arguments to flags that are not processed by the JVM". 
> Sorry to mention the JEP again, but I'm trying to explain current 
> implementation, I'm not refusing the enhancement if needed. :)
>
> In this particular case, OldPLABSize for serial GC can be limited same 
> as parallel gc unintentionally.
> Because PLAB max is limited by TLAB max and TLAB max is set by 
> CollectedHeap::max_tlab_size() which serial gc also has.

Ok. I see. So, the current implementation would work for SerialGC, but 
if we improve with adding other ways for min/max size for PLABs it may 
stop working for SerialGC. That is a valid point.

>
> However, I think it will be better to remain as is for consistency.

Agreed.

In that case the changes look good. Reviewed.

Thanks,
Bengt

>
> Thanks,
> Sangheon
>
>
>>
>> Thanks,
>> Bengt
>>
>>> Testing: JPRT, runtime/commandline JTREG tests for all platforms.
>>>
>>> Thanks,
>>> Sangheon
>>
>