Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline native call to java/lang/Class.getStackClass when possible #19365

Conversation

r30shah
Copy link
Contributor

@r30shah r30shah commented Apr 23, 2024

Native call to java/lang/Class.getStackClass from the methods that are
being inlined in another method can be folded to load of JavaLangClass
from J9Class pointer when the inlined index of the caller of the
getStackClass is higher than the known constant argument to
getStackClass.

Signed-off-by: Rahil Shah rahil@ca.ibm.com

@r30shah r30shah changed the title Improve java lang class get stack class Inline native call to java/lang/Class.getStackClass when possible Apr 23, 2024
@r30shah r30shah force-pushed the improveJavaLangClassGetStackClass branch 5 times, most recently from c296f4a to 7b47c09 Compare April 26, 2024 20:15
@r30shah r30shah marked this pull request as ready for review April 26, 2024 20:15
@r30shah r30shah requested a review from dsouzai as a code owner April 26, 2024 20:15
@r30shah r30shah force-pushed the improveJavaLangClassGetStackClass branch 4 times, most recently from 7352ac1 to a15ccbf Compare April 26, 2024 20:25
@r30shah
Copy link
Contributor Author

r30shah commented Apr 26, 2024

@joransiu / @vijaysun-omr / @hzongaro Can I get your review on this change. I have tested this change internally through OpenJ9 - Personal/job/Pipeline-Build-Test-Personal/21826/. I see some criu related failures on x86 (Not related to these changes).

For the background, on JDK18 and up, we have introduced call to getStackClass [1][2][3] at various critical path which triggered a stack class iteration and stack walk. If we are inlining to the point that we can know the caller of getStackClass at index at compile time, we can inline those calls. On ILOG-ODM, these extra calls to getStackClass resulted in 5% performance penalty in throughput. With this change, we recovered that 5% back.

[1].

return forName(className, getStackClass(1));

[2].
Class<?> caller = getStackClass(1);

[3].
Class<?> caller = getStackClass(1);

@vijaysun-omr
Copy link
Contributor

The change itself looks good to me. There appears to be some sort of trailing H char in the commit message, that it would be good to rectify. The change also includes a change for Class.forName as a recognized method, but this isn't mentioned in the commit message or title. After these questions are answered, we could run more testing.

Copy link
Member

@joransiu joransiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic looks fine to me, besides some wording nits of the performTransformation message.

runtime/compiler/env/VMJ9.cpp Outdated Show resolved Hide resolved
@r30shah r30shah force-pushed the improveJavaLangClassGetStackClass branch from a15ccbf to 591fd57 Compare April 29, 2024 23:25
@r30shah
Copy link
Contributor Author

r30shah commented Apr 29, 2024

The change itself looks good to me. There appears to be some sort of trailing H char in the commit message, that it would be good to rectify. The change also includes a change for Class.forName as a recognized method, but this isn't mentioned in the commit message or title. After these questions are answered, we could run more testing.

I have removed the trailing H character from commit message.
I have also cleaned up the code for Class.forName(). I originally implemented this change to improve calls to getStackClass from Class.forName(), but while working on it, I realized it doesn't need to be only from Class.forName. So in further refinement of the changes, I covered other cases as well. So now we do not need Class.forName to be recognized anymore.

@vijaysun-omr
Copy link
Contributor

jenkins test sanity all jdk21

@r30shah
Copy link
Contributor Author

r30shah commented May 1, 2024

Checking out the failures on some of the builds.

  1. JDK21 x86-64 : Build passes but job failed with following error when trying to upload the files
22:06:44  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:44  Attempting retry #1
22:06:47  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:47  Attempting retry #2
22:06:50  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:50  Attempting retry #3
22:06:52  [consumer_1] Deploying artifact: https://ub18p8art.casa.cs.unb.ca/artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/test-images.tar.gz
22:06:53  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:53  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:53  Attempting retry #1
22:06:56  Error occurred for request PUT /artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/OpenJ9-JDK21-x86-64_linux-20240429-204010.tar.gz;build.parentNumber=483;build.parentName=Pipeline_Build_Test_JDK21_x86-64_linux;build.buildIdentifier=eclipse-openj9%2Fopenj9%2319365;build.timestamp=1714437508323;build.name=Build_JDK21_x86-64_linux_Personal;build.number=128 HTTP/1.1: No route to host (Host unreachable).
22:06:56  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:56  Attempting retry #1
22:06:56  Attempting retry #2
22:06:59  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:06:59  Error occurred for request PUT /artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/OpenJ9-JDK21-x86-64_linux-20240429-204010.tar.gz;build.parentNumber=483;build.parentName=Pipeline_Build_Test_JDK21_x86-64_linux;build.buildIdentifier=eclipse-openj9%2Fopenj9%2319365;build.timestamp=1714437508323;build.name=Build_JDK21_x86-64_linux_Personal;build.number=128 HTTP/1.1: No route to host (Host unreachable).
22:06:59  Attempting retry #3
22:06:59  Attempting retry #2
22:07:02  Error occurred for request GET /artifactory/api/system/version HTTP/1.1: No route to host (Host unreachable).
22:07:02  Error occurred for request PUT /artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/OpenJ9-JDK21-x86-64_linux-20240429-204010.tar.gz;build.parentNumber=483;build.parentName=Pipeline_Build_Test_JDK21_x86-64_linux;build.buildIdentifier=eclipse-openj9%2Fopenj9%2319365;build.timestamp=1714437508323;build.name=Build_JDK21_x86-64_linux_Personal;build.number=128 HTTP/1.1: No route to host (Host unreachable).
22:07:02  Attempting retry #3
22:07:05  Error occurred for request PUT /artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/OpenJ9-JDK21-x86-64_linux-20240429-204010.tar.gz;build.parentNumber=483;build.parentName=Pipeline_Build_Test_JDK21_x86-64_linux;build.buildIdentifier=eclipse-openj9%2Fopenj9%2319365;build.timestamp=1714437508323;build.name=Build_JDK21_x86-64_linux_Personal;build.number=128 HTTP/1.1: No route to host (Host unreachable).
22:07:05  Error occurred for request PUT /artifactory/ci-openj9/Build_JDK21_x86-64_linux_Personal/128/test-images.tar.gz;build.parentNumber=483;build.parentName=Pipeline_Build_Test_JDK21_x86-64_linux;build.buildIdentifier=eclipse-openj9%2Fopenj9%2319365;build.timestamp=1714437508323;build.name=Build_JDK21_x86-64_linux_Personal;build.number=128 HTTP/1.1: No route to host (Host unreachable).
22:07:05  Attempting retry #1
22:07:05  [consumer_0] An exception occurred during execution:
22:07:05  java.lang.RuntimeException: java.net.NoRouteToHostException: No route to host (Host unreachable)
22:07:05  	at org.jfrog.build.extractor.clientConfiguration.util.spec.SpecDeploymentConsumer.consumerRun(SpecDeploymentConsumer.java:44)
22:07:05  	at org.jfrog.build.extractor.producerConsumer.ConsumerRunnableBase.run(ConsumerRunnableBase.java:11)
22:07:05  	at java.base/java.lang.Thread.run(Thread.java:840)
  1. JDK21 PPC64LE : Build aborted with following error,
All nodes of label ‘[ci.role.build&&hw.arch.ppc64le&&sw.os.cent.7](https://openj9-jenkins.osuosl.org/label/ci.role.build&&hw.arch.ppc64le&&sw.os.cent.7/)’ are offline

Also checking out OpenJ9 slack : Seems like bunch of UNB machine went down on Friday, (https://openj9.slack.com/archives/CDS7QE9HB/p1714157270356889 ](https://openj9.slack.com/archives/C8PQL5N65/p1714397285116169) which was not fixed yesterday. Looking at https://openj9-jenkins.osuosl.org/label/ci.geo.unb/, seems like bunch of there are still down

@r30shah
Copy link
Contributor Author

r30shah commented May 2, 2024

Jenkins test sanity xlinux jdk21

@r30shah
Copy link
Contributor Author

r30shah commented May 2, 2024

Failure seen in JDK21 x86 build was due tio #19431, Now as it is merged, relaunching the JDK21 x86 build

@r30shah
Copy link
Contributor Author

r30shah commented May 2, 2024

Jenkins test sanity xlinux jdk21

1 similar comment
@r30shah
Copy link
Contributor Author

r30shah commented May 8, 2024

Jenkins test sanity xlinux jdk21

@r30shah
Copy link
Contributor Author

r30shah commented May 8, 2024

Seems like last launched build finished successfully but test job could not download the build to start the tests.

I am going to rebase my changes and run the tests internally again to cover some of the missing targets.

Native call to java/lang/Class.getStackClass from the methods that are
being inlined in another method can be folded to load of JavaLangClass
from J9Class pointer when the inlined index of the caller of the
getStackClass is higher than the known constant argument to
getStackClass.

Signed-off-by: Rahil Shah <rahil@ca.ibm.com>
@r30shah r30shah force-pushed the improveJavaLangClassGetStackClass branch from 591fd57 to 8202d53 Compare May 8, 2024 21:13
@r30shah
Copy link
Contributor Author

r30shah commented May 8, 2024

jenkins test sanity all jdk21

@r30shah
Copy link
Contributor Author

r30shah commented May 9, 2024

Looking into the build results (Both from internal and open)

  1. JDK21 AARCH64 Linux : Failed in test testJitserverArguments_0 with following error, issue is already seen : testJitserverArguments_0_FAILED FAILURE: JITServer::StreamFailure: Connect failed: Connection refused #19151
19:06:04   [ERR] JITServer is ready to accept incoming requests
19:06:04   [ERR] can't bind server address: Address already in use
19:06:04   [ERR] Failed to open server socket on port 43896

I do have this target passing on hyc jenkins : build

  1. PPC64LE linux : Build aborted due to machine unavailability, corresponding build from internal jenkins, both sanity.functional and sanity.openjdk passes.

  2. x86-64 Linux : Builds can not be downloaded to test, corresponding build from internal hyc jenkins: build

  3. x86-64 Mac : Same issue where test job can not reach to artifactory to download builds, I also ran this target internally in build where sanity.functional passes (Sanity.openjdk was aborted due to infrastructure issue).

@vijaysun-omr I am not sure when the external infrastructure issues will be resolved, but I did try to cover most of the platforms combining internally and externally. Here is the JDK11 and JDK21 internal build. One failure seen internally on s390 is same as #19151 which is passing externally.

This one can be merged if you are OK with the testing.

@vijaysun-omr
Copy link
Contributor

Thanks @r30shah

I am going to merge this on the basis of the justification that you provided.

@vijaysun-omr vijaysun-omr merged commit 0e3af5d into eclipse-openj9:master May 9, 2024
12 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants