RFR: JDK-8198445: Access API for primitive/native arraycopy

[Per Liden]

Hi Roman,

On 03/06/2018 12:56 PM, Roman Kennke wrote:
> Currently, the Access API is only used for oop-arraycopy, but not for
> primitive arraycopy. GCs might want to intercept this too, e.g. resolve
> src and dst arrays.
> 
> There *is* an implementation of primitive arraycopy in the Access API,
> but it doesn't even compile, because Raw::arraycopy() does not take src
> and dst oop operands, but it's called like that. The only reason why
> this does not blow up (I think) is that because nobody calls it, the
> compiler doesn't even get there.
> 
> This change fixes the Access API/impl and adds the relevant calls into
> it (in C1 and runtime land). C2 uses arraycopy stubs (which cannot be
> handled here) or calls out to the ArrayKlass::copy_array(), which should
> be covered with this change.
> 
> It should be possible to use the same Access API for Java-array <->
> native-array bulk transfers, which currently use the rather ugly
> typeArrayOop::XYZ_addr() + memcpy() pattern. I'll address that in a
> separate change though.
> 
> http://cr.openjdk.java.net/~rkennke/8198445/webrev.00/

src/hotspot/share/c1/c1_Runtime1.cpp
------------------------------------

-    memmove(dst_addr, src_addr, length << l2es);
-    return ac_ok;
+    return HeapAccess<>::arraycopy(arrayOop(src), arrayOop(dst), 
src_addr, dst_addr, length << l2es) ? ac_ok : ac_failed;

I think we should always return ac_ok here, and change 
Heap<>::arraycopy() to return void, since it will always return true for 
primitive types.


src/hotspot/share/gc/shared/barrierSet.hpp
------------------------------------------
-      return Raw::arraycopy(src_obj, dst_obj, src, dst, length);
+      return Raw::arraycopy(src, dst, length);

I'd suggest that we adjust the Raw::arraycopy() signature instead to 
accept these arguments (even if they are later unused), to keep this 
symmetric with oop_arraycopy() and avoid doing these types of 
"decisions" in this layer.


src/hotspot/share/oops/accessBackend.cpp
----------------------------------------

+  void arraycopy_conjoint<char>(char* src, char* dst, size_t length) {
+    Copy::conjoint_jbytes(src, dst, length);
+  }

This looks like an unnecessary addition. I suggest you adjust the code 
in src/hotspot/share/c1/c1_Runtime1.cpp to use jbyte* instead char* in 
the address calculations. I.e. these lines:

      char* src_addr = (char*) ((oopDesc**)src + ihs) + (src_pos << l2es);
      char* dst_addr = (char*) ((oopDesc**)dst + ihs) + (dst_pos << l2es);

cheers,
Per

> 
> Tests: tier1 ok
> 
> Please review!
> Thanks, Roman
>