[foreign-memaccess+abi] RFR: 8273905: Foreign API refresh [v4]

[Uwe Schindler]

On Fri, 17 Sep 2021 17:16:58 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> As outlined in [1], there are areas of the foreign API we'd like to improve upon, such as:
>> 
>> * dereference: there is a mismatch between API points which create segments (which take a layout) and dereference API points, which do not take layouts
>> 
>> * role of `MemoryAddress`: in Java 17, `MemoryAddress` has become a stateful carrier, which is attached to a scope. This is inconvenient, as in most cases, a memory address is just a raw pointer that arises when interacting with native code.
>> 
>> * resource scopes: the API for scopes has too many flavors to pick from, many of which overlap. The fact that scopes and segment allocators are unrelated forces clients to introduce ad hoc conversions from scopes to allocators, and library developers to add overloads. Finally, the API for acquiring scopes doesn't work well with try-with-resources, and could also be simplified. 
>> 
>> I will add separate comments to explain how the API has changed to resolve the above issues.
>> 
>> [1] - https://mail.openjdk.java.net/pipermail/panama-dev/2021-September/014946.html
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup up code, following some IDEA suggestions.
>   * unused imports
>   * suggestions for new language features
>   * drop unused impl declarations
>   * removed size constructor parameter in value layout subclasses (except `OfAddress`)

I found some minor javadoc things (just by looking at it and searching for defintiions, so there may be other problems).

I did some testing in Apache Lucene with a build of your branch: https://github.com/uschindler/lucene/tree/draft/jdk-foreign-mmap-jdk18 (the relevant change to previous version using the old MemoryCopy stuff is this commit: https://github.com/uschindler/lucene/commit/2de65069a7419edd43f5fc3b63e6e9fa077e2235

The new API looks really good. I also removed all usage of `VarHandle` in the Lucene implementation and just used the versions of `MemorySegment#get(ValueLayout.Xxx,...)` for our positional reads. Interestingly the performance seemed to get a bit better without direct use of VarHandles (maybe because you use `@ForceInline`), but not significant (inside the benchmark's std dev).

The most interesting code for you is here: https://github.com/uschindler/lucene/blob/draft/jdk-foreign-mmap-jdk18/lucene/core/src/java/org/apache/lucene/store/MemorySegmentIndexInput.java

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java line 183:

> 181:      * <blockquote><pre>{@code
> 182:     MemorySegment segment = MemorySegment.allocateNative(2, ResourceScope.newConfinedScope());
> 183:     VarHandle SHORT_VH = ValueLayout.JAVA_SHORT.varHandle(short.class);

The parameter `short.class` in this example is obsolete with new API.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 873:

> 871:      *                might affect the behavior of the returned memory mapped segment (see {@link #force()}).
> 872:      * @param scope the segment scope.
> 873:      * @return a new mapped memory segment.

Thanks for this change (and the others around the returned segment scope)!

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

Marked as reviewed by uschindler (no project role).

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