[jdk16] RFR: 8258242: Type profile pollution occurs when memory segments of different kinds are used
Claes Redestad
redestad at openjdk.java.net
Mon Dec 14 15:51:00 UTC 2020
On Mon, 14 Dec 2020 14:46:41 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!
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
More information about the security-dev
mailing list