RFR(XS): 8213410: UseCompressedOops @requirement check fails fails on 32-bit system
David Holmes
david.holmes at oracle.com
Mon Nov 12 22:35:15 UTC 2018
Hi Boris,
On 12/11/2018 10:18 PM, Boris Ulasevich wrote:
> Hi David,
>
> Ok. Here is the solution exploiting 64-bit & UseCompressedOops == true
> check (works ok and ARM64 and makes tests disabled on ARM32):
> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.03/
Looks good.
> I will try to go forward on code-tools-dev and I will correct these
> tests later if we get short-circuit boolean evaluation in jtreg.
Thanks!
David
> thank you,
> Boris
>
> On 12.11.2018 0:45, David Holmes wrote:
>> Hi Boris,
>>
>> On 9/11/2018 9:03 PM, Boris Ulasevich wrote:
>>> Hi David,
>>>
>>> Thank you.
>>>
>>> Don't you think it is better to learn jtreg to perform short-circuit
>>> boolean evaluation?
>>
>> Sure but that will take a lot longer to happen. You'll need to file a
>> bug in the CODE-TOOLS project and then discuss on code-tools-dev. Once
>> the fix is accepted we then have to wait for the next release of jtreg
>> to be promoted and then OpenJDK updated to start using it.
>>
>> Meanwhile if you want the tests to actually run they will need to be
>> modified to work with existing jtreg behaviour.
>>
>> Thanks,
>> David
>>
>>> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.02
>>> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.02_jtreg
>>>
>>> By the way, with the given change in jtreg two failed tests from the
>>> original list works Ok because they already have 64-bit check on
>>> previous lines:
>>> * @requires (sun.arch.data.model == "64")
>>> * @requires vm.opt.final.UseCompressedOops
>>>
>>> regards,
>>> Boris
>>>
>>> On 09.11.2018 10:39, David Holmes wrote:
>>>> Hi Boris,
>>>>
>>>> On 7/11/2018 6:54 PM, David Holmes wrote:
>>>>> Hi Boris,
>>>>>
>>>>> On 7/11/2018 6:06 PM, Boris Ulasevich wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> Yes, at first glance it is weird. We have actually three states
>>>>>> for vm.opt.final.UseCompressedOops: true, false and null. Null
>>>>>> means "not applicable" - when current VM does not support the
>>>>>> option with the given name. Here is another approach for the issue
>>>>>> (I am not sure it is a good one):
>>>>>> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.01/
>>>>>
>>>>> I like that approach! I'm just not sure everyone else will
>>>>> necessarily agree :) This proposal might need a wider audience than
>>>>> just hotspot-dev.
>>>>>
>>>>> Let me make enquiries with jtreg-use at openjdk.java.net and see what
>>>>> the general intent was here.
>>>>
>>>> That was uninformative as to intent as Jon initially thought both
>>>> variants should fail the same way, but on further reflection it is
>>>> likely due to the difference between trying to evaluate null as a
>>>> boolean - which gives an error - and doing "true".equals(null) ,
>>>> which is just false.
>>>>
>>>> I also see now that this isn't a general facility for using
>>>>
>>>> @requires <some VM flag>
>>>>
>>>> because we only actually support three directly:
>>>>
>>>> protected void vmOptFinalFlags(Map<String, String> map) {
>>>> vmOptFinalFlag(map, "ClassUnloading");
>>>> vmOptFinalFlag(map, "UseCompressedOops");
>>>> vmOptFinalFlag(map, "EnableJVMCI");
>>>> }
>>>>
>>>> but that also shows this is not specific to UseCompressedOops
>>>> because EnableJVMCI only exists in a VM that was built with JVMCI
>>>> support.
>>>>
>>>> I still think the correct way for this current case to be handled in
>>>> tests is by using:
>>>>
>>>> @requires vm.bits == 64 & UseCompressedOops
>>>>
>>>> though that requires jtreg to do short-circuit boolean evaluation,
>>>> which it appears not to do. :( Though:
>>>>
>>>> @requires vm.bits == 64 & UseCompressedOops == true
>>>>
>>>> should work (albeit perhaps due to implementation accident).
>>>>
>>>> Cheers,
>>>> David
>>>> -----
>>>>
>>>>> Aside: if the UseCompressOops checks is preceded by a 64-bit check
>>>>> does it still fail? (Just wondering whether jtreg does
>>>>> short-circuit evaluation for these @require statements.)
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> thank you,
>>>>>> Boris
>>>>>>
>>>>>> On 07.11.2018 1:36, David Holmes wrote:
>>>>>>> Hi Boris,
>>>>>>>
>>>>>>> I'm somewhat perplexed as to what the difference is between:
>>>>>>>
>>>>>>> @requires vm.opt.final.UseCompressedOops
>>>>>>>
>>>>>>> and
>>>>>>>
>>>>>>> @requires vm.opt.final.UseCompressedOops == true
>>>>>>>
>>>>>>> if they are indeed different then that seems to be a bug in the
>>>>>>> requirement handling in the jtreg support code.
>>>>>>>
>>>>>>> I'm also wondering whether any such requirement should always be
>>>>>>> proceeded by a check for 64-bits?
>>>>>>>
>>>>>>> Otherwise I would expect
>>>>>>>
>>>>>>> @requires vm.opt.final.UseCompressedOops
>>>>>>>
>>>>>>> to be false in 32-bit, so again a problem with the jtreg support
>>>>>>> code.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 7/11/2018 12:43 AM, Boris Ulasevich wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Please review this patch to fix jtreg @requires
>>>>>>>> vm.opt.final.UseCompressedOops flag evaluation fail reproduced
>>>>>>>> on ARM32: "invalid boolean value: null for expression
>>>>>>>> vm.opt.final.UseCompressedOops".
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~bulasevich/8213410/webrev.00/
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8213410
>>>>>>>>
>>>>>>>> The fix was checked on ARM32 (tests disabled) and AARCH64 (works
>>>>>>>> Ok by default, and becomes disabled with
>>>>>>>> -javaoptions:"-XX:-UseCompressedOops" jtreg option).
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Boris Ulasevich
More information about the hotspot-dev
mailing list