8000 Move SharedClassAPI test native code into openj9 by annaibm · Pull Request #21636 · eclipse-openj9/openj9 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Move SharedClassAPI test native code into openj9 #21636

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Move SharedClassAPI test native code into openj9 #21636

wants to merge 1 commit into from

Conversation

annaibm
Copy link
Contributor
@annaibm annaibm commented Apr 14, 2025
  • Move the sharedClasses native code from openj9_systemtest into OpenJ9,
    as part of removing the native compiler dependency in system tests.

related:adoptium/aqa-tests#5965

@annaibm
Copy link
Contributor Author
annaibm commented Apr 14, 2025 •
edited
Loading

Grinder link:
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK21_x86-64_linux_Personal/1728/

14:37:35  [ 22%] Linking C shared library ../../libsharedClasses.so
14:37:35  [ 22%] Built target run_constgen
14:37:35  [ 22%] Building C object runtime/tests/shared/CMakeFiles/SharedClassesNativeAgent.dir/__/__/copyright.c.o
14:37:35  [ 22%] Building C object runtime/tests/port/CMakeFiles/pltest.dir/j9dumpTest.c.o
14:37:35  Scanning dependencies of target j9zip
14:37:35  [ 22%] Linking C shared library ../../libSharedClassesNativeAgent.so
14:37:35  [ 22%] Building C object runtime/zip/CMakeFiles/j9zip.dir/zcpool.c.o
14:37:35  [ 22%] Built target sharedClasses

Once this change is merged, will make changes in below JDK versions to include the shared classes in test-images.
https://github.com/ibmruntimes/openj9-openjdk-jdk
https://github.com/ibmruntimes/openj9-openjdk-jdk24
https://github.com/ibmruntimes/openj9-openjdk-jdk21
https://github.com/ibmruntimes/openj9-openjdk-jdk17
https://github.com/ibmruntimes/openj9-openjdk-jdk11
https://github.com/ibmruntimes/openj9-openjdk-jdk8

@annaibm annaibm marked this pull request as draft April 14, 2025 19:19
@annaibm annaibm force-pushed the addShared branch 4 times, most recently from 9efd83a to 03afebf Compare May 12, 2025 18:37
@annaibm annaibm marked this pull request as ready for review May 13, 2025 14:24
@llxia llxia changed the title Move SharedClassAPI native code into openj9 Move SharedClassAPI test native code into openj9 May 13, 2025
@llxia
Copy link
Contributor
llxia commented May 13, 2025

Just for the record, sharedClasses.c is moved from https://github.com/adoptium/openj9-systemtest/blob/master/openj9.test.sharedClasses.jvmti/src/native/sharedClasses.c

@llxia
Copy link
Contributor
llxia commented May 13, 2025

The above job link has expired. Could you run it again? We also need to test on other platforms.

@llxia
Copy link
Contributor
llxia commented May 13, 2025

@pshipton could you also review the change? Thanks

@keithc-ca keithc-ca self-requested a review May 13, 2025 14:56
@annaibm
Copy link
Contributor Author
annaibm commented May 13, 2025 •
edited
Loading

Grinder link:
JDK21
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK21_x86-64_linux_Personal/1855/

15:54:04  Scanning dependencies of target sharedClasses
15:54:04  [ 23%] Building C object runtime/tests/sharedclassapi/CMakeFiles/sharedClasses.dir/sharedClasses.c.o
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/fltdmath.c.o
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/fltmath.c.o
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/fltodd.c.o
15:54:04  [ 23%] Building C object runtime/tests/sharedclassapi/CMakeFiles/sharedClasses.dir/__/__/copyright.c.o
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/fltrem.c.o
15:54:04  [ 23%] Linking C shared library ../../libsharedClasses.so
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/genericSignalHandler.c.o
15:54:04  [ 23%] Built target sharedClasses
15:54:04  [ 23%] Building C object runtime/util/CMakeFiles/j9util.dir/hshelp.c.o

JDK11
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK11_x86-64_linux_Personal/7697/

pshipton
pshipton reviewed May 13, 2025
View reviewed changes
runtime/tests/sharedclassapi/sharedClasses.c Outdated
@@ -0,0 +1,641 @@
/*******************************************************************************
* Copyright (c) 2017, 2023 IBM Corp.
Copy link
Member

Choose a reason for hiding this comment

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

Pls fix the Copyright format to match the others.
Copyright IBM Corp. and others 2017

keithc-ca
keithc-ca requested changes May 13, 2025
View reviewed changes
Copy link
Contributor
@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Please apply the feedback throughout.

@annaibm annaibm force-pushed the addShared branch 3 times, most recently from fdffdb8 to 3b73b6e Compare May 16, 2025 17:59
keithc-ca
keithc-ca requested changes May 16, 2025
View reviewed changes
10000
Copy link
Contributor
@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Please restore some of the blank lines. A (single) blank line before each function or significant block improves clarity.

@annaibm annaibm force-pushed the addShared branch 2 times, most recently from 0c836d4 to 3b0be74 Compare May 20, 2025 15:44
@annaibm annaibm requested a review from keithc-ca May 20, 2025 20:09
@keithc-ca keithc-ca mentioned this pull request May 21, 2025
Merged
keithc-ca
keithc-ca requested changes May 21, 2025
View reviewed changes
Copy link
Contributor
@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

These comments should be applied throughout, not just to the specific examples identified.

@annaibm annaibm requested a review from keithc-ca May 27, 2025 15:56
keithc-ca
keithc-ca requested changes May 27, 2025
View reviewed changes
@annaibm annaibm force-pushed the addShared branch 4 times, most recently from a7bb420 to 7b01d9d Compare May 29, 2025 13:58
keithc-ca
keithc-ca requested changes May 29, 2025
View reviewed changes
keithc-ca
keithc-ca requested changes May 29, 2025
View reviewed changes
Copy link
Contributor
@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Please squash.

@keithc-ca
Copy link
Contributor

I don't see this test included in any playlist (in OpenJ9). Shouldn't this change add that?

@annaibm
Copy link
Contributor Author
annaibm commented May 29, 2025

I don't see this test included in any playlist (in OpenJ9). Shouldn't this change add that?

yes, you are right that this test isn't added to an OpenJ9 playlist because it's already included in the existing aqa-tests playlist under:
🔗 sharedClasses/playlist.xml#L26

As part of the task: "Move SharedClassAPI test native code into OpenJ9", we migrated the native code (sharedClasses.c) from the original location in openj9-systemtest into OpenJ9 to eliminate native compilation within the test and stop building it in the tests.

The test itself is already active and run via that playlist, so no additional playlist changes in OpenJ9 are needed.
sharedClasses/playlist.xml#L26

@keithc-ca
Copy link
Contributor

Jenkins test system.extended zlinux jdk21

@keithc-ca
Copy link
Contributor

I botched the test command. Rather than start over and run more testing than necessary, please run (or explain how I could run) a grinder to test this. I couldn't find a playlist.xml that refers to the new library.

@annaibm
Copy link
Contributor Author
annaibm commented Jun 3, 2025

Right now, we are only moving the native code (sharedClasses.c) into OpenJ9 and building the libsharedClasses.so library as part of the OpenJ9 build process. We haven’t yet updated the SharedClassesAPI test to point to or load this new library path. (test: sharedClasses/playlist.xml#L26)

The Grinder run is still using the old path from openj9-systemtest for libsharedClasses.so.
Original run: https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/51106/

  • The next step will be to change the test paths within SharedClassesAPI to use this newly built library, but that hasn’t been done yet.

So at this stage, you won’t find any references to the new library in the existing playlist.xml or test lists, since the test itself still uses the old library path.

Currently, the following personal build test images contain the libsharedClasses.so library, as demonstrated by the Grinder builds below:

linux:
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK8_x86-64_linux_Personal/3592/
Screenshot 2025-06-03 at 2 27 41 PM

win:
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK21_x86-64_windows_Personal/532/
mac
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK17_aarch64_mac_Personal/504/
aix:
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK11_ppc64_aix_Personal/1523/
zos : (zos repos yet to be updated with the library)
https://hyc-runtimes-jenkins.swg-devops.com/job/Build_JDK11_s390x_zos_Personal/4469/

@keithc-ca
Copy link
Contributor

haven’t yet updated the SharedClassesAPI test to point to or load this new library path

The library should be in the test-image, so I would not expect any path to need to be updated.

@annaibm
Move SharedClassAPI test native code into openj9
d9faa9d 
 - Move the sharedClasses native code from openj9_systemtest into OpenJ9.
as part of removing the native compiler dependency in system tests.

related:adoptium/aqa-tests#5965

Signed-off-by: Anna Babu Palathingal <anna.bp@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pshipton pshipton pshipton left review comments

@keithc-ca keithc-ca keithc-ca requested changes

Assignees
No one assigned
Labels
comp:test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@annaibm @llxia @keithc-ca @pshipton
0