eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 721 forks source link

Native method prefixing undone by JIT for library methods #6450

Open urisimchoni opened 5 years ago

urisimchoni commented 5 years ago

This has been observed for at least sun.misc.unsafe.compareAndSwapInt, as used by AtomicInteger, in OpenJ9 .

We've implemented wrapping of sun.misc.unsafe.compareAndSwapInt() using native method prefixing technique (cf. https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#SetNativeMethodPrefix). The purpose is to instrument this function, so the result is to call an agent whenever compareAndSwapInt() gets called.

In a nutshell, we modify sun.misc.Unsafe as follows:

  1. rename native method compareAndSwapInt() to wrapped_compareAndSwapInt()
  2. add a non-native compareAndSwapInt() that would call the agent and then call wrapped_compareAndSwapInt().

We then use JVMTI's SetNativeMethodPrefix() to have the wrapped_compareAndSwapInt() call reach the original native compareAndSwapInt().

Things seem to work well at first, but when running a performance test with AtomicInteger, the events stop arriving after several thousands (supposedly, when the AtomicInteger is JIT-compiled).

When running with -Xint (no JIT), things work correctly.

Initial analysis by the OpenJ9 team suggests that as AtomicInteger gets compiled, a heuristic is applied to inline calls to some known native methods. So if the compiled method has a call to Unsafe.compareAndSwapInt(), the compiler emits code that does the actual CAS instead of calling the function, but in the process the wrapper is bypassed.

Version information: openjdk version "1.8.0_212" OpenJDK Runtime Environment (build 1.8.0_212-b04) Eclipse OpenJ9 VM (build openj9-0.14.2, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20190521_315 (JIT enabled, AOT enabled) OpenJ9 - 4b1df46fe OMR - b56045d2
JCL - a8c217d402 based on jdk8u212-b04

fjeremic commented 5 years ago

Are you able to provide us with a minimal reproduction of the issue? Judging from the description we should be reporting the call to the wrapper, even if we inline the CAS intrinsic (which will always be done if we see such a call during compilation).

urisimchoni commented 5 years ago

wrapdemo.tar.gz

To reproduce (native agent is x86-64 Linux): Download and untar wrapdemo.tar.gz - it contains two .java files and a native agent binary. From within the directory containing the files: javac Hello.java Callbacks.java java -Xbootclasspath/p:./ -agentpath:./libNativeAgent.so Hello

The expected result is for two numbers to be printed - 1000000, followed by a number slightly more than 1000000 (the first is the value of an AtomicInteger after 1000000 increments, the second is the number of time the agent has been called). The actual result is that the second number is a few thousands.

The agent wraps sun.misc.Unsafe.compareAndSwapInt() by means of native method prefixing.

Incidentally, the same issue exists for HotSpot. Howerver, with HotSpot it's possible to wrap those methods via native method binding.

hzongaro commented 5 years ago

There is a work-in-progress pull request #6598 submitted that addresses part of this issue. In particular, that pull request identifies methods in the Java class library that the JIT compiler ordinarily would recognize that are expected to be native. If such a method is not actually native, the JIT compiler will not recognize the method.

However, there are also methods that are not native that the JIT compiler recognizes. In the case of the example wrapdemo.tar.gz attached above, the implementation of AtomicInteger.incrementAndGet() calls sun.misc.Unsafe.getAndAddInt(). The method Unsafe.getAndAddInt() is implemented in Java (it is not native) and, in turn, calls the native method Unsafe.compareAndSwapInt. However, Unsafe.getAndAddInt is recognized by the JIT compiler, and the JIT compiler generates code that doesn't attempt to call Unsafe.compareAndSwapInt, so the JIT compiler doesn't get a chance to detect that Unsafe.compareAndSwapInt was replaced.

@andrewcraik has suggested a follow-up change so that a recognized method, like Unsafe.getAndAddInt, that is not native, but that is dependent on a native method, like Unsafe.compareAndSwapInt is only recognized by the JIT compiler if the native methods on which it depends actually are native. That change will be needed to fully resolve the issue that was reported. I will work on that next.