[jdk16] RFR: 8258242: Type profile pollution occurs when memory segments of different kinds are used [v2]
Maurizio Cimadamore
mcimadamore at openjdk.java.net
Mon Dec 14 18:07:14 UTC 2020
> 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
-------------
Changes:
- all: https://git.openjdk.java.net/jdk16/pull/19/files
- new: https://git.openjdk.java.net/jdk16/pull/19/files/324d5b92..81472e6d
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk16&pr=19&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk16&pr=19&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk16/pull/19.diff
Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/19/head:pull/19
PR: https://git.openjdk.java.net/jdk16/pull/19
More information about the security-dev
mailing list