[13] RFR(XXS) 8225369: [AOT] vm/classfmt/cpl/cplres001/cplres00101m004/cplres00101m004.html fails

[Tom Rodriguez]

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.

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
>>>>
>>>
>