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

[Erik Österlund]

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