[foreign-memaccess+abi] RFR: 8257692: Using both heap and native segments can degrade performance

[Jorn Vernee]

On Fri, 11 Dec 2020 15:23:24 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch fixes a problem with type profile pollution when segments of different kinds are used on the same memory access var handle, or on the same `MemoryAccess` static method.
> 
> In principle, argument profiling should kick in for VarHandles and MethodHandles, and that should be enough at least to avoid pollution when using var handles directly. In reality, this is not the case; as Vlad discovered after relatively intense debugging session (thanks!), the VarHandle implementation code has to cast the incoming segment to the `MemorySegmentProxy` internal interface. This creates problems for C2, as concrete segment implementations have _two_ interface types: `MemorySegment` and the internal `MemorySegmentProxy` class. Side casts from one to the other are not supported well, and can cause loss of type profiling infomation.
> 
> To solve this problem we realized, in hindisght, that `MemorySegmentProxy` didn't really needed to be an interface and that it could easily be converted to an abstract class. Alone this solves 50% of the issues, since that makes direct var handle access robust to pollution issues. The remaining problems (using accessors in `MemoryAccess` class) can be addressed the usual way, by adding argument type profiling for the methods in that class (similarly to what we've done for `ScopedMemoryAccess`).
> 
> Here are some numbers before the patch:
> 
> Benchmark                                            Mode  Cnt   Score   Error  Units
> LoopOverPollutedSegments.heap_segment_floats_VH      avgt   30  11.535 ? 0.039  ms/op
> LoopOverPollutedSegments.heap_segment_floats_static  avgt   30  10.860 ? 0.162  ms/op
> LoopOverPollutedSegments.heap_segment_ints_VH        avgt   30  11.479 ? 0.202  ms/op
> LoopOverPollutedSegments.heap_segment_ints_static    avgt   30  10.562 ? 0.027  ms/op
> LoopOverPollutedSegments.heap_unsafe                 avgt   30   0.240 ? 0.003  ms/op
> LoopOverPollutedSegments.native_segment_VH           avgt   30  11.603 ? 0.154  ms/op
> LoopOverPollutedSegments.native_segment_static       avgt   30  10.613 ? 0.128  ms/op
> LoopOverPollutedSegments.native_unsafe               avgt   30   0.243 ? 0.003  ms/op
> 
> As you can see there is quite a big difference between unsafe access and all the other modes. Here are the results after the patch:
> 
> Benchmark                                            Mode  Cnt  Score   Error  Units
> LoopOverPollutedSegments.heap_segment_floats_VH      avgt   30  0.244 ? 0.002  ms/op
> LoopOverPollutedSegments.heap_segment_floats_static  avgt   30  0.301 ? 0.001  ms/op
> LoopOverPollutedSegments.heap_segment_ints_VH        avgt   30  0.245 ? 0.003  ms/op
> LoopOverPollutedSegments.heap_segment_ints_static    avgt   30  0.302 ? 0.004  ms/op
> LoopOverPollutedSegments.heap_unsafe                 avgt   30  0.242 ? 0.003  ms/op
> LoopOverPollutedSegments.native_segment_VH           avgt   30  0.246 ? 0.004  ms/op
> LoopOverPollutedSegments.native_segment_static       avgt   30  0.295 ? 0.006  ms/op
> LoopOverPollutedSegments.native_unsafe               avgt   30  0.245 ? 0.003  ms/op
> 
> That is, the situation is back to normal. Thanks to @JornVernee and @iwanowww for the help!

Marked as reviewed by jvernee (Committer).

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/Utils.java line 55:

> 53:             .orElse("deny");
> 54: 
> 55:     public static final MethodHandle SEGMENT_FILTER;

Why this change?

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

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