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

[leerho]

Yes, it bothers me a little, although I clearly could live with whatever
you decide to do.
For your consideration, here are some of my thoughts :)

Variable naming is tough, but I could argue that the length specification
is hardly neutral.  Under the covers, a char[] length gets converted into a
length in bytes, which are the units for segments, and of course, defines
the length of the transfer.  In our minds as developers of the
implementations of this API it appears neutral, because we know it
ultimately ends up in units of bytes. To the user, however, the length
specification parameter in the API call has at least two properties that
make it closely associated with the heap primitive array as opposed to an
association with the segment:

   - It is specified in the units of the array, e.g. chars for a char[].
   The user will contrast this with other parts of the FMA API where the
   length units of segments are always bytes.
   - The carrier for the value is an int, while the carrier for a segment
   length specification is always a long.

If we spell out the current parameters just moving the length to the end it
would look like:

   - copyFromArray(char[] srcArray, int srcIndexChars, dstSegment, long
   dstOffsetBytes, int copyLengthChars, order).
   Note the alternation of units and carriers: {char[], int chars, segment,
   long bytes, int chars).  So the parameters associated with the source
   char[] are split up.  I find this awkward.
   - copyToArray(srcSegment, long srcOffsetBytes, char[] dstArray, int
   dstIndexChars, int copyLengthChars, order).
   Here, at least, the 3 parameters that the user would specify while
   contemplating the dstArray are together.

I'm sure I would have to refer to the javadoc almost every time I used
these methods to keep the units and carriers straight.

System.arrayCopy(...) is a nice model, which I like, but note that srcPos,
dstPos and length all are in the same units (the elements units of src and
dest) and with the same carrier: int.  It is a much simpler model than what
we are trying to do here.

What I like about the original proposed {source, destination] flow model is
that the 3 parameters that are associated with the heap array are always
together and it is highly likely that the user has those 3 parameters in
mind at the same time when constructing the call. The alternative model of
{segment parameters, array parameters} also has the 3 parameters of the
array together and it has the length at the end (with order), and would be
very easy to remember.

There is no perfect solution here.

Cheers,
Lee.


On Thu, Jun 24, 2021 at 1:51 PM John Rose <john.r.rose at oracle.com> wrote:

>
>
> On Jun 24, 2021, at 1:38 PM, Maurizio Cimadamore <
> maurizio.cimadamore at oracle.com<mailto:maurizio.cimadamore at oracle.com>>
> wrote:
>
> So:
>
> - copyToArray(srcSegment, srcOffsetBytes, dstArray, dstIndexChars,
> numChars, order);
> - copyFromArray(srcArray, srcIndexChars, dstSegment, dstOffsetBytes,
> numChars, order);
>
> or
>
> - copyToArray(srcSegment, srcOffsetBytes, dstArray, dstIndexChars,
> numChars, order);
> - copyFromArray(dstSegment, dstOffsetBytes, srcArray, srcIndexChars,
> numChars, order);
>
> Depending on how the other parameters should be sorted. I personally
> prefer:
>
> * source first
> * destination second
> * length last (and order after that)
>
> (which means the first of the two proposal above).
>
> As John says, that's how System::arrayCopy (and many other libs) do it, I
> don't think I see the need for going down another path.
>
> Is there a specific concern you have against that?
>
> It feels like violent agreement here:  System::arraycopy
> has left-to-right data flow (yay, also Lee’s point!), and
> ends with the common length value.
>
> (Plus the byte order at the end too.  I’d sort that at
> the very last, because it is the most neglect-able.)
>
> You know, you could almost name this primitive
> “arraycopy” and just rely on overloads.  The “From”
> and “To” words are a little noisy, and they may
> distract readers into thinking that they affect
> argument order.  I think we agree they shouldn’t;
> it’s simpler to have a left-to-right rule.
>
> I was recently working on a non-static version of
> a similar API (bulk copies!), and in that case the
> receiver object was always the non-array.  That
> requires help from the name, in the form of
> “In” and “Out” words which tell whether data
> is entering or leaving the receiver object:
>
> interface Words {
>     // bulk copy
>     default long copyIn(long destPos, long[] src, int srcStart, int
> srcEnd) {
>         for (int i = srcStart; i < srcEnd; i++)
>             set(destPos++, src[i]);
>         return destPos;
>     }
>     default int copyOut(long start, long end, long[] dest, int destPos) {
>         for (long i = start; i < end; i++)
>             dest[destPos++] = get(i);
>         return destPos;
>     }
>
>     long length();
>     long get(long index);
>     void set(long index, long value);
>     …
> }
>
> But for a static method, it’s better to have a static
> ordering of arguments!
>
>
>
>