RFR(XS): JDK-8199780: SetMemory0 and CopyMemory0 in unsafe.cpp need to resolve their operands

[Erik Österlund]

Hi Roman,

On 2018-03-20 11:36, Roman Kennke wrote:
> Same reason as splitting resolve -> resolve_for_read/resolve_for_write
> in other routines: being able to distinguish read and write access.
> Also, I'd rather be careful to put this stuff in central places that
> might over-cover it.

It sounds like the motivation for this in my opinion more fragile call 
site chasing code is optimization.
What is the performance difference? Has this showed up in any profiles? 
Whenever robustness is traded for performance, it would be great to have 
some understanding about how much performance was lost.

> I've missed Unsafe_CopySwapMemory0, good find (has this been added
> recently?). I'll add Access calls there, and meditate a little bit how
> to put this into a more central place to avoid having to fix this for
> every code change in unsafe.cpp ;-)

No, this has been around for at least 2 years. In this file, address 
resolution is consistently done with index_oop_from_field_offset_long, 
so I think if you want to play it safe (and I know I would), then I 
would put the Access<>::resolve in there, and leave the rest of the call 
sites the way they are.

Thanks,
/Erik

> Thanks, Roman
>
>> Is there a good reason why the Access<>::resolve is not performed inside
>> of index_oop_from_field_offset_long instead of its callsites. For
>> example, it looks like barriers are missing in Unsafe_CopySwapMemory0,
>> that you would get for free by putting the resolve barrier in the API
>> used in this file for resolving addresses.
>>
>> Thanks,
>> /Erik
>>
>> On 2018-03-19 15:44, Roman Kennke wrote:
>>>    SetMemory0 and CopyMemory0 in unsafe.cpp read and write from/to
>>> objects, and thus need to resolve their operands via Access::resolve()
>>> before accessing them.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8199780
>>> Webrev:
>>> http://cr.openjdk.java.net/~rkennke/JDK-8199780/webrev.00/
>>>
>>> I'll say again that I'd prefer resolve_for_read() and
>>> resolve_for_write(), but for now the strong resolve() will suffice. ;-)
>>>
>>> Can I please get a review?
>>>
>>> Roman
>>>
>