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

[Maurizio Cimadamore]

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

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