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

[Roman Kennke]

Wouldn't that lead to mis-aligned accesses when you try to copy an array
of, e.g., bytes, but shoehorn it to pointer-size? Or am I misunderstanding?

An idea would be to copy equal-sized elements using the corresponding
Copy::conjoint_XXX_atomic() calls, e.g. char and short would use the
short version, boolean and byte would use the byte version (or is
boolean sometimes int?!), float and int would use the int version and
long and double the long version. WDYT?

Roman


> My spontaneous idea was to have an overload for type erased void* that
> when called with ARRAYCOPY_ATOMIC maps to Copy::conjoint_memory_atomic
> which AFAIK is conservatively atomic regardless of what the precise
> element type is.
> 
> But if you have better ideas, I am open for suggestions.
> 
> Thanks,
> /Erik
> 
> On 2018-03-06 17:03, Roman Kennke wrote:
>> Am 06.03.2018 um 14:35 schrieb Roman Kennke:
>>> Makes me wonder: why attempt to be smart in c1_Runtime1.cpp, when we can
>>> just as well call typeArrayOop::copy_array() and have it do the right
>>> thing? Or go even further and also do it for oop-arraycopy?
>> Something like:
>>
>> http://cr.openjdk.java.net/~rkennke/8198445/8198445-1.patch
>>
>> This wouldn't compile because of bunch of missing
>> arraycopy_conjoint_atomic defintions for extra types like jfloat,
>> jdouble, jboolean, etc, which in turn would be missing the same
>> Copy::conjoint_jFluffs_atomic() which drags in a bunch of platform
>> specific stuff... and my question before I go there is: do we want all
>> that? Or can you think of a better way to solve it?
>>
>> Roman
>>
>>> Roman
>>>
>>>> The ARRAYCOPY_ATOMIC decorator makes the arraycopy atomic over the size
>>>> of the passed in elements.
>>>> In this case, it looks like the address has been type erased to void*,
>>>> and hence lost what the element size was. There is currently no
>>>> overload
>>>> accepted for type erased element - only accurate elements {jbyte,
>>>> jshort, jint, jlong}.
>>>>
>>>> So it looks like an overload must be added to accept type erased void*
>>>> elements and make that call conjoint_memory_atomic when the
>>>> ARRAYCOPY_ATOMIC decorator is passed in.
>>>>
>>>> Thanks,
>>>> /Erik
>>>>
>>>> On 2018-03-06 13:56, David Holmes wrote:
>>>>> On 6/03/2018 10:54 PM, Erik Österlund wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> It is atomic with the ARRAYCOPY_ATOMIC decorator. If that comment is
>>>>>> correct (I do not know if it is), then the ARRAYCOPY_ATOMIC decorator
>>>>>> should probably be used here.
>>>>> If that code implements a Java array copy then yes it is required to
>>>>> be 32-bit atomic. Do you need the decorator to get 32-bit atomicity?
>>>>>
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> /Erik
>>>>>>
>>>>>> On 2018-03-06 13:48, David Holmes wrote:
>>>>>>> Hi Roman,
>>>>>>>
>>>>>>> Not a review as I'm not familiar enough with the Access API, but in
>>>>>>> src/hotspot/share/c1/c1_Runtime1.cpp the comments above the changed
>>>>>>> code need updating - probably deleting. I assume the Access API
>>>>>>> arraycopy is atomic?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 6/03/2018 9: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/
>>>>>>>>
>>>>>>>> Tests: tier1 ok
>>>>>>>>
>>>>>>>> Please review!
>>>>>>>> Thanks, Roman
>>>>>>>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20180308/262fd0dc/signature.asc>