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

[Stefan Karlsson]

On 2019-02-13 17:12, coleen.phillimore at oracle.com wrote:
>
>
> 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.

I think you dropped serviceability-dev (TO:ed).

StefanK

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