JEP 370 Feedback

[Maurizio Cimadamore]

Hi Philippe,
first, thanks a lot for the feedback, much appreciated. Some responses 
inline below.

On 20/04/2020 20:36, Philippe Marschall wrote:
> Hello
>
> I'm not sure whether this is the right channel. I had a quick look at
> JEP 370 and since it's an incubator feature I wanted to take the
> opportunity to provide some feedback:
>
> The biggest limitation I'm currently seeing is the missing ability to
> deal with MemorySegment on the native / JNI side. Mainly I'm missing
> equivalents of the following JNI functions:
>
> - NewDirectByteBuffer
> - GetDirectBufferAddress
> - GetDirectBufferCapacity

This is a good point - (direct) ByteBuffers can be allocated from JNI 
code - but memory segments cannot. Similarly, it would be useful to have 
functions to get the base address of a segment, or the raw address of a 
MemoryAddress.

That said, I'm a bit confused when I see the comments below:

>
> These functions are required when:
>
> - libraries expect memory to be allocated and released using library
> specific functions
> - allocations need to happen with specific flags currently not supported
> by the JDK
> - a native thread calls into Java code
>
> Yes, this can be achieved by going through a ByteBuffer, however this
> has the following disadvantages:

This seems to hint at more than just having plain memory segment support 
from JNI - it seems as if you'd like to create your own custom memory 
segments.

As I pointed out in other emails, a segment is really made up of many 
pieces:

* spatial bounds (e.g a size)
* temporal bounds (info as to whether the segment is still alive)
* the cleanup action associated with the MemorySegment::close operation
* confinement thread (the thread that is allowed to operate on the segment)
* some access modes (e.g. read-only, etc.)

So, an API like newDirectByteBuffer lets you specify the spatial bounds 
(base address + length) and that's pretty much it. ByteBuffer has no 
thread ownership, so that's not a detail to be worried about there. And, 
byte buffer created that way also have no associated cleanup action.

A very similar operation to newDirectByteBuffer is provided by the 
method on Foreign in the latest Panama branch:

https://github.com/openjdk/panama-foreign/blob/foreign-jextract/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/Foreign.java#L80

This method takes a MemoryAddress and creates a segment of a certain 
size under it, w/o any confinement. The segment cannot be closed, and 
it's always alive.

We could expose something like that in JNI. But that won't be as 
powerful as what you are asking for. And, at the same time, given that 
the API we have are enough to take a 'long' and turn it into some kind 
of an unchecked segment, are you sure that you would prefer doing that 
using JNI rather than in Java? If so, may I ask why?


> - a DirectByteBuffer has to be allocated which is noticeable for small
> allocations
> - no timely release of memory through try-with-resources, instead a
> custom native method has to be provided and called
>
> Should these be added it would be helpful if the equivalent of
> NewDirectByteBuffer would take a function pointer to a release function
> and optionally an attachment which will be called by 
> MemorySegment#close().

The question of building completely customizable (native) segments comes 
up from time to time; it is a tricky balance - on the one hand I 
obviously see the appeal of going down that path; on the other hand I 
also think it's a slippery slope, as it could encourage abusing of the 
memory segment abstraction which is intended to be a relatively tight 
abstraction.

My sense is that in some of the cases where, e.g. we use a direct buffer 
(or, in the future, a segment) to represent a simple struct, we might 
end up being tempted to model C APIs that feature symmetric functions like

allocateFoo -> Foo*
destroyFoo(Foo*)

as a MemorySegment whose cleanup action ends up calling "destroyFoo". 
While this will technically work, I believe that, if a programmer wants 
a Foo abstraction, such an abstraction should be coded explicitly - e.g. 
by wrapping a segment, or an address:

class Foo implements AutoCloseable {
     MemorySegment _segment;

     void close() { // my cleanup logic
}

Note that the future steps of Panama will make it a lot easier to call 
some custom native library function using a "native method handle", so 
coding such abstractions will, I think, be better than using opaque 
MemorySegment all over the place where who reads the code then will have 
to guess what happens when MemorySegment::close is called.

>
> How would incubator features be made available to JNI code? Through a
> #define? Through a different header?
>
> What would also be helpful is the ability to efficiently convert a
> MemorySegment to a Java String without an intermediate byte[] or a NIO
> Buffer used by a Charset.

We are starting to do some work in that direction - as part of the 
native interop work, but for now we're creating an intermediate array.


>
> I had a quick, unscientific look at performance compared to
> malloc/calloc/free with NewDirectByteBuffer and it looks as if the
> performance of JEP 370 compares favorably. Were I did find a performance
> deficit was when comparing with large allocations using mmap and large
> pages, either single threaded or multi-threaded with the allocation size
> matching the page size (2M). This is not unexpected.

The allocation performances are basically bound by what malloc does; I 
agree that in the general case users might have different requirements 
when it comes to allocation, so it makes sense to investigate how to 
allow the memory segment API to play nice with such allocators. But, see 
above, doing that in a way that is _safe_ (e.g. so that no crash can 
occur in the Java code) typically involves restricting the flexibility 
of how memory segment can be constructed. And, there is a fine line 
between "use a segment to wrap the memory regions generated by your 
custom allocator" (which is a fine - albeit unsafe, in the general case 
- use case) and "use a segment to wrap domain model abstractions" (which 
is, I think, not fine at all).

What we have tried to do in the Foreign class was to support the 
relevant use cases from a native interop perspective, w/o trying to go 
too overboard with customization.

Maurizio

>
> Cheers
> Philippe