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

[Erik Österlund]

Hi Roman,

On 2018-03-08 11:25, Roman Kennke wrote:
> 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?

No it would not. The element type is void, not void*. Because the 
element type is type erased. And that would map to 
Copy::conjoint_memory_atomic, which is precisely what typeArrayKlass.cpp 
maps such an arraycopy to today. Assuming elements are aligned to their 
natural size (which they are), Copy::conjoint_memory_atomic is 
guaranteed to be at least as atomic as you need your elements to be, up 
to an element size of 8.

> 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?

I'm not sure I see what the advantage of that would be.

Thanks,
/Erik

> 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
>>>>>>>>>
>