Make AOT Compilation generate the same code as JIT Compilation for enter/exit tracing

[dsouzai]

There are two different types of method enter/exit tracing available:

1. -Xtrace:print=mt,methods=...
2. JVMTI method enter/exit hooks

JIT Implementation

If either of these types of tracing are possible, then the ILGenerator generates IL containing TR::MethodEnterHook and/or TR::MethodExitHook op codes. Later on in the compilation, TR_J9VMBase::lowerMethodHook is invoked on these op codes.

If -Xtrace is specified on a particular method, then the compiler generates a call to the appropriate helper [1]. Otherwise, if the JVM may have to report method enter/exit hooks to a JVMTI agent, then a runtime test is generated [2].

AOT Implementation

Like the JIT case, if either of these types of tracing are possible, then the ILGenerator generates IL containing TR::MethodEnterHook and/or TR::MethodExitHook op codes, on which TR_J9VMBase::lowerMethodHook is invoked. The difference though is that regardless of the type of tracing, the compiler generates a TR_MethodEnterExitGuard [3]. At AOT load time, if either type of tracing is possible, the guard is activated; otherwise, it is left as a NOP.

[1]

openj9/runtime/compiler/env/VMJ9.cpp

Line 2980 in 45ed10a

bool isTrace = isMethodTracingEnabled(j9method);

[2]

openj9/runtime/compiler/env/VMJ9.cpp

Line 3076 in 45ed10a

else if (!isTrace && comp->cg()->getSupportsPartialInlineOfMethodHooks())

[3]

openj9/runtime/compiler/env/VMJ9.cpp

Line 3000 in 45ed10a

TR::Node *cmp = comp->createAOTGuard(comp, methodCall->getInlinedSiteIndex(), root, 0, TR_MethodEnterExitGuard);

---

This PR makes the code generated during an AOT compilation the same as what is generated during a JIT compilation with respect to enter/exit tracing. The main functional difference is that in the existing implementation, if tracing was not enabled on a Load run, if the code had the guard, it would be left as a NOP to not report to the VM whereas in the proposed implementation, it would continue to report to the VM (in the case of -Xtrace) or continue to have the runtime test (in the case of the JVMTI callback) thus increasing the path length. However, I don't the added complexity of having a completely different implementation for AOT is justified to prevent this extra path length considering that it will only happen if one generates AOT with the tracing and then removes the option in a subsequent run.

This PR also includes some refactoring to facilitate this change. Specifically:

• Removes the _extra field from the binary template
• Moves cross platform relocation flags to the lower nibble
• Upgrades the relocation flags to be 16 bits

The reason for this is to have additional flags in the method header as well as the inlined method relocation records. The method header flags indicate if method tracing is enabled (-Xtrace) as well as if method enter/exit events can be hooked (JVMTI). The inlined method relocation records only need a flag to validate if method tracing is enabled; this is because the JVMTI hooks are not method specific and so it suffices to only check the AOT method header; in fact a further refactoring could move this flag to the AOT Header (which covers all methods in the SCC) - this is likely going to be needed for CRIU to allow methods that both have and don't have tracing support.

Depends on eclipse-omr/omr#7039

[dsouzai]

fyi @vijaysun-omr @mstoodle tagging you guys in case you have some concerns about the slight change in functionality.

[jdmpapin]

Oh, and I don't think I caught why the flags needed to be expanded to 16-bit. What's causing that?

[dsouzai]

Oh, and I don't think I caught why the flags needed to be expanded to 16-bit. What's causing that?

It's because with the addition of TR_RelocationFlags, I needed 8 bits worth of relocation flags, along with another few bits for the TR_RelocationFlagUtilities over in OMR. That's why I had first swapped the latter flags to the lower nibble before increasing the size of the former; essentially these two types of flags can't overlap (because one is used as additional information specific to a relocation record, while the other is used to indicate wide offsets and/or PC relative addressing).

[jdmpapin]

OK, I see. It's because there are 5 flags there in addition to the 4 bits reserved for TR_RelocationFlagUtilities

It seems to me that only one flag value is really necessary in TR_RelocationFlags at the moment. inlinedMethodIsStatic and inlinedMethodIsSpecial are redundant because they're only used like !inlinedMethodIsStatic && !inlinedMethodIsSpecial, which is equivalent to inlinedMethodIsVirtual. And then inlinedMethodIsVirtual, staticSpecialMethodFromCpIsSplit, and needsFullSizeRuntimeAssumption all apply to disjoint sets of relocation kinds

But there is some maintainability benefit to keeping them separate, and the larger flags field shouldn't affect the size of most relocation records anyway (at least on 64-bit), so I think it's fine the way it is 👍 🙂

[dsouzai]

@jdmpapin addressed review comments; good for review again. I ran a bunch of internal tests, and it looks like there's no issues on all platforms (including 32-bit); the only exception is aarch64 which I wasn't able to run any tests on.

[dsouzai]

@jdmpapin review reminder.

[jdmpapin]

Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk11,jdk17,jdk21 depends eclipse-omr/omr#7039

[dsouzai]

jdk11 aarch64 test failure

[2023-07-24T18:32:44.722Z] STDERR:
[2023-07-24T18:32:44.722Z]  stdout: [OK
[2023-07-24T18:32:44.722Z] ];
[2023-07-24T18:32:44.722Z]  stderr: [JVMDUMP034I User requested System dump using '/home/jenkins/workspace/Test_openjdk11_j9_sanity.openjdk_aarch64_linux_Personal_testList_1/aqa-tests/TKG/output_1690223011864/jdk_lang_1/work/scratch/0/core.20230724.183231.82035.0001.dmp' through com.ibm.jvm.Dump.SystemDump
[2023-07-24T18:32:44.722Z] JVMDUMP010I System dump written to /home/jenkins/workspace/Test_openjdk11_j9_sanity.openjdk_aarch64_linux_Personal_testList_1/aqa-tests/TKG/output_1690223011864/jdk_lang_1/work/scratch/0/core.20230724.183231.82035.0001.dmp
[2023-07-24T18:32:44.722Z] Exception in thread "Thread-1" java.lang.RuntimeException: simulateUncaughtExitEvent
[2023-07-24T18:32:44.722Z] 	at UncaughtExitSimulator.throwRuntimeException(UncaughtExceptionsTest.java:94)
[2023-07-24T18:32:44.722Z] 	at UncaughtExitSimulator.run(UncaughtExceptionsTest.java:100)
[2023-07-24T18:32:44.722Z] ]
[2023-07-24T18:32:44.722Z]  exitValue = 0
[2023-07-24T18:32:44.722Z] 
[2023-07-24T18:32:44.722Z] java.lang.Exception: failures: 1
[2023-07-24T18:32:44.722Z] 	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:96)
[2023-07-24T18:32:44.723Z] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[2023-07-24T18:32:44.723Z] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[2023-07-24T18:32:44.723Z] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[2023-07-24T18:32:44.723Z] 	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
[2023-07-24T18:32:44.723Z] 	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312)
[2023-07-24T18:32:44.723Z] 	at java.base/java.lang.Thread.run(Thread.java:839)

looks to be the same as #11930

[dsouzai]

jdk 21 win64 failure seems to be infra related; no tests seem to have been launched.

[jdmpapin]

Jenkins test sanity.functional,sanity.openjdk win jdk21 depends eclipse-omr/omr#7039

[dsouzai]

jdk17_j9_sanity.functional_x86-64_windows failed because of #16544 :

[2023-07-25T01:30:31.509Z] 01:30:28.901 0x1aa00 omrport.359    *   ** ASSERTION FAILED ** at f:\users\jenkins\workspace\build_jdk17_x86-64_windows_personal\omr\port\common\omrmemtag.c:145: ((memoryCorruptionDetected))
[2023-07-25T01:30:31.509Z] JVMDUMP039I Processing dump event "traceassert", detail "" at 2023/07/24 21:30:28 - please wait.

jdk17_j9_sanity.openjdk_x86-64_windows is failing because of #10975 (except this is 64bit and jdk17) :

[2023-07-24T22:22:07.123Z] command: build Basic
[2023-07-24T22:22:07.123Z] reason: Named class compiled on demand
[2023-07-24T22:22:07.123Z] elapsed time (seconds): 0.0
[2023-07-24T22:22:07.123Z] 
[2023-07-24T22:22:07.123Z] ACTION: main -- Error. Program `C:\Users\jenkins\workspace\Test_openjdk17_j9_sanity.openjdk_x86-64_windows_Personal_testList_0\openjdkbinary\j2sdk-image\bin\java' timed out (timeout set to 2400000ms, elapsed time including timeout handling was 2415109ms).
[2023-07-24T22:22:07.123Z] REASON: User specified action: run main/othervm/native/timeout=300 -Djava.security.manager=allow -Djdk.lang.Process.launchMechanism=fork Basic 
[2023-07-24T22:22:07.123Z] TIME:   2415.109 seconds

I'm fairly certain jdk21_j9_sanity.openjdk_x86-64_windows is also failing for the same reason; the timeout occurs because javac seems to be taking too long to complete.

[jdmpapin]

Normally I'd just ignore known failures, but I'm restarting the builds to see them pass because this is for a coordinated merge

[jdmpapin]

Jenkins test sanity.openjdk alinux jdk11 depends eclipse-omr/omr#7039

[jdmpapin]

Jenkins test sanity.openjdk alinux64 jdk11 depends eclipse-omr/omr#7039

[jdmpapin]

Jenkins test sanity.openjdk win jdk17,jdk21 depends eclipse-omr/omr#7039

[jdmpapin]

Jenkins test sanity.functional,sanity.openjdk win jdk17 depends eclipse-omr/omr#7039

[dsouzai]

Similar error on jdk17_j9_sanity.openjdk_x86-64_windows:

[2023-07-26T00:13:30.178Z] ACTION: main -- Error. Program `F:\Users\jenkins\workspace\Test_openjdk17_j9_sanity.openjdk_x86-64_windows_Personal_testList_1\openjdkbinary\j2sdk-image\bin\java' timed out (timeout set to 960000ms, elapsed time including timeout handling was 1075618ms).

[jdmpapin]

Yesterday OMR was promoted manually despite some known issues appearing in the acceptance build, including exactly this pair of timeouts (EnoughSeedTest+ApiTest in sanity.openjdk/jdk_security1_1 on Windows x86-64, JDK17), so I won't bother restarting it again