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

[forax at univ-mlv.fr]

----- 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