RFR 8152343: JVMCI test tasks: Unit tests for MetaAccessProvider

Christian Thalinger christian.thalinger at oracle.com
Wed May 11 21:03:39 UTC 2016


> On May 11, 2016, at 10:43 AM, Dmitrij Pochepko <dmitrij.pochepko at oracle.com> wrote:
> 
> Also performed mx eclipseformat and checkstyle and prepared v04
> 
> http://cr.openjdk.java.net/~dpochepk/8152343/webrev.04/ <http://cr.openjdk.java.net/~dpochepk/8152343/webrev.04/>

That looks good.

> 
> Thanks,
> Dmitrij
>> Hi,
>> 
>> i've added encoded values generation and changed test code to verify expected values based on generation input values.
>> 
>> Please take a look at http://cr.openjdk.java.net/~dpochepk/8152343/webrev.03/ <http://cr.openjdk.java.net/%7Edpochepk/8152343/webrev.03/>
>> 
>> Thanks,
>> Dmitrij
>>> 
>>>> On May 7, 2016, at 11:41 AM, dmitrij pochepko <dmitrij.pochepko at oracle.com <mailto:dmitrij.pochepko at oracle.com>> wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> On May 6, 2016, at 4:25 AM, Dmitrij Pochepko <dmitrij.pochepko at oracle.com <mailto:dmitrij.pochepko at oracle.com>> wrote:
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> please review patch for 8152343: JVMCI test tasks: Unit tests for MetaAccessProvider
>>>>>> 
>>>>>> A new tests were added for jdk.vm.ci.meta.MetaAccessProvider implementation. An existing TestMetaAccessProvider.java was used to add new tests.
>>>>>> 
>>>>>> webrev:  <http://cr.openjdk.java.net/%7Edpochepk/8152343/webrev.01/>http://cr.openjdk.java.net/~dpochepk/8152343/webrev.01/ <http://cr.openjdk.java.net/~dpochepk/8152343/webrev.01/>+    private static final int[] VALID_ENCODED_VALUES = new int[]{-180, -436, -10932, -2147483572};
>>>>> What are these values?  Maybe it would make more sense to have them in hex-form?
>>>> These values are encoded values for deoptReason=Aliasing, deoptAction=InvalidateRecompile and debugId={0, 1, 42, 8388607}
>>>> so, we have 0, 1, <some non-border value>, 0x7FFFFF which is a maximum debugId value here(all 23 bits set)
>>>> 
>>>> I've created another webrev with hex values and respective comment added.
>>>> 
>>>> http://cr.openjdk.java.net/~dpochepk/8152343/webrev.02 <http://cr.openjdk.java.net/%7Edpochepk/8152343/webrev.02>
>>> 
>>> Better but I still don’t like them being hardcoded.  You should calculate the values with some defined constants so that someone after you can understand what’s going on and be able to make changes.
>>> 
>>> Also, what is this supposed to test?
>>> +    @Test
>>> +    public void decodeDeoptReasonTest() {
>>> +        for (int encoded : VALID_ENCODED_VALUES) {
>>> +            JavaConstant value = JavaConstant.forInt(encoded);
>>> +            DeoptimizationReason reason = metaAccess.decodeDeoptReason(value);
>>> +            DeoptimizationReason reason2 = metaAccess.decodeDeoptReason(value);
>>> +            assertNotNull("Expected not null reason", reason);
>>> +            assertEquals("Expected equal reasons", reason, reason2);
>>> +        }
>>> +    }
>>> That two invocations of the same method on the same receiver with the same value return the same value?  I don’t see how that could fail.
>>> 
>>>> 
>>>> Thanks,
>>>> Dmitrij
>>>>> 
>>>>> 
>>>>>> 
>>>>>> CR:  <https://bugs.openjdk.java.net/browse/JDK-8152343>https://bugs.openjdk.java.net/browse/JDK-8152343 <https://bugs.openjdk.java.net/browse/JDK-8152343>
>>>>>> 
>>>>>> I've tested it on linux_x64.
>>>>>> 
>>>>>> Thanks,
>>>>>> Dmitrij
>>>>> 
>>>> 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160511/fc78086e/attachment-0001.html>


More information about the hotspot-compiler-dev mailing list