RFR: 8198286: Direct memory accessors in typeArrayOop.hpp should use Access API

[Roman Kennke]

Hi Eric,

On Tue, Feb 20, 2018 at 5:41 PM, Erik Österlund
<erik.osterlund at oracle.com> wrote:
> Hi Per,
>
> (looping in hotspot-dev as this seems to touch more than runtime)
>
> On 2018-02-20 17:03, Per Liden wrote:
>>
>> Hi Erik,
>>
>> As we discussed, coming up with a good name for the new Access call is
>> really hard. All good/descriptive alternatives I can come up with tend to be
>> way to long. So, next strategy is to pick something that fits into the reset
>> of the API. With this in mind I'd like to suggest we just name it: oop
>> Access<>::resolve(oop obj)
>>
>> The justification would that this this matches the one-verb style we have
>> for the other functions (load/store/clone) and it seems that you anyway
>> named the internal parts just "resolve", such as BARRIER_RESOLVE, and
>> resolve_func_t.
>
>
> Sure.
>
> Here is a full webrev with my proposal for this RFE, now that we agree on
> the direction:
> http://cr.openjdk.java.net/~eosterlund/8198286/webrev.00/
>
> Incremental from the prototype I sent out for early turnaround yesterday:
> http://cr.openjdk.java.net/~eosterlund/8198286/webrev.00_inc/
>
> It is now enforced that *_addr_raw() functions are to be used by the GC
> only, when the GC knows addresses are stable. All other address resolution
> goes through non-raw address resolution functions that at a lower level end
> up calling the resolve barrier on Access, which can be overridden by
> Shenandoah. There are in total two callers of Access<>::resolve: on
> oopDesc::field_addr and arrayOop::base. The rest is derived from that.
>
> @Roman: Hope this works well for Shenandoah.

I will review it ASAP.

Just a quick note: In Shenandoah we have introduced an API in
BarrierSet to check bool BarrierSet::is_safe(oop) which checks exactly
the to-space invariant, i.e. if it's safe to access that oop for
writing. I'd like that to be upstream if possible, and maybe this
would be a good opportunity to add it? Maybe rename is_stable() or
assert_stable() (and maybe keep it under #ifdef ASSERT)?

Thanks for doing this!

Roman