[foreign-preview] RFR: 8281595: ASM-ify scope acquire/release for down call parameters

[Jorn Vernee]

On Tue, 15 Feb 2022 14:04:30 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Hi,
>> 
>> This patch rewrites the scope acquire/release logic that we do for downcalls inside ASM, in the generated specialized binding class. This also drops the fallback path we had for more than 5 scopes. The new code now always checks to see if every scope is unique. (I think that should be okay, but please let me know if otherwise).
>> 
>> I've also added a benchmark by Maurizio to this patch, which was showing a failure to scalar replace some scoped arguments with the old MH logic (reason discussed in [JDK-8281387](https://bugs.openjdk.java.net/browse/JDK-8281387)). With the new ASM-based logic, there is no sign of allocations (other than some noise):
>> 
>> 
>> Benchmark                                                     Mode  Cnt   Score    Error   Units
>> PointerInvoke.panama_call_as_address                          avgt   30  13.181 ±  0.320   ns/op
>> PointerInvoke.panama_call_as_address:·gc.alloc.rate           avgt   30   0.043 ±  0.049  MB/sec
>> PointerInvoke.panama_call_as_address:·gc.alloc.rate.norm      avgt   30   0.001 ±  0.001    B/op
>> PointerInvoke.panama_call_as_address:·gc.count                avgt   30     ≈ 0           counts
>> PointerInvoke.panama_call_as_long                             avgt   30  12.943 ±  0.287   ns/op
>> PointerInvoke.panama_call_as_long:·gc.alloc.rate              avgt   30   0.065 ±  0.054  MB/sec
>> PointerInvoke.panama_call_as_long:·gc.alloc.rate.norm         avgt   30   0.002 ±  0.001    B/op
>> PointerInvoke.panama_call_as_long:·gc.count                   avgt   30     ≈ 0           counts
>> PointerInvoke.panama_call_as_new_segment                      avgt   30  14.309 ±  0.177   ns/op
>> PointerInvoke.panama_call_as_new_segment:·gc.alloc.rate       avgt   30   0.173 ±  0.144  MB/sec
>> PointerInvoke.panama_call_as_new_segment:·gc.alloc.rate.norm  avgt   30   0.005 ±  0.004    B/op
>> PointerInvoke.panama_call_as_new_segment:·gc.count            avgt   30     ≈ 0           counts
>> PointerInvoke.panama_call_as_segment                          avgt   30  13.273 ±  0.191   ns/op
>> PointerInvoke.panama_call_as_segment:·gc.alloc.rate           avgt   30  ≈ 10??           MB/sec
>> PointerInvoke.panama_call_as_segment:·gc.alloc.rate.norm      avgt   30  ≈ 10??             B/op
>> PointerInvoke.panama_call_as_segment:·gc.count                avgt   30     ≈ 0           counts
>> 
>> 
>> Thanks,
>> Jorn
>
> src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java line 486:
> 
>> 484: 
>> 485:     private void emitReleaseScopes() {
>> 486:         for (int scopeLocal : scopeSlots) {
> 
> Isn't this emitting more code than required? E.g. if we pass 10 primitives, we will emit 10 scope checks at the end of the adapter? Granted, since all these locals contain "null", there's no correctness issue here - but it is odd to (i) have a local for a scope corresponding to a primitive type (even though we know we never need it) and have extra logic in the preamble/postamble because of it.

There is only a scope slot local for Addressable and NativeSymbol parameters. See the logic that initializes `scopeSlots`:


scopeSlots = Arrays.copyOf(scopeSlots, numScopes); // fit to size


(the array is the size of the parameter list initially, but is adjusted downwards. Maybe that should be made clearer...)

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

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