[foreign-memaccess] API refinements

[Maurizio Cimadamore]

Hi,
during the code review for the foreign memory access API (see [1]), 
several issues were raised; these are now tracked in [2].

Before doing any code change, I'd like to get some shared consensus on 
the direction we should take. Here is a list of the issues:

* Should MemoryAddress implement Comparable?

I believe the answer here should be NO. In general comparing addresses 
is tricky - and it seems to me that, given that MemoryAddress has a 
well-defined equals() behavior, you can still implement e.g. loops where 
you want to keep offsetting an address until you reach a certain limit.

As to whether MemorySegment should implement Comparable, I believe 
answer should also be NO. I know that ByteBuffer implements Comparable, 
and it has method to compare the contents, and to find the first index 
at which two byte buffers mismatch. But I think this is a very marginal 
use case for the memory access API, and there's always an escape hatch: 
project segments down to a byte buffer and take it from there.

* should we merge MemoryLayouts and MemoryLayout?

I'm on the fence on this one. The fact that we have a MemoryLayouts is 
very much driven by the fact that the API lives in an incubating module, 
and that some layouts cannot live where they should (e.g. JAVA_INT 
should really be Integer::LAYOUT). Since we also have a MemoryHandles 
(which will be replaced by some methods inside MethodHandles), I really 
don't see a big reason as to why we should refactor the API at this 
point. E.g. this seems to be more a question that we should ask again 
when the API is finalized, and exits incubation.

* MemorySegment::acquire() -- replace with MemorySegment::fork?

I came up with that - and I think I still like it. Acquire seems to 
suggest that it has something to do with threads and ownership - which 
it does, but in reality you can acquire a segment even from same owner. 
Fork seems a more neutral term, and, to me helps better visualizing 
what's really going on (e.g. a tree of segments is being created).

* should we have a way to ask a Layout if its size is specified ?

This is a tricky one. On the one hand, if not all layouts have a size, 
one could argue that Layout::bitSize should return an optional. But that 
seems excessively punitive for the vast majority of layouts that are 
never going to use unbound sequences. This might leads down to two 
conclusions:

- maybe unbound sequences do not carry enough weight - perhaps replace 
them with a zero-length sequence, and be done - or even get rid of 
zero-length sequences and let layout only be for things whose size is 
defined. (and maybe come back with a better mechanism to speak about 
"size holes" later, and variable-length layouts in general)

- if we keep unbound sequences, let's just add an hasSize() predicate

Of the two I'm warming up with just ditch unbound sequences period (e.g. 
all layouts must have a well-defined size).

* There doesn't seem to be a predicate for "can I call close without an 
exception?" (isAlive && isAccessible) isn't sufficient.

I think the answer here is - and that's FINE. I'm of the opinion that if 
an exception occurs when calling close(), 90% it signals a bug in the 
code, not just something that you want to proactively check against. 
E.g. somebody else forked your segment w/o you knowing. On top of that, 
I don't think that, even if we wanted to, there would be a way to do a 
'canClose()' check which would mean anything - that is, the predicate 
could return true, but then, what if, just before you call close() 
another thread re-acquires the segment? I think this is a lost cause.

* MemoryAddress::offset() vs. MemoryAddress::offset(long) -- not much 
distance between these two semantically different operations

I think the solution that I like here is to drop MemoryAddress::offset() 
completely, and to provide a new functionality to create a slice based 
on a given address (which is the primary motivation for having the 
offset() method). Here we have two options:

- MemoryAddress::asSlice(long bytes) -> MemorySegment
- MemorySegment::asSlice(MemoryAddress base, long bytes)

The former leads to more compact code, but it feels a bit dirtier - 
anyway, I don't feel too strongly here.

* MemorySegment::isAccessible() -- as the A* word is overloaded, some 
other name should be found?

Yes, the A-word is a mess - but some of the proposals I've see (e.g. 
"isOwnedByCurrentThread") look equally terrible. If somebody has a 
better alternative I'd be happy to hear it - if not, I think keeping the 
A-word is tolerable.


Thoughts?

Maurizio

[1] - 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/063836.html
[2] - https://bugs.openjdk.java.net/browse/JDK-8235837