Fix null restricted array related issues for value types #20112

a7ehuo · 2024-09-04T18:07:53Z

(1) Add utility methods for null restricted arrays
(2) Fix null restricted array related issues

Add recognized method jdk/internal/value/ValueClass.newArrayInstance
Add VP fixed class constraint if ValueClass.newArrayInstance creates null restricted array
Rename isArrayCompTypePrimitiveValueType to isArrayNullRestricted to reflect the updated logic
Update isArrayNullRestricted on how to determine whether or not an array is a null restricted array
Disable transformation based on type hint which is no longer sufficient enough to decide whether or not the array is null restricted or not
If flattenable arrays are enabled, do not create fixed class constraint for parm array class based on signature since both nullable and null restricted arrays share the same signature

Depends on

Related: #19913, #19914, #19708

a7ehuo · 2024-09-04T18:29:48Z

@hzongaro May I ask you to review this change? Thank you very much!

This PR is created as a draft and marked as WIP for now because of all the dependencies. Regarding to the implementation itself, it is ready for review.

Since this PR depends on eclipse-omr/omr#7452, it should be review along with eclipse-omr/omr#7452.

hzongaro

I think these changes look good - just a few minor comments.

Could you expand on the reason for these changes in the commit comment? May I also ask you to remove the Markdown from the commit comment?

runtime/oti/j9.h

runtime/compiler/env/VMJ9.h

runtime/compiler/env/VMJ9.cpp

runtime/compiler/env/J9ClassEnv.cpp

runtime/compiler/env/VMJ9Server.cpp

hzongaro

Sorry - I somehow neglected to review the Value Propagation changes in my first pass through this pull request

runtime/compiler/optimizer/J9ValuePropagation.cpp

runtime/compiler/env/j9method.cpp

runtime/compiler/optimizer/J9ValuePropagation.cpp

a7ehuo · 2024-10-08T15:41:26Z

@hzongaro All comments have been addressed in the latest commits, except for the following three that will be addressed in the future PRs due to the complexity. Ready for another review. Thank you!

(1) #20112 (comment)
(2) #20112 (comment)
(3) #20112 (comment)

Please note that I have rebased this PR to the latest master branch and this PR requires coordinated merge with eclipse-omr/omr#7452.

hzongaro

Thanks for the updates! Just a few more comments.

runtime/compiler/env/j9method.cpp

runtime/compiler/optimizer/J9ValuePropagation.cpp

runtime/compiler/runtime/SymbolValidationManager.cpp

Whether or not an array is null-restricted is no longer decided by array component type but by the J9ClassArrayIsNullRestricted flag in array class. Only the nullRestrictedArrayClass of the J9Class has J9ClassArrayIsNullRestricted set. (1) Add getNullRestrictedArrayClassFromComponentClass to retrieve nullRestrictedArrayClass from J9Class (2) Add isArrayNullRestricted to check if an array class is null restricted by checking J9ClassArrayIsNullRestricted. Signed-off-by: Annabelle Huo <[email protected]>

a7ehuo · 2024-10-11T22:36:33Z

@hzongaro All comments are addressed. Ready for another review. Thank you!

I rebased the code in order to resolve the merge conflict in CommunicationStream.hpp

hzongaro

Thanks for the updates! Just a couple of more minor comments.

runtime/compiler/optimizer/J9ValuePropagation.cpp

a7ehuo · 2024-10-15T16:47:24Z

@hzongaro All comments are addressed. Ready for another review. Thank you!

hzongaro

Looks good. Thanks!

hzongaro · 2024-10-15T18:33:30Z

Jenkins test sanity.functional,extended.functional xlinuxval,plinuxval,zlinuxval,alinuxval jdknext depends eclipse-omr/omr#7452

hzongaro · 2024-10-15T18:34:24Z

Jenkins test sanity.functional,sanity.openjdk xlinuxvalst,plinuxvalst,zlinuxvalst,alinuxvalst jdknext depends eclipse-omr/omr#7452

hzongaro · 2024-10-15T18:41:40Z

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

a7ehuo · 2024-10-16T13:20:35Z

Test_openjdk11_j9_sanity.openjdk_aarch64_linux_Personal: Known issue #13756

[2024-10-15T20:03:29.491Z] java.lang.Exception
[2024-10-15T20:03:29.491Z] 	at ReplayCacheTestProc.main0(ReplayCacheTestProc.java:279)
[2024-10-15T20:03:29.491Z] 	at ReplayCacheTestProc.main(ReplayCacheTestProc.java:326)
[2024-10-15T20:03:29.491Z] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[2024-10-15T20:03:29.491Z] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[2024-10-15T20:03:29.491Z] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[2024-10-15T20:03:29.491Z] 	at java.base/java.lang.reflect.Method.invoke(Method.java:572)
[2024-10-15T20:03:29.491Z] 	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
[2024-10-15T20:03:29.491Z] 	at java.base/java.lang.Thread.run(Thread.java:839)
[2024-10-15T20:03:29.491Z] java.lang.Exception
[2024-10-15T20:03:29.491Z] 	at ReplayCacheTestProc.main0(ReplayCacheTestProc.java:279)
[2024-10-15T20:03:29.491Z] 	at ReplayCacheTestProc.main(ReplayCacheTestProc.java:326)
...
[2024-10-15T20:04:55.398Z] jdk_security4_0_FAILED

Test_openjdk11_j9_sanity.openjdk_x86-64_windows_Personal: Known issue #17743

[2024-10-16T02:47:35.873Z] java.lang.RuntimeException: PortUnreachableException not thrown.
[2024-10-16T02:47:35.873Z] 	at Unreachable.main(Unreachable.java:102)
...
[2024-10-16T02:52:31.349Z] jdk_security4_1_FAILED

Test_openjdk17_j9_sanity.openjdk_ppc64_aix_Personal: Known issue: #19962

[2024-10-15T22:10:03.705Z] ACTION: main -- Error. Agent communication error: java.io.EOFException; check console log for any additional details
[2024-10-15T22:10:03.705Z] REASON: Assumed action based on file name: run main TestCipherMode 
...
[2024-10-15T22:15:02.584Z] jdk_security2_1_FAILED

Test_openjdk17_j9_sanity.openjdk_x86-64_windows_Personal: Known issue #18708

[2024-10-16T01:51:10.631Z] Caused by: java.lang.OutOfMemoryError: Java heap space
[2024-10-16T01:51:10.631Z] 	at java.base/java.lang.String.repeat(String.java:5202)
...
[2024-10-16T02:02:18.562Z] jdk_util_0_FAILED

Test_openjdk8_j9_sanity.functional_x86-64_linux_Personal: Known issue: #19224

[2024-10-15T22:44:29.181Z]  [ERR] /home/jenkins/workspace/Test_openjdk8_j9_sanity.functional_x86-64_linux_Personal_testList_1/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/jitserver/jitserverconfig.sh: line 30: lsof: command not found
...
[2024-10-15T22:44:29.181Z] testJitserverArguments_0_FAILED

Valhalla test passes on x86-64 which is what I have tested locally. Valhalla on other platforms (Power, Z, aarch64) fail, which I will take a look at next.

I'm not sure about vt_standard tests yet

hzongaro · 2024-10-16T13:34:21Z

I took a quick look at the sanity.functional and sanity.openjdk "vt-standard" testing. It looks like those tests have seen many failures in recent runs. For instance, an aarch64 Linux sanity.functional vt-standard PR test run from earlier this month yielded 55 failures.

We'll need to do further investigation, but it seems unlikely that this pull request is responsible for the failures. These failures will not hold back merging this pull request.

a7ehuo · 2024-10-16T20:06:15Z

Valhalla test passes on x86-64 which is what I have tested locally. Valhalla on other platforms (Power, Z, aarch64) fail, which I will take a look at next.

Two main tests fail on Power, Z, and Aarch64: ValueTypeArrayTestsJIT and ValueTypeSystemArraycopyTests.

I looked at the failure in ValueTypeArrayTestsJIT first. The failure is related to how instanceOf is evaluated on Power, Z, and Aarch64. On x86, we call helper jitInstanceOf which explains why it passes.

It failed because instanceOf doesn't think null restricted array PointPV[] is an instance of [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV; in ValueTypeArrayTests.runTest

[ValueTypeArrayTests] [INFO] calling runTest testArrays[3] NullRestrictedArray nullObj kinds[0] OBJ_TYPE 1 kinds[0] OBJ_TYPE 1
[ValueTypeArrayTests] [INFO] --------------------------
[ValueTypeArrayTests] [INFO] runTest staticArrayKind=OBJ_TYPE 1 staticSourceKind=OBJ_TYPE 1
[ValueTypeArrayTests] [INFO] runTest arr: actualArrayKind=IFACE_TYPE 2 sourceVal: actualSourceKind=NULL_REF 0 //<--- wrong actualArrayKind should be PRIM_TYPE 4 since testArrays[3] is NullRestrictedArray
[ValueTypeArrayTests] [INFO] runTest expectedExceptionClass null
[ValueTypeArrayTests] [INFO] assignDispatch arrKind=OBJ_TYPE srcKind=OBJ_TYPE idx=1
[ValueTypeArrayTests] [INFO] runTest caughtThrowable=java.lang.ArrayStoreException

hzongaro · 2024-10-17T14:27:53Z

I looked at the failure in ValueTypeArrayTestsJIT first. The failure is related to how instanceOf is evaluated on Power, Z, and Aarch64. On x86, we call helper jitInstanceOf which explains why it passes.

It failed because instanceOf doesn't think null restricted array PointPV[] is an instance of [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV; in ValueTypeArrayTests.runTest

Thanks, @a7ehuo! Should the relevant tests still be disabled and an issue opened, or do you plan to try to fix this problem in this pull request?

a7ehuo · 2024-10-17T14:56:21Z

I plan to disable the related two tests (ValueTypeArrayTestsJIT, ValueTypeSystemArraycopyTests) for now and open new PRs when ready.

There are already issues opened on them. I think I will reuse them for future submission. I will make a note in these two issues that this PR doesn't fix them on all hardware.
#19913
#19914

I'll run some Jenkins Valhalla tests on all platforms before pushing another commit

(1) Add recognized methods from jdk/internal/value/ValueClass and jdk/internal/value/NullRestrictedCheckedType (2) Add VP fixed class constraint if ValueClass.newArrayInstance creates null restricted array using NullRestrictedCheckedType (3) Rename isArrayCompTypePrimitiveValueType to isArrayNullRestricted to reflect the updated logic Update isArrayNullRestricted on how to determine whether or not an array is a null restricted array (4) Disable transformation based on type hint which is no longer sufficient enough to decide whether or not the array is null restricted or not (5) If flattenable arrays are enabled, do not create fixed class constraint for parm array class based on signature since both nullable and null restricted arrays share the same signature Related: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708 Signed-off-by: Annabelle Huo <[email protected]>

a7ehuo · 2024-10-18T17:47:09Z

@hzongaro I reverted test/functional/Valhalla/playlist.xml to baseline and tested internally that the Valhalla functional tests pass on all platforms. It's ready for another review. Thanks you!

hzongaro

Looks good. Thanks! I'll rerun testing, in order to verify that the playlist.xml was updated correctly, and also so I can ensure the coordinated merge that will be required can proceed.

hzongaro · 2024-10-21T01:28:26Z

Jenkins test sanity.functional,extended.functional xlinuxval,plinuxval,zlinuxval,alinuxval jdknext depends eclipse-omr/omr#7452

hzongaro · 2024-10-21T01:29:07Z

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

hzongaro · 2024-10-21T12:13:30Z

Jenkins test sanity.functional,sanity.openjdk xmac jdk11 depends eclipse-omr/omr#7452

a7ehuo · 2024-10-21T14:06:22Z

Checked three failed tests and all known issues (details as below).

Waiting for openjdk11_j9_sanity.functional_x86-64_mac to complete

Test_openjdk21_j9_sanity.openjdk_x86-64_mac: Known issue: #19987

[2024-10-21T02:28:49.205Z] STDERR:
[2024-10-21T02:28:49.205Z] javax.security.auth.login.LoginException: Unable to obtain Principal Name for authentication 
[2024-10-21T02:28:49.205Z] 	at jdk.security.auth/com.sun.security.auth.module.Krb5LoginModule.promptForName(Krb5LoginModule.java:818)
…

[2024-10-21T02:29:13.877Z] -----------------------------------
[2024-10-21T02:29:13.877Z] jdk_security4_1_FAILED
[2024-10-21T02:29:13.877Z] -----------------------------------

Test_openjdk8_j9_sanity.functional_x86-64_linux: Known issue: #18599

[2024-10-21T04:03:57.367Z]  [ERR] /home/jenkins/workspace/Test_openjdk8_j9_sanity.functional_x86-64_linux_Personal_testList_1/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/jitserver/jitserverconfig.sh: line 30: lsof: command not found
[2024-10-21T04:03:57.367Z]  [ERR] /home/jenkins/workspace/Test_openjdk8_j9_sanity.functional_x86-64_linux_Personal_testList_1/aqa-tests/TKG/../../jvmtest/functional/cmdLineTests/jitserver/jitserverconfig.sh: line 30: lsof: command not found
[2024-10-21T04:03:57.367Z]  [ERR] Generating RSA private key, 2048 bit long modulus
…
[2024-10-21T04:03:57.367Z]  [ERR] failed to configure SSL ecdh: Success
[2024-10-21T04:03:57.367Z]  [ERR] Failed to initialize the SSL context
[2024-10-21T04:03:57.367Z]  [ERR] 
[2024-10-21T04:03:57.367Z]  [ERR] JITServer is ready to accept incoming requests
…

[2024-10-21T04:04:04.002Z] -----------------------------------
[2024-10-21T04:04:04.002Z] testJitserverArguments_0_FAILED
[2024-10-21T04:04:04.002Z] -----------------------------------

Test_openjdk8_j9_sanity.openjdk_s390x_linux: Known issue: #14948 (comment)

[2024-10-21T03:27:55.722Z] Caused by: java.io.IOException: write beyond end of stream
[2024-10-21T03:27:55.722Z] 	at java.util.zip.DeflaterOutputStream.write(DeflaterOutputStream.java:201)
[2024-10-21T03:27:55.722Z] 	at CloseInflaterDeflaterTest.lambda$testDeflaterOutputStream$4(CloseInflaterDeflaterTest.java:153)
[2024-10-21T03:27:55.722Z] 	at org.testng.Assert.expectThrows(Assert.java:1005)
[2024-10-21T03:27:55.722Z] 	... 30 more
[2024-10-21T03:27:55.722Z] test CloseInflaterDeflaterTest.testDeflaterOutputStream(false): failure
…
[2024-10-21T03:27:55.723Z] Caused by: java.io.IOException: write beyond end of stream
[2024-10-21T03:27:55.723Z] 	at java.util.zip.DeflaterOutputStream.write(DeflaterOutputStream.java:201)
[2024-10-21T03:27:55.723Z] 	at CloseInflaterDeflaterTest.lambda$testDeflaterOutputStream$4(CloseInflaterDeflaterTest.java:153)
[2024-10-21T03:27:55.723Z] 	at org.testng.Assert.expectThrows(Assert.java:1005)
[2024-10-21T03:27:55.723Z] 	... 30 more
[2024-10-21T03:27:55.723Z] test CloseInflaterDeflaterTest.testGZip(true): failure
…
[2024-10-21T03:27:55.723Z] test CloseInflaterDeflaterTest.testZipCloseEntry(java.util.zip.ZipOutputStream@108de3fe): failure
[2024-10-21T03:27:55.723Z] java.io.IOException
[2024-10-21T03:27:55.723Z] 	at CloseInflaterDeflaterTest$1.write(CloseInflaterDeflaterTest.java:54)
…
[2024-10-21T03:28:01.017Z] -----------------------------------
[2024-10-21T03:28:01.017Z] jdk_util_1_FAILED
[2024-10-21T03:28:01.017Z] -----------------------------------

a7ehuo · 2024-10-21T14:32:49Z

Test_openjdk11_j9_sanity.functional_x86-64_mac has passed

hzongaro · 2024-10-21T14:41:16Z

Failures were due to known issues. OMR pull request eclipse-omr/omr#7452 that this change depends upon has been merged and promoted. Merging.

a7ehuo added comp:jit project:valhalla Used to track Project Valhalla related work depends:omr Pull request is dependent on a corresponding change in OMR depends:omr:breaking labels Sep 4, 2024

a7ehuo requested a review from hzongaro September 4, 2024 18:12

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch 2 times, most recently from c469e30 to 516a676 Compare September 4, 2024 18:21

a7ehuo mentioned this pull request Sep 4, 2024

Fix null restricted array related issues for value types eclipse-omr/omr#7452

Merged

1 task

hzongaro reviewed Sep 9, 2024

View reviewed changes

theresa-m mentioned this pull request Sep 9, 2024

Always create nullable array before null-restricted #20139

Merged

theresa-m mentioned this pull request Sep 18, 2024

Remove Q type checks in Valhalla code #18157

Open

a7ehuo mentioned this pull request Oct 3, 2024

Value Types: java.lang.ArrayStoreException with null-restricted array with -Xint #20223

Closed

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch 2 times, most recently from bb24a35 to 3af4ff9 Compare October 8, 2024 15:33

a7ehuo marked this pull request as ready for review October 8, 2024 15:34

a7ehuo requested a review from dsouzai as a code owner October 8, 2024 15:34

a7ehuo changed the title ~~WIP: Fix null restricted array related issues for value types~~ Fix null restricted array related issues for value types Oct 8, 2024

hzongaro reviewed Oct 11, 2024

View reviewed changes

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch 5 times, most recently from d9a0a5c to de9f0d5 Compare October 11, 2024 22:19

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch from de9f0d5 to 135b856 Compare October 11, 2024 22:32

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch from 135b856 to 6782be9 Compare October 11, 2024 22:40

hzongaro reviewed Oct 15, 2024

View reviewed changes

runtime/compiler/optimizer/J9ValuePropagation.cpp Outdated Show resolved Hide resolved

runtime/compiler/optimizer/J9ValuePropagation.cpp Outdated Show resolved Hide resolved

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch from 6782be9 to cc811af Compare October 15, 2024 16:46

hzongaro approved these changes Oct 15, 2024

View reviewed changes

hzongaro mentioned this pull request Oct 17, 2024

java.lang.IllegalAccessException thrown for jdk.internal.misc.Unsafe.theUnsafe in Valhalla vt_standard testing #20372

Open

a7ehuo force-pushed the PR-valuetypes-null-restricted-array branch from cc811af to f2c432a Compare October 18, 2024 17:44

a7ehuo mentioned this pull request Oct 18, 2024

Fix ValueTypeArrayTestsJIT test failures #19913

Open

hzongaro approved these changes Oct 21, 2024

View reviewed changes

hzongaro merged commit f165fd4 into eclipse-openj9:master Oct 21, 2024
96 of 99 checks passed

a7ehuo deleted the PR-valuetypes-null-restricted-array branch October 28, 2024 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix null restricted array related issues for value types #20112

Fix null restricted array related issues for value types #20112

a7ehuo commented Sep 4, 2024 •

edited

Loading

a7ehuo commented Sep 4, 2024

hzongaro left a comment

hzongaro left a comment

a7ehuo commented Oct 8, 2024

hzongaro left a comment

a7ehuo commented Oct 11, 2024

hzongaro left a comment

a7ehuo commented Oct 15, 2024

hzongaro left a comment

hzongaro commented Oct 15, 2024

hzongaro commented Oct 15, 2024

hzongaro commented Oct 15, 2024

a7ehuo commented Oct 16, 2024

hzongaro commented Oct 16, 2024 •

edited

Loading

a7ehuo commented Oct 16, 2024

hzongaro commented Oct 17, 2024

a7ehuo commented Oct 17, 2024

a7ehuo commented Oct 18, 2024

hzongaro left a comment

hzongaro commented Oct 21, 2024

hzongaro commented Oct 21, 2024

hzongaro commented Oct 21, 2024

a7ehuo commented Oct 21, 2024 •

edited

Loading

a7ehuo commented Oct 21, 2024

hzongaro commented Oct 21, 2024

Fix null restricted array related issues for value types #20112

Fix null restricted array related issues for value types #20112

Conversation

a7ehuo commented Sep 4, 2024 • edited Loading

a7ehuo commented Sep 4, 2024

hzongaro left a comment

Choose a reason for hiding this comment

hzongaro left a comment

Choose a reason for hiding this comment

a7ehuo commented Oct 8, 2024

hzongaro left a comment

Choose a reason for hiding this comment

a7ehuo commented Oct 11, 2024

hzongaro left a comment

Choose a reason for hiding this comment

a7ehuo commented Oct 15, 2024

hzongaro left a comment

Choose a reason for hiding this comment

hzongaro commented Oct 15, 2024

hzongaro commented Oct 15, 2024

hzongaro commented Oct 15, 2024

a7ehuo commented Oct 16, 2024

hzongaro commented Oct 16, 2024 • edited Loading

a7ehuo commented Oct 16, 2024

hzongaro commented Oct 17, 2024

a7ehuo commented Oct 17, 2024

a7ehuo commented Oct 18, 2024

hzongaro left a comment

Choose a reason for hiding this comment

hzongaro commented Oct 21, 2024

hzongaro commented Oct 21, 2024

hzongaro commented Oct 21, 2024

a7ehuo commented Oct 21, 2024 • edited Loading

a7ehuo commented Oct 21, 2024

hzongaro commented Oct 21, 2024

a7ehuo commented Sep 4, 2024 •

edited

Loading

hzongaro commented Oct 16, 2024 •

edited

Loading

a7ehuo commented Oct 21, 2024 •

edited

Loading