Apache CXF test case fails on OpenJ9, may be due to Object.hashCode() not being native

[pshipton]

From OpenJ9 Slack https://openj9.slack.com/archives/C8312LCV9/p1712509405994699

Benjamin Marwell Overlooked in #cxf on ASF Slack: If you check out Apache CXF, then goto cxf/systests/cdi/cdi-owb/cdi-no-apps-owb and run mvn clean install, it will fail due to differences in handling Unsafe.

Benjamin Marwell Original post: So deep diving into the OWB test suites which fail on IBM Java -- the exception happens in org.eclipse.jetty.util.component.ContainerLifeCycle.

 jakarta.enterprise.context.ContextNotActiveException: WebBeans context with scope type annotation @RequestScoped does not exist within current thread
 Error while sending SystemEvent to a CDI Extension! org.apache.webbeans.portable.events.discovery.AfterDeploymentValidationImpl

Jamie Goodyear I have been working on Apache CXF main branch https://github.com/apache/cxf Attempting to make it build cleanly on IBM Semeru Open edition Java 17. When building the OpenWebBeans test suites with other JVMs the test cases all pass, on IBM Java it fails.

[INFO] Apache CXF CDI Integration System Tests - OWB with multiple apps FAILURE [ 12.851 s]
[INFO] Apache CXF CDI Integration System Tests - OWB with no apps SKIPPED
[INFO] Apache CXF CDI Integration System Tests - OWB with producers SKIPPED

To reproduce this all one needs to do is:

git clone https://github.com/apache/cxf.git
cd cxf
git checkout main (this is default, just want to ensure you're on main)
mvn clean install

The failed tests generally will have a failure stack trace similar to below:

at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:574)
or
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)

This reproduces weather on MaxOS on AArch64, CentOS Linux on PPC64LE, or Windows on x64. Using HotSpot based JVMs will allow the tests to pass.

Jamie Goodyear We may have a solution:

public native int java.lang.Object.hashCode()
vs
public int java.lang.Object.hashCode()

[pshipton]

@TobiAjila fyi

[tajila]

@theresa-m Please take a look at this

[mstoodle]

@theresa-m @tajila I know it's a busy time, but any update here? Just curious :)

[theresa-m]

I don't have an update yet. I was looking at a different user issue since the failure was resolved on the Apache CXF side but will be looking into it more next week.

[theresa-m]

I am working on a change to make OpenJ9's Object.hashCode a native method. To prevent many calls back to Java J9VMInternals.fastIdentityHashCode and possibly J9VMInternals.isArray can also be moved to native. The value types case will need to be moved to, I will focus on that second. Opinions on how far it makes sense to take this @TobiAjila ?

[tajila]

I think there is a going to be a performance penalty as we have a Java fast path for the JIT.

[tajila]

@gacholio What are your thoughts on this?

[gacholio]

Relying on a method being native is foolish - Oracle will certainly have no qualms about changing this (or any other method) as new features are implemented. This would also fail if the method were instrumented by native method prefixing.

If, for some reason, we decide to fix this, the performance penalty could be mostly avoided by the JIT doing custom code generation when it detects one of the two identity hashCode paths (System or Object), which might mean changes to every platform codegen (I'm not sure if a common IL would suffice).

In the Object case, the JIT would need to be able to know (via devirtualization?) that the basic hashCode is being requested, rather than compiling the optimized version of Object.hashCode which uses the JITHelpers methods, so a purely virtual call to hashCode would still incur the penalty.

[rmannibucau]

Guess it should somehow converge to the reference jvm method signature - including all flags/modifiers - for compatibility reason. Technically there is no real point since native or not can converge in terms of perf when you own the jvm - even if not trivial. So +1 to converge whatever the outcome at jdk discussion level is.

[tajila]

@hzongaro Thoughts on GACs comments?

[hzongaro]

If, for some reason, we decide to fix this, the performance penalty could be mostly avoided by the JIT doing custom code generation when it detects one of the two identity hashCode paths (System or Object), which might mean changes to every platform codegen (I'm not sure if a common IL would suffice).

Several developers have begun looking at ensuring the JIT compiler yields good performance for various intrinsic methods across all supported platforms. We'll look at Object.hashCode() within that context and report back on any progress over the next few months.

[tajila]

Okay, I think its best for the time being to wait for the JIT team to complete their investigation. In the near future we may be able to switch this to a native at no cost.

[rmannibucau]

side question: can a review of the signatures (including this kind of flag) be done or at least a diff with openjdk be published :pray: ?

[tajila]

side question: can a review of the signatures (including this kind of flag) be done or at least a diff with openjdk be published 🙏 ?

@pshipton do we publish docs for the non-J9 specific packages and modules?

[pshipton]

do we publish docs for the non-J9 specific packages and modules?

No. Plus flags such as "native" aren't part of the javadoc.