RFR (T) 8228855: Test runtime/CommandLine/OptionsValidation/TestOptionsWithRanges fails after JDK-8227123

[David Holmes]

On 1/08/2019 7:57 am, coleen.phillimore at oracle.com wrote:
> On 7/31/19 5:30 PM, David Holmes wrote:
>> On 31/07/2019 11:39 pm, coleen.phillimore at oracle.com wrote:
>>> On 7/31/19 9:25 AM, coleen.phillimore at oracle.com wrote:
>>>> On 7/31/19 9:12 AM, David Holmes wrote:
>>>>> I haven't seen Coleen's original mail turn up yet, so I'll respond 
>>>>> here.
>>>
>>> I haven't gotten the email yet either.
>>>>>
>>>>> Shouldn't the range be handled by the constraint function:
>>>>
>>>> It is not handled that way in ObjAlignmentInBytes.
>>>
>>> What I meant is that ObjectAlignmentInBytes has the constraint 
>>> function AND the range.  SurvivorAlignmentInBytes should be the 
>>> same.  The constraint function tests that it's > ObjectAlignmentInBytes.
>>>
>>>   158   lp64_product(intx, ObjectAlignmentInBytes, 
>>> 8,                             \
>>>   159           "Default object alignment in bytes, 8 is 
>>> minimum")                \
>>>   160           range(8, 
>>> 256)                                                     \
>>>   161 constraint(ObjectAlignmentInBytesConstraintFunc,AtParse) \
>>
>> Okay, so specifying the range is reasonable and I guess specifying the 
>> same range as ObjectAlignmentInBytes is also reasonable.
>>
>> Note however that the default value for SurvivorAlignmentInBytes is 0 
>> which is outside of the specified range, so I'm not sure if that makes 
>> sense. AFAICS the constraint function only applies if the flag is 
>> explicitly set so that default value is ignored. I don't know if that 
>> is the case for the range check ??
> 
> I think the range check is applied after ergonomics, and the code in 
> arguments has:
> 
>    if (SurvivorAlignmentInBytes == 0) {
>      SurvivorAlignmentInBytes = ObjectAlignmentInBytes;
>    }

Okay. Hard to remember how all this hangs together.

> Probably the default should be changed to 8, and this code removed. Or 
> maybe not, maybe it should be:
> 
> if (FLAG_IS_DEFAULT(SurvivorAlignmentInBytes)) {
>     SurvivorAlignmentInBytes = ObjectAlignmentInBytes;
> }

Yes that looks more reasonable - assuming explicitly setting to 0 is 
disallowed because it is outside the range.

> But I don't know.  Maybe we should have a P5 RFE to clean this up.
> 
>>
>> But given the dependency between these two flags the test won't be 
>> able to adjust SurvivorAlignmentInBytes independently of 
>> ObjectAlignmentInBytes.
> 
> If both options are supplied, there's a test that S >= O.

My point is that TestOptionsWithRanges can't just cycle 
SurvivorAlignmentInBytes through its specified range because it doesn't 
know about the additional constraints.

I can't see how this test is supposed to work.

David
-----

> Thanks,
> Coleen
>>
>> David
>> -----
>>
>>>
>>> Coleen
>>>>
>>>> Coleen
>>>>>
>>>>>  SurvivorAlignmentInBytesConstraintFunc
>>>>>
>>>>> ?
>>>>>
>>>>> David (signing off for the night)
>>>>>
>>>>> On 31/07/2019 11:04 pm, Aleksey Shipilev wrote:
>>>>>> On 7/31/19 2:56 PM, coleen.phillimore at oracle.com wrote:
>>>>>>> open webrev at 
>>>>>>> http://cr.openjdk.java.net/~coleenp/2019/8228855.01/webrev
>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8228855
>>>>>>
>>>>>> Looks good and trivial.
>>>>>>
>>>>
>>>
>