[foreign-memaccess+abi] Integrated: 8274285: By reference parameters should be kept alive during downcalls

[Maurizio Cimadamore]

On Fri, 24 Sep 2021 13:10:44 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch adds an adapter around the downcall method handle which keeps alive all by-reference arguments by acquiring/releasing their associated scopes.
> 
> I've implemented the adapted in a separate method in `SharedUtils`, because `SharedUtils::wrapWithAllocator` is relatively complex, and has mostly to do with creating a scope and closing it after the call.
> Also, `wrapWithAllocator` is only called when specialized method handles are active, which means adding logic there doesn't fix the programmable fallback case.
> 
> Since we plan to do more implementation restructuring in this area, I preferred to keep things simple for now.
> 
> Also note that the acquire/release logic isn't too smart - if multiple arguments backed by the same scoped are passed, we do separate acquire/release for each of them. Again, I didn't want to spend too many cycles given that the implementation of this area might change, so my aim for now was to make things safe; we can improve efficiency later.
> 
> Here are some benchmarks, for now, which compare the difference between passing a struct by reference using the struct's address (e.g. calling `MemorySegment::address` before the downcall), vs. passing the struct directly. In the first case, the scope is the global scope, so acquiring/relesing is a no-op. In the latter case we have a real scope, which we need to acquire and release.
> 
> 
> Benchmark                                                           Mode  Cnt   Score   Error  Units
> CallOverheadConstant.panama_identity_memory_address_confined        avgt   30  13.094 ? 0.220  ns/op
> CallOverheadConstant.panama_identity_memory_address_confined_3      avgt   30  16.051 ? 0.230  ns/op
> CallOverheadConstant.panama_identity_memory_address_shared          avgt   30  13.287 ? 0.172  ns/op
> CallOverheadConstant.panama_identity_memory_address_shared_3        avgt   30  15.812 ? 0.365  ns/op
> 
> Benchmark
> CallOverheadConstant.panama_identity_struct_ref_confined    avgt   30  11.544 ? 0.170  ns/op
> CallOverheadConstant.panama_identity_struct_ref_confined_3  avgt   30  12.213 ? 0.228  ns/op
> CallOverheadConstant.panama_identity_struct_ref_shared      avgt   30  17.214 ? 0.560  ns/op
> CallOverheadConstant.panama_identity_struct_ref_shared_3    avgt   30  33.132 ? 0.934  ns/op
> 
> 
> As it can be seen, in the confined case the numbers are good. In the shared case there's more overhead - that's the price for safety. We will be able to make this faster (by only acquiring scopes which have not already been acquired) - but note that users can always opt out of safety by projecting the `Addressable` down to a raw memory address, which would remove all the costs.

This pull request has now been integrated.

Changeset: 208d6b7b
Author:    Maurizio Cimadamore <mcimadamore at openjdk.org>
URL:       https://git.openjdk.java.net/panama-foreign/commit/208d6b7b959e222b2141ff192a442bde86a08292
Stats:     298 lines in 13 files changed: 267 ins; 18 del; 13 mod

8274285: By reference parameters should be kept alive during downcalls

Reviewed-by: jvernee

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

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