[foreign-memaccess+abi] RFR: 8270376: Finalize API for memory copy [v7]
Maurizio Cimadamore
mcimadamore at openjdk.java.net
Fri Aug 6 14:22:46 UTC 2021
On Fri, 6 Aug 2021 14:09:22 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 updated the pull request incrementally with two additional commits since the last revision:
>
> - Fix javadoc for MemoryAccess
> - Consolidate all access routines inside MemorySegments
I ended up with a pretty drastic rewrite, as I was not too happy with how the code was laid out. After trying out a number of different solutions, I realized that my main beefs were:
1. Operations pertaining to segments were spread into multiple class (MemoryCopy and MemoryAccess)
2. The number of operations offered by MemoryAccess seemed too big (this has been mentioned before - e.g. by index vs. by offset)
3. The role of copyFrom in the new world was not very clear
For these reasons I decided to:
1. only keep offset-based accessors in MemoryAccess - this reduces greatly the amount of methods
2. thanks to (1) we can now move MemoryCopy methods in MemoryAccess as well - the idea is that al the methods in MemoryAccess are about reading/writing Java values from/to memory segments (either a single value or in bulk), so I think the move makes a lot of sense
3. I renamed all accessors to use readByte/writeByte (for single access) and readBytes/writeBytes (for bulk access). This helps seeing the various methods as different ways to read/write a segment (the name read/write, especially in copy operation always refer to a segment - this is similar to what the ByteBuffer API already does).
4. I've turned the static copy method in MemorySegment back to instance methods - e.g. back to `copyFrom`. These are the "real" copy primitives, and everything else can be derived from there (of course the static accessors don't really do that, to avoid creation of garbage objects, but that's the spirit).
I think this makes the API more compact, more usable and easier to explain.
In a followup PR, I also plan to drop most of the allocation methods from SegmentAllocator. I think it would be better to keep SegmentAllocator a neutral abstraction, while at this point in time it still has stuff from NativeScope, in an attempt to ease the native interop use cases. I think jextract is in a better position to help there, and we will do something to counteract this loss.
A javadoc of the new MemoryAccess class is available here:
http://cr.openjdk.java.net/~mcimadamore/panama/memorycopy_finalize_javadoc/javadoc/jdk/incubator/foreign/MemoryAccess.html
-------------
PR: https://git.openjdk.java.net/panama-foreign/pull/568
More information about the panama-dev
mailing list