On Mon, 14 Dec 2020 14:46:41 GMT, Maurizio Cimadamore <[email protected]>
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!
src/java.base/share/classes/jdk/internal/access/foreign/MemorySegmentProxy.java
line 32:
> 30:
> 31: /**
> 32: * This proxy interface is required to allow instances of the {@code
> MemorySegment} interface (which is defined inside
"This abstract base class.."?
I don't mind the current name, but should the class be renamed
`AbstractMemorySegmentProxy`?
-------------
PR: https://git.openjdk.java.net/jdk16/pull/19
