[13] RFR(XXS) 8225369: [AOT] vm/classfmt/cpl/cplres001/cplres00101m004/cplres00101m004.html fails
dean.long at oracle.com
dean.long at oracle.com
Mon Jun 24 18:55:36 UTC 2019
On 6/21/19 5:33 PM, Vladimir Kozlov wrote:
> On 6/21/19 4:39 PM, Tom Rodriguez wrote:
>>
>>
>> dean.long at oracle.com wrote on 6/21/19 2:59 PM:
>>> On 6/21/19 1:35 PM, Tom Rodriguez wrote:
>>>>
>>>>
>>>> dean.long at oracle.com wrote on 6/21/19 1:14 PM:
>>>>> If we only have one API, renaming
>>>>> resolvePossiblyCachedConstantInPool to resolveConstantInPool would
>>>>> make it more consistent with other resolve methods. What do you
>>>>> think?
>>>>
>>>> I assume the naming is to provide a clue to the API use in
>>>> ConstantPool. It does seem like it's really only about constants
>>>> and not general constant pool resolution right?
>>>>
>>> Yes, it's about those types of constants that are Objects. The
>>> resolution could involve calling into Java, but we should only do
>>> that once. That's why the cache is important. Going through the
>>> cache should be the default, and every other lookup and resolve
>>> method should be going through the cache, so the PossiblyCached part
>>> seems confusing and redundant now.
>>
>> Is this something that's changed in 13+? It would be nice to
>> maintain JVMCI source consistency between 8 and 13 where possible.
>> Anyway, I don't feel strongly either way about the name. It's pure
>> internals anyway.
>
> Please, keep name the same for consistency with jvmci-8.
>
OK, here's the webrev that removes the unused method, but does not rename:
http://cr.openjdk.java.net/~dlong/8225369/remove/
While I was at it, I merged ResolveConstantInPoolTest into
ResolvePossiblyCachedConstantInPoolTest, but it's not strictly
necessary. I could keep them separate.
dl
> Vladimir
>
>>
>> tom
>>
>>>
>>> dl
>>>> tom
>>>>
>>>>>
>>>>> dl
>>>>>
>>>>> On 6/21/19 1:09 PM, dean.long at oracle.com wrote:
>>>>>> On 6/21/19 9:42 AM, Tom Rodriguez wrote:
>>>>>>> Doesn't that make resolveConstantInPool completely unused?
>>>>>>
>>>>>> Yes, except for a jtreg test, which can be removed.
>>>>>>
>>>>>>> It should probably be deleted and the javadoc updated for
>>>>>>> resolvePossiblyCachedConstantInPool.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>> dl
>>>>>>
>>>>>>>
>>>>>>> tom
>>>>>>>
>>>>>>> dean.long at oracle.com wrote on 6/18/19 10:52 AM:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8225369
>>>>>>>> http://cr.openjdk.java.net/~dlong/8225369/webrev/
>>>>>>>>
>>>>>>>> This test triggers a race, which can result in different
>>>>>>>> resolved values of the MethodHandle constant. The VM chooses a
>>>>>>>> winner and stores that value in the constant pool cache,
>>>>>>>> however Graal is not checking the cache. The trivial fix is to
>>>>>>>> use the proper API that goes through the cache.
>>>>>>>>
>>>>>>>> dl
>>>>>>
>>>>>
>>>
More information about the hotspot-compiler-dev
mailing list