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

[Maurizio Cimadamore]

On 11/08/2021 22:09, leerho wrote:
> 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.
>
That is still on the cards, yes.
>
>   * 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.
>
ResourceScope is too low level to allow for extensibility - it is tied 
with VM internals, especially in its handling of shared scope closure.

I have to admit, I have trouble understanding what you are proposing - 
e.g. what this callback would be for. What calls to resource scope do 
you wish to intercept?

Maurizio


> Cheers,
> Lee.
>
>
> On Fri, Aug 6, 2021 at 3:16 PM Uwe Schindler 
> <uschindler at openjdk.java.net <mailto:uschindler at openjdk.java.net>> wrote:
>
>     On Fri, 6 Aug 2021 17:01:10 GMT, Maurizio Cimadamore
>     <mcimadamore at openjdk.org <mailto: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
>     <https://urldefense.com/v3/__https://github.com/uschindler/lucene/blob/7ed6da79239fa8f860ad58bf5568d0001927b6a2/lucene/core/src/java/org/apache/lucene/store/MemorySegmentIndexInput.java*L148-L211__;Iw!!ACWV5N9M2RV99hQ!cJbHnhybyFH65EJVILSVCJ6LPvrUMdKybd0OaX1LOdR0oFD8nbKXZVCy_LxASZapcmg6lV0$>
>
>     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
>     <https://git.openjdk.java.net/panama-foreign/pull/568>
>