Part 1 Proposal for JDK-8264594

[Maurizio Cimadamore]

Thanks.

While I agree that a segment is just a bunch of bytes, I think also 
think that having an API which takes two indices at the same time, one 
logical and one physical is kind of evil. I bet that a lot of developers 
will get that wrong, at least the first time.

What you say about segments being an array of bytes, and thus not 
needing index-based overloads can also be said for MemoryAccess class - 
but I believe that the index-based accessors in the are quite popular.

But overall, my worry is not performances, or symmetry with respect to 
other APIs - my main worry is what I said above: that people will just 
assume that indices are logical for both the segment and the array - so 
overloads with clearly different names should help quite a bit IMHO.

Maurizio

On 03/06/2021 20:50, leerho wrote:
> Maurizio,
> Here is the MemoryCopy class (w/o byte swap) and the TestMemoryCopy 
> class (w/o testing byte swap).
>
> I copied from the TestMemoryAccess class in that I noticed that there 
> is no package statement.  I am not sure what @SuppressWarnings are 
> allowed in your test environment, I suspect I should remove all of them.
>
> When the MemorySegment copyFrom(MemorySegment, MemoryLayout, 
> ByteOrder) becomes available I can add the tests for that.
>
> *IMHO*
>
>     I feel strongly that there is no need to additionally overload
>     these methods with the segment offset specified in array index
>     units.  It is so trivial to convert from one to the other and it
>     can be done easily in the method argument with a simple
>     multiply by 2, 4 or 8. This should compile down to a simple shift,
>     which becomes a single cycle CPU instruction.  So this is not a
>     performance issue.
>
>     Furthermore, it is best that the user becomes accustomed to
>     thinking of a segment fundamentally as an array of bytes.  Once a
>     segment is loaded with some primitive array, the segment loses the
>     context of the type of the array it was loaded with; in effect, a
>     kind of "type erasure" similar to Java's generics.  This is one of
>     the reasons that MemorySegments can be so powerful.
>
>
>     I have been programming with the "MemorySegment" concept for a
>     number of years now and find that keeping in mind that segments
>     are just bytes is very useful.
>
>
> Let me know what you think.
>
> Lee.
>
>
>
> On Mon, May 31, 2021 at 6:56 AM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com 
> <mailto:maurizio.cimadamore at oracle.com>> wrote:
>
>     Hi Lee,
>     this looks good.
>
>     One thing I note is that there's an ambiguity as to whether the
>     segment index is expressed as a logical index, or a raw byte
>     offset. Your snippet does the latter. If we want to follow
>     MemoryAccess, perhaps that calls for 2 overloads
>     (copyFromArrayAtIndex/copyFromArrayAtOffset), as I imagine both
>     could be useful, depending on the case?
>
>     The javadoc will have to say something when the segment being used
>     is backed by the very array that is the source/target of the copy
>     (we have some text like that in MemorySegment::copyFrom).
>
>     I think it would be helpful to progress further with this, add the
>     remaining templates (w/o ByteOrder, for now) test and see how it
>     works in practice.
>
>     I will add (or just file a simple PR, so that you can just borrow
>     from it - should be a single method) something to do the segment
>     copy with swap.
>
>     Thanks!
>     Maurizio
>
>     On 28/05/2021 22:37, leerho wrote:
>>     Maurizio,
>>
>>     Again sorry about the delay.
>>
>>     Attached is a template proposal for the MemoryCopy class.
>>
>>     I can't complete this without your proposed new copyFrom(...)
>>     method in MemorySegment. As it is written, it should work, but
>>     without the byte swap capability.
>>
>>     I can complete the rest of the primitives like this template, if
>>     you would like with javadocs.  I could also start writing tests,
>>     but without the byte-swap.
>>
>>     Let me know what would be most helpful.
>>
>>     Cheers,
>>
>>     Lee
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>     On Thu, May 20, 2021 at 11:36 AM leerho <leerho at gmail.com
>>     <mailto:leerho at gmail.com>> wrote:
>>
>>         Of course!
>>
>>         On Thu, May 20, 2021 at 9:42 AM Maurizio Cimadamore
>>         <maurizio.cimadamore at oracle.com
>>         <mailto:maurizio.cimadamore at oracle.com>> wrote:
>>
>>
>>             On 20/05/2021 17:13, leerho wrote:
>>>             I am not sure if the /dstSegSlice/ requires the
>>>             /srcCopyLen/. I would hope that it is smart enough to
>>>             realize that the input length is smaller than the given
>>>             offset minus the segment size.
>>
>>             asSlice has an overload that just takes an offset and
>>             infers the resulting size from there.
>>
>>             But that doesn't seem what you want here - as you want
>>             the slice to have a specific size (the size of the input
>>             array).
>>
>>             MemorySegment::copyFrom wants the two segments to have
>>             the same size, so I think you need that.
>>
>>             In terms of performance, there's no difference between
>>             asSlice(offset) and asSlice(offset, size) - you have to
>>             create a new segment anyway.
>>
>>             Maurizio
>>
>>