[foreign-memaccess] RFR: 8249879: Split MemorySegment and MemoryAddress [v3]

[Maurizio Cimadamore]

On Mon, 27 Jul 2020 11:00:29 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address Paul's review comments.
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java line 57:
> 
>> 56:         MemoryLayout.ofValueBits(32, ByteOrder.BIG_ENDIAN).withName("value")
>> 57: ));
>> 58:  * }</pre></blockquote>
> 
> There's one too many closing parenthesis here, and the sentence above still talks about a SequenceLayout, which is now
> removed.

Will fix

> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 300:
> 
>> 299:      */
>> 300:     default MemorySegment asSlice(MemoryAddress address) {
>> 301:         return asSlice(address.segmentOffset(this));
> 
> FWIW the `MemorySegment asSlice(MemoryAddress address, long newSize)` overload is still missing from the matrix.

We can add that at a later stage

> src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryAddressImpl.java line 50:
> 
>> 49:     @Override
>> 50:     public long segmentOffset(MemorySegment segment) {
>> 51:         Objects.requireNonNull(segment);
> 
> Maybe this makes more sense on MemorySegment as `offsetOf(MemoryAddress)` ? WDYT?

Honestly I like it more the way it is, I'd prefer to consider after seeing some of the use cases.

> test/micro/org/openjdk/bench/jdk/incubator/foreign/LoopOverNonConstantMapped.java line 149:
> 
>> 148:         for (int i = 0; i < ELEM_SIZE; i ++) {
>> 149:             res += MemoryAccess.getIntAtIndex(segment, i * CARRIER_SIZE);
>> 150:         }
> 
> Why this multiply by CARRIER_SIZE? That should be done by the accessor.

mistake - will fix (but this has nothing to do with this patch, I think?)

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/260