RFR: 8218734: SA: Incorrect and raw loads of OopHandles

[coleen.phillimore at oracle.com]

On 2/13/19 10:40 AM, Stefan Karlsson wrote:
> On 2019-02-13 14:40, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 2/11/19 3:39 AM, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patch to fix the resolving of oops inside the 
>>> (VM) OopHandles.
>>>
>>> https://cr.openjdk.java.net/~stefank/8218734/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8218734
>>>
>>> Before this patch the OopHandle::_obj was assumed to be located at 
>>> offset 0, and the SA resolved OopHandle (Klass::_java_mirror and 
>>> ClassLoaderData::_class_loader) without the required load barrier 
>>> for ZGC. I've added a new class VMOopHandle (The SA already has a 
>>> OopHandle), which handles the resolving (load or load + barrier).
>>
>> This looks good but unfortunate that the SA has a different 
>> OopHandle. Maybe it would be more accurate to call it AccessOopHandle 
>> to imply that it has to use barriers?
>
> Maybe. I'm not sure I like the name AccessOopHandle any better, but if 
> you feel strongly about this I'll change it.
I don't feel strongly about it.
>
>>>
>>> The OopHandle type is grouped under the Oops section in 
>>> vmStructs.cpp. It's not an oop, so I added a newline to separate it 
>>> out from the rest. Any suggestion of a better location in that file?
>>>
>>
>> Seems ok to me.  The change looks fine.  Thank you for fixing this.
>
> Thanks for reviewing. I'll change getAddressField to getField as 
> suggested by Jini in another thread.
>

Good, I didn't see any replies and was wondering if it should be sent to 
another mailing list.

Coleen

> Thanks,
> StefanK
>
>>
>> Thanks,
>> Coleen
>>> Tested with the jtreg tests in serviceability/sa + patches to make 
>>> ZGC usable with the SA's hprof dumping.
>>>
>>> Thanks,
>>> StefanK
>>