RFR: JDK-8198285: More consistent Access API for arraycopy

[Roman Kennke]

Ping? Can I get a review?

Thanks, Roman


> I just realized we use ptrdiff_t for offset in the rest of the API. I am
> not sure that it's really a good fit. ptrdiff_t is meant to be used in
> pointer arithmetic and is relative to the size of the pointer type. We
> use it to give an offset in bytes from object start to the field, which
> is always positive and can be unsigned (ptrdiff_t is signed) and is
> always in bytes. Alas, I changed the API to use ptrdiff_t for now to
> make it consistent. If we want to change this, we shall change them all.
> 
> Please review the updated full webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8198285/webrev.01/
> 
> Thanks, Roman
> 
>>  Currently, the arraycopy API in access.hpp gets the src and dst oops,
>> plus the src and dst addresses. In order to be most useful to garbage
>> collectors, it should receive the src and dst oops together with the src
>> and dst offsets instead, and let the Access API / GC calculate the src
>> and dst addresses.
>>
>> For example, Shenandoah needs to resolve the src and dst objects for
>> arraycopy, and then apply the corresponding offsets. With the current
>> API (obj+ptr) it would calculate the ptr-diff from obj to ptr, then
>> resolve obj, then re-add the calculate ptr-diff. This is fragile because
>> we also may resolve obj in the runtime before calculating ptr (e.g. via
>> arrayOop::base()). If we then pass in the original obj and a ptr
>> calculated from another copy of the same obj, the above resolution logic
>> would not work. This is currently the case for obj-arraycopy.
>>
>> I propose to change the API to accept obj+offset, in addition to ptr for
>> both src and dst. Only one or the other should be used. Heap accesses
>> should use obj+offset and pass NULL for raw-ptr, off-heap accesses (or
>> heap accesses that are already resolved.. use with care) should pass
>> NULL+0 for obj+offset and the raw-ptr. Notice that this also allows the
>> API to be used for Java<->native array bulk transfers.
>>
>> An alternative would be to break the API up into 4 variants:
>>
>> Java->Java transfer:
>> arraycopy(oop src, size_t src_offs, oop dst, size_t dst_offs, size_t len)
>>
>> Java->Native transfer:
>> arraycopy(oop src, size_t src_offs, D* raw_dst, size_t len)
>>
>> Native->Java transfer:
>> arraycopy(S* src_raw, oop dst, size_t dst_offs, size_t len)
>>
>> 'Unsafe' transfer:
>> arraycopy(S* src_raw, D* dst_raw, size_t len)
>>
>>
>> But that seemed to be too much boilerplate copy+pasting for my taste.
>> (See how having this overly complicated template layer hurts us?)
>>
>> Plus, I had a better idea: instead of accepting oop+offset OR T* for
>> almost every Access API, we may want to abstract that and take an
>> Address type argument, which would be either HeapAddress(obj, offset) or
>> RawAddress(T* ptr). GCs may then just call addr->address() to get the
>> actual address, or specialize for HeapAddress variants and resolve the
>> objs and then resolve the address. This would also allow us to get rid
>> of almost half of the API (all the *_at variants would go) and some
>> other simplifications. However, this seemed to explode the scope of this
>> RFE, and would be better handled in another RFE.
>>
>> This changes makes both typeArrayKlass and objArrayKlass use the changed
>> API, plus I identified all (hopefully) places where we do bulk
>> Java<->native array transfers and make them use the API too. Gets us rid
>> of a bunch of memcpy calls :-)
>>
>> Please review the change:
>> http://cr.openjdk.java.net/~rkennke/JDK-8198285/webrev.00/
>>
>> Thanks, Roman
>>
> 
>