RFR (M) 8186088: ConstantPoolCache::_resolved_references is not a JNIHandle

[serguei.spitsyn at oracle.com]

Hi Coleen,

Looks good.

http://cr.openjdk.java.net/~coleenp/8186088.02/webrev/src/share/vm/oops/oopHandle.hpp.html

   There is unneeded space after the const:

   50   oop resolve() const         { return (_obj == NULL) ? (oop)NULL : *_obj; }


Thanks,
Serguei


On 8/14/17 08:38, coleen.phillimore at oracle.com wrote:
>
>
> On 8/13/17 9:04 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 12/08/2017 7:34 AM, coleen.phillimore at oracle.com wrote:
>>> Summary: Make an OopHandle type to replace jobject to encapsulate 
>>> these oop pointers in metadata and module entry.
>>>
>>> This replaces places where there's a jobject that doesn't point into 
>>> the JNIHandles, notably things allocated in ClassLoaderData::_handles.
>>>
>>> There were platform specific changes that I tried to carefully make 
>>> - can someone test them for s390, aarch64 and ppc?
>>>
>>> Tested with tier1 testing, JPRT (all oracle platforms), nsk.jvmti, 
>>> monitoring, parallel class loading and g1class unloading tests.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8186088.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8186088
>>
>> Is it possible to put the declaration of 
>> MacroAssembler::resolve_oop_handle into the shared macroAssembler.hpp 
>> file to avoid the need to add it to the platform specific hpp files?
>>
>> I'm unsure about the OopHandle containing set_atomic. First if it is 
>> present then the _obj field should be volatile. But then it also 
>> raises the question: if we can race to set this, do we need 
>> load-acquire versions of the accessors? Based on the single existing 
>> usage I don't think we presently do. But I think it may be 
>> cleaner/simpler to not have set_atomic, make the OopHandle immutable, 
>> and do the cmpxchg back in the caller code by atomically updating _pd 
>> (which should also be volatile even today) with a new OopHandle.
>
> I couldn't convince the compiler to compile an Atomic::cmpxchg_ptr() 
> to an address of OopHandle since it is not a pointer but a struct. The 
> casting would be wrong.   So I added a resolve_acquire() for the 
> protection_domain case in ModuleEntry, and some commentary.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8186088.02/webrev
>
> thanks,
> Coleen
>
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Coleen
>>>
>>>
>