RFR: JDK-8198445: Access API for primitive/native arraycopy (original) (raw)

Per Liden per.liden at oracle.com
Tue Mar 6 13🔞21 UTC 2018

On 03/06/2018 01:47 PM, Per Liden wrote:

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::copyarray(), 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::XYZaddr() + memcpy() pattern. I'll address that in a separate change though. http://cr.openjdk.java.net/~rkennke/8198445/webrev.00/ src/hotspot/share/c1/c1Runtime1.cpp ------------------------------------ -    memmove(dstaddr, srcaddr, length << l2es);_ _-    return acok;_ _+    return HeapAccess<>::arraycopy(arrayOop(src), arrayOop(dst), srcaddr, dstaddr, length << l2es) ? acok : acfailed;_ _I think we should always return acok 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(srcobj, dstobj, 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 ooparraycopy() and avoid doing these types of "decisions" in this layer. src/hotspot/share/oops/accessBackend.cpp ---------------------------------------- +  void arraycopyconjoint(char* src, char* dst, sizet length) { +    Copy::conjointjbytes(src, dst, length); +  } This looks like an unnecessary addition. I suggest you adjust the code in src/hotspot/share/c1/c1Runtime1.cpp to use jbyte* instead char* in the address calculations. I.e. these lines: char* srcaddr = (char*) ((oopDesc**)src + ihs) + (srcpos << l2es); char* dstaddr = (char*) ((oopDesc**)dst + ihs) + (dstpos << l2es);

I missed this one...

src/hotspot/share/oops/typeArrayKlass.cpp

You're loosing the atomic part here, which should translate to an ARRAYCOPY_ATOMIC decorator.

Erik will send out a proposal for how to deal with the atomic stuff when we've erased the type information, like in C1's stub.

/Per

cheers, Per

Tests: tier1 ok Please review! Thanks, Roman

More information about the hotspot-gc-dev mailing list