RFR JDK-8234049: Implementation of Memory Access API (Incubator)

[Maurizio Cimadamore]

Hi,
as part of the effort to upstream the changes related to JEP 370 
(foreign memory access API) [1], I'd like to ask for a code review for 
the corresponding core-libs and hotspot changes:

http://cr.openjdk.java.net/~mcimadamore/panama/8234049/

A javadoc for the memory access API is also available here:

http://cr.openjdk.java.net/~mcimadamore/panama/memaccess_javadoc/jdk/incubator/foreign/package-summary.html

Note: the patch passes tier1, tier2 and tier3 testing (**)


Here is a brief summary of the changes in java.base and hotspot (the 
remaining new files are implementation classes and tests for the new API):

* ciField.cpp - this one is to trust final fields in the foreign memory 
access implementation (otherwise VM doesn't trust memory segment bounds)

* Modules.gmk - these changes are needed to require that the incubating 
module is loaded by the boot loader (otherwise the above changes are 
useless)

* library_call.cpp - this one is a JIT compiler change to treat 
Thread.currentThread() as a well-known constant - which helps a lot in 
the confinement checks (thanks Vlad!)

* various Buffer-related changes; these changes are needed because the 
memory access API allows a memory segment to be projected into a byte 
buffer, for interop reasons. As such, we need to insert a liveness check 
in the various get/put methods. Previously we had an implementation 
strategy where a BB was 'decorated' by a subclass called ScopedBuffer - 
but doing so required some changes to the BB API (e.g. making certain 
methods non-final, so that we could decorate them). Here I use an 
approach (which I have discussed with Alan) which doesn't require any 
public API  changes, but needs to add a 'segment' field in Buffer - and 
then have constructors which keep track of this extra parameter.

* FileChannel changes - these changes are required so that we can reuse 
the Unmapper class from the MemorySegment implementation, to 
deterministically deallocate a mapped memory segment. This should be a 
'straight' refactoring, no change in behavior should occur here. Please 
double check.

* VarHandles - this class now provides a factory to create memory access 
VarHandle - this is a bit tricky, since VarHandle cannot really be 
implemented outside java.base (e.g. VarForm is not public). So we do the 
usual trick where we define a bunch of proxy interfaces (see 
jdk/internal/access/foreign) have the classes in java.base refer to 
these - and then have the implementation classes of the memory access 
API implement these interfaces.

* JavaNIOAccess, JavaLangInvokeAccess - because of the above, we need to 
provide access to otherwise hidden functionalities - e.g. creating a new 
scoped buffer, or retrieving the properties of a memory access handle 
(e.g. offset, stride etc.), so that we can implement the memory access 
API in its own separate module

* GensrcVarHandles.gmk - these changes are needed to enable the 
generation of the new memory address var handle implementations; there's 
an helper class per carrier (e.g. VarHandleMemoryAddressAsBytes, ...). 
At runtime, when a memory access var handle is needed, we dynamically 
spin a new VH implementation which makes use of the right carrier. We 
need to spin because the VH can have a variable number of access 
coordinates (e.g. depending on the dimensions of the array to be 
accessed). But, under the hood, all the generated implementation will be 
using the same helper class.

* tests - we've tried to add fairly robust tests, often checking all 
possible permutations of carriers/dimensions etc. Because of that, the 
tests might not be the easiest to look at, but they have proven to be 
pretty effective at shaking out issues.

I think that covers the main aspects of the implementation and where it 
differs from vanilla JDK.

P.S.

In the CSR review [2], Joe raised a fair point - which is MemoryAddress 
has both:

offset(long) --> move address of given offset
offset() --> return the offset of this address in its owning segment

And this was considered suboptimal, given both methods use the same name 
but do something quite different (one is an accessor, another is a 
'wither'). one obvious option is to rename the first to 'withOffset'. 
But I think that would lead to verbose code (since that is a very common 
operation). Other options are to:

* rename offset(long) to move(long), advance(long), or something else
* drop offset() - but then add an overload of MemorySegment::asSlice 
which takes an address instead of a plain long offset

I'll leave the choice to the reviewers :-)



Finally, I'd like to thank Mark, Brian, John, Alan, Paul, Vlad, Stuart, 
Roger, Joe and the Panama team for the feedback provided so far, which 
helped to get the API in the shape it is today.

Cheers
Maurizio

(**) There is one failure, for "java/util/TimeZone/Bug6329116.java" - 
but that is unrelated to this patch, and it's a known failing test.

[1] - https://openjdk.java.net/jeps/370
[2] - https://bugs.openjdk.java.net/browse/JDK-8234050