[foreign-memaccess] RFR: 8254343: Revisit API for supporting mapped memory segments

Sat Oct 10 16:34:31 UTC 2020

Whipped up a quick prototype: 
https://github.com/openjdk/jdk/compare/master...JornVernee:Exact_VarHandle

This doesn't implement asGeneric() yet but that should be straightforward.

This is implement using a boolean flag on VarForm, and saving the exact 
method types as well as the erased ones (both are needed to be able to 
convert back and forth). If the flag is set, an extra check is done in 
VarHandleGuards (not sure this is currently 100% correct currently, 
since the generator script was not updated the last time around, so I 
had to do some ad hoc tinkering).

This works on a simple test case like this:

MemoryLayout layout = MemoryLayout.ofSequence(10, JAVA_INT);
VarHandle vh = layout.varHandle(int.class, MemoryLayout.PathElement.sequenceElement());
//vh = vh.asExact();
try (MemorySegment segment = MemorySegment.allocateNative(layout)) {
     for (int i =0; i <10; i++) {
         vh.set(segment.baseAddress(), i, i);
     }
}

If you un-coment the line with asExact() you should get:

Exception in thread "main" java.lang.invoke.WrongMethodTypeException: 
expected (Object,long,int)void but found (MemoryAddress,int,int)void
     at 
java.base/java.lang.invoke.VarHandleGuards.guard_LII_V(VarHandleGuards.java:862)
     at main.Main.main(Main.java:18)

Cheers,
Jorn

On 10/10/2020 16:58, forax at univ-mlv.fr wrote:
> ----- Mail original -----
>> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>> À: "Remi Forax" <forax at univ-mlv.fr>, "Maurizio Cimadamore" <mcimadamore at openjdk.java.net>, "Paul Sandoz"
>> <Paul.Sandoz at oracle.com>
>> Cc: "panama-dev at openjdk.java.net'" <panama-dev at openjdk.java.net>
>> Envoyé: Samedi 10 Octobre 2020 14:34:13
>> Objet: Re: [foreign-memaccess] RFR: 8254343: Revisit API for supporting mapped memory segments
>> On 10/10/2020 12:20, Remi Forax wrote:
>>> Six month ago, i had to track a perf issue due to a VarHandle doing the asType()
>>> conversion at runtime when performing a CAS.
>>>
>>> I wonder if we should not introduce a way to have "exact" VarHandle that throw a
>>> runtime exception instead of doing a asType() conversion when performing an
>>> action,
>>> because as a user if I'm using the complex VarHandle API is because i want
>>> performance.
>> Yes, that's something I know Jorn has been thinking about (e.g. adding
>> an 'asExact' adapter).
>>
>> That said, the question here is whether we should have an API which
>> potentially leads the user down the wrong (inexact) path.
>>
>> IMHO, having a polymorphic MemorySegment access coordinate that you are
>> going to use with a VarHandle is a recipe for trouble; having a single
>> exported MemorySegment type at least minimizes the chance of these kind
>> of mistakes. In other words, the API should strive, as much as possible,
>> to drive you towards correct and performant code. It seems like that's
>> not the case currently.
>>
>> So, I don't think it's an "either or" situation: I'm for having an API
>> which minimizes mistakes *and* for having some sort of "exact" var
>> handle combinator (which might be useful when operating on lots of
>> different coordinates).
> yes,
> at the same time you can not change the behavior of Lookup.findVarHandle()/findStaticVarHandle,
> you can only fix the VarHandle created on a MemorySegment.
>
> so it seems that the VarHandle created on a MemorySegment should be exact and that two new instance methods asExact and asGeneric should be added to VarHandle to be able to move from one semantics to the other.
>
>> Maurizio
>>
> Rémi
>
>>> ----- Mail original -----
>>>> De: "Maurizio Cimadamore" <mcimadamore at openjdk.java.net>
>>>> À: "panama-dev at openjdk.java.net'" <panama-dev at openjdk.java.net>
>>>> Envoyé: Vendredi 9 Octobre 2020 23:15:18
>>>> Objet: [foreign-memaccess] RFR: 8254343: Revisit API for supporting mapped
>>>> memory segments
>>>> As I was polishing the memory access API ahead of the upstream integration, I
>>>> realized of a fatal flaw with the
>>>> `MemorySegment` vs. `MappedMemorySegment` split. This split works against the
>>>> idiomatic usage of the VarHandle API:
>>>> since memory access var handles feature a `MemorySegment` coordinate, clients
>>>> using a `MappedMemorySegment` will be
>>>> using *inexact* access mode (which goes through an extra, expensive `asType`
>>>> adaptation).
>>>>
>>>> The only way to remove the perfomance issue is to make deep changes in the
>>>> VarHandle machinery to support a more lax
>>>> form of `asType` adaptation, but adding this to support mapped segments seems,
>>>> frankly, overkill.
>>>>
>>>> On top of that, while discussing this with Paul, I realized that no liveness
>>>> check was performed on the load()/unload()
>>>> operation, and that these operations, more generally, did not get the `@Scoped`
>>>> treatment, meaning they will crash the
>>>> VM when  used with shared segments.
>>>>
>>>> This patch rectifies these issues; it removes the `MappedMemorySegment`
>>>> interfaces, and it introduce a `MemoryMapping`
>>>> abstraction instead. Clients can ask a memory mapping out of a segment; the
>>>> memory mapping will contain the usual
>>>> `load`/`force` operation, so there's no loss in expressiveness of the API.
>>>>
>>>> For instance, to force contents of a mapped segment to be written in the
>>>> underlying file, a client can do:
>>>>
>>>> segment.asSlice(20, 100)
>>>>         .mapping()
>>>>         .ifPresent(MemoryMapping::force);
>>>>
>>>> We believe this to be a reasonable compromise, which allows us to keep a single
>>>> MemorySegment interface (instead of
>>>> two), which removes the var handle performance issue, and in turn enables other
>>>> improvements, such as turning the
>>>> `spliterator` method into a true instance method.
>>>>
>>>> -------------
>>>>
>>>> Commit messages:
>>>> - Revamp API for mapped segments
>>>> - Add scope checks to MappedMemorySegment operations
>>>>
>>>> Changes: https://git.openjdk.java.net/panama-foreign/pull/377/files
>>>> Webrev: https://webrevs.openjdk.java.net/?repo=panama-foreign&pr=377&range=00
>>>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8254343
>>>>    Stats: 501 lines in 10 files changed: 295 ins; 154 del; 52 mod
>>>>    Patch: https://git.openjdk.java.net/panama-foreign/pull/377.diff
>>>>    Fetch: git fetch https://git.openjdk.java.net/panama-foreign
>>>>    pull/377/head:pull/377
>>>>
>>>> PR: https://git.openjdk.java.net/panama-foreign/pull/377