[foreign-memaccess+abi] RFR: 8270376: Finalize API for memory copy

[Maurizio Cimadamore]

On Tue, 13 Jul 2021 16:49:06 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch ties up some loose ends with the MemoryCopy API, and it also prepares the ground for some other related refactorings in this area.
> 
> The meat of this patch is represented by the various changes in MemoryCopy, where all methods were renamed to simply `copy`, and the length parameter (now called `elementCount`) is always moved to the end.
> 
> I've also simplified naming of parameters, as I think distinguishing between `index` and `offset` is enough (e.g. an array has an index, a segment has an offset).
> 
> You will see that, at the very end of the class, three more copy methods have been added, which deal fully in terms of segments. I have also moved the complex layout-based memory segment copy operation (which does the swap) as a static method in this class, as I believe the static form makes the method more regular and usable.
> 
> I've made some changes to our uses of `copyFrom` in the linker, to use the new methods in `MemoryCopy` when the copy operation was performing slicing in the source/target segment, which I think makes the code more readable. Of course these changes are completely optional and could be omitted as well.

Thanks for the comments John - I agree pretty much with all the points you raise, so I think it's worth spending some more cycles to try and get this right. I do think that there is an useful distinction between primitive copy methods, and sugary ones, and we should try to make that distinction clearer in the API.

One thing I wasn't too sure about was if having a single entry point taking Object would create havoc - as the code will have to access array length reflectively, and all that - and I can guess it can get bad under profile pollution? This was the main reason as to why, few years ago, we went with the `MemorySegment::ofArray` API with all the overloads (instead of having only one taking an object) - because we had inlining issues with the monomorphic version. Maybe it's time to try again - or are there "tricks" that I overlooked?

I'm assuming the code would need to access at least:

* array length (can be done with Reflection.getLength which is an intrinsic)
* unsafe base/stride (can be done with Unsafe::arrayBaseOffset/arrayIndexScale)

In order to perform the bulk copy call. Do you foresee performance issues if we go down that path? If not, does that mean that we should retest our assumption when it comes to MemorySegment::ofArray as well?

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/568