On Mon, 14 Dec 2020 18:07:14 GMT, Maurizio Cimadamore <mcimadam...@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! > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix javadoc Marked as reviewed by redestad (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk16/pull/19
