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

[leerho]

Hello Maurizio,
I can see that "MemoryCopy" has now been nicely integrated into
MemoryAccess.  It looks good.

I have one question and a plea:

   - Do you still plan on adding a "isOverlapping(Segment)" to
   MemorySegment?  As I mentioned before, this simplifies the code required
   when attempting to copy elements between segments where the elements are
   not primitives.
   - A while ago I requested a simple empty Interface (added to Scope) that
   could be extended by the user to provide a call back mechanism between the
   code operating on a segment/slice and the higher-level code that is
   controlling the scope.  This would allow, for example, a simplified way for
   the lower level code to request a larger segment to operate with.  Both
   MemorySegment and ResourceScope are now sealed interfaces, which would lock
   out any possibility for users to add this kind of functionality later.  I
   realize this could also be provisioned as a separate argument passed
   between the higher-level code and the lower-level code, but this is not as
   elegant as having this simple mechanism built into the Java code.

Cheers,
Lee.


On Fri, Aug 6, 2021 at 3:16 PM Uwe Schindler <uschindler at openjdk.java.net>
wrote:

> On Fri, 6 Aug 2021 17:01:10 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.
> >
> > Maurizio Cimadamore has refreshed the contents of this pull request, and
> previous commits have been removed. The incremental views will show
> differences compared to the previous content of the PR. The pull request
> contains one new commit since the last revision:
> >
> >   Revert name of accessors in MemoryAccess
>
> Hi,
> I tested the latest variant with Lucene. Performance seems OK, heap
> profile is also fine (identical to our old bytebuffer code, so no extra
> allocations by panama). Code:
> https://github.com/uschindler/lucene/blob/7ed6da79239fa8f860ad58bf5568d0001927b6a2/lucene/core/src/java/org/apache/lucene/store/MemorySegmentIndexInput.java#L148-L211
>
> I am fine with both APIs in memory access, although I liked the first
> variant using write/read a bit more. But I have seen the same problems with
> order of parameters.
>
> -------------
>
> PR: https://git.openjdk.java.net/panama-foreign/pull/568
>