On Mon, 14 Dec 2020 14:46:41 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! This pull request has now been integrated. Changeset: 7ff9c856 Author: Maurizio Cimadamore <mcimadam...@openjdk.org> URL: https://git.openjdk.java.net/jdk16/commit/7ff9c856 Stats: 225 lines in 8 files changed: 215 ins; 0 del; 10 mod 8258242: Type profile pollution occurs when memory segments of different kinds are used Reviewed-by: vlivanov, redestad ------------- PR: https://git.openjdk.java.net/jdk16/pull/19
