[foreign-memaccess] RFR: JDK-8242495: Restructure implementation of memory segments

[Paul Sandoz]

On Fri, 10 Apr 2020 15:25:50 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Currently memory segments use a monomorphic implementation. This implementation doesn't differentiate between heap
>> segments and native segments and, as a result can lead to type profile pollution.
>> Moreover, for native segment, the accessor to the unsafe base object is redundant (e.g. base is always null), but we
>> can't easily shortcircuit that if we use a monomorphic implementation.
>> Finally, it would be desirable, at some point, to add public sub-interfaces of MemorySegment, such as
>> MappedMemorySegment which could provide a force() method. Unfortunately it would be cumbersome to do that with an
>> underlying monomorphic implementation, unless we make force() total on all segments (this work will likely be addressed
>> in a separate, follow-up PR).  This patch introduces a new abstract class which contain most of the memory segment
>> implementation (it is essentially the same as the old MemorySegmentImpl).  Three subclasses are also added:
>> 
>> * NativeMemorySegmentImpl - to model native segments (base() returns null)
>> * HeapMemorySegmentImpl - to model heap segments (base() returns some array)
>> * MappedMemorySegmentImpl - to model memory mapped segments (this extends from NativeMemorySegment)
>> 
>> Each subclass overrides a dup() method, which must be used by the abstract subclass to implement view methods in a
>> generic way. Each subclass also defines what base() and min() look like.
>> Since now we have different segment implementation classes, I also decided to move all the factory methods from Utils
>> to the segment impl class they belong, which leads to cleaner code.
>> Note that, for heap segment there is a subtle trick: instead of storing the `base` object (an array) we store
>> a *lambda* which retrieves the base object. This allows us to retain a monomorphic heap segment implementation, but,
>> thanks to lambdas, we get sharply typed accessors to the base object, which helps out C2 when determining whether
>> memory barriers should be added or not. This approach strikes a good balance between performance and code readability.
>> Should we find cases where this approach is not enough, we could always fall back to create separate subclasses for
>> each array type.  I've added benchmarks for both heap and mapped segments (they borrow heavily from the current
>> LoopOverNonConstant), so that at least we can check performances in all cases. Numbers look good and similar across the
>> whole spectrum of segments. As a comparison, before this patch, heap segments were roughly 10x slower than off-heap
>> segments.  This patch also tidies up the byte buffer support; that is, it make sure that when going from a buffer to a
>> segment (and back) all the correct information is preserved; while, before this patch, this was the case for native vs.
>> heap segments, information about mapped segments was lost (as a "normal" direct buffer was created out of a mapped
>> segment). This was also true when creating a segment from a mapped buffer - where a regular native segment was created.
>> While semantics of access operations was not impacted by the aforementioned problems, calling force() on a buffer
>> derived from a mapped segment might not behave as expected; this patch fixes that. To make sure that information is
>> correctly preserved, I have enhanced the byte buffer test.
>
> test/micro/org/openjdk/bench/jdk/incubator/foreign/ParallelSum.java line 189:
> 
>> 188:
>> 189:     static class SumSegment extends CountedCompleter<Integer> {
>> 190:
> 
> I've updated the parallel test to use a non-recursive CountedCompleter instead of a RecursiveAction, since the
> RecursiveAction was generating a lot of noise during JMH benchmark, which made it very hard to validate performances.

Oh, I missed that, yes that's better.

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

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