[foreign-memaccess+abi] RFR: Add support for high-level functions to copy to and from Java arrays [v2]

Maurizio Cimadamore mcimadamore at openjdk.java.net
Thu Jun 24 09:47:55 UTC 2021


On Mon, 21 Jun 2021 21:52:18 GMT, Lee Rhodes <github.com+12941506+leerho at openjdk.org> wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryCopy.java line 137:
>> 
>>> 135:         MemorySegment srcSegmentSlice =
>>> 136:                 MemorySegment.ofArray(srcArray).asSlice(srcIndexChars * 2L, srcCopyLengthChars * 2L);
>>> 137:         MemorySegment dstSegmentSlice = dstSegment.asSlice(dstOffsetBytes, srcCopyLengthChars * 2L);
>> 
>> Little weird to see `srcCopyLengthChars` being used for the destination slice. But the length is the same for both of course. Maybe just drop the `src` prefix? (it's a nit though).
>
> The variable name `srcCopyLengthChars` is for clarity to the user of the API, who does not see the implementation code. Because there are two different types of units in the method call, one in Chars and one in Bytes, it is important to clarify the units of the measure as well as which of source or destination it is associated with.

Honestly, I tend to agree with Jorn here. In a copy, there is:
* a source (either an array or a segment)
* a source offset (expressed either as element index, or as byte offset)
* a destination (either an array or a segment)
* a destination offset (expressed either as element index, or as byte offset)
* a length (expressed either as number of elements, or as number of bytes)

There's only one length - it's important to distinguish between Chars vs. Bytes - e.g. copyLengthChars vs. copyLengthBytes. But adding the `src` seems confusing because the `length` parameter is, IMHO neutral, and just indicates how many "things" have to be bulk-copied.

In fact, I'll go further and say that, when editing this patch I kept finding the placement of the length argument confusing. Having it as the trailing argument would be clearer IMHO.

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

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


More information about the panama-dev mailing list