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

[coleen.phillimore at oracle.com]

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 did look at that but the platform independent macroAssembler.hpp file 
is empty and only includes the platform dependent one.

The platform dependent one is a full class declaration, so isn't 
additive of the platform independent one unfortunately.  There are a lot 
of duplication which would be nice to clean up.

>
> 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 had trouble writing this in the caller, which is where I would have 
preferred it also.   But I certainly don't want to make resolve() do a 
load_acquire for this case.  I'll try to rewrite it.

Coleen
>
> Thanks,
> David
>
>> Thanks,
>> Coleen
>>
>>