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.

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

Disable countDecimalDigits transformation for AOT compilations #16273

Merged
merged 1 commit into from
Nov 14, 2022
Merged

Disable countDecimalDigits transformation for AOT compilations #16273

merged 1 commit into from
Nov 14, 2022

Conversation

dchopra001
Copy link
Contributor

Fixes #15855

@dchopra001
Copy link
Contributor Author

dchopra001 commented Nov 7, 2022 •
edited by jdmpapin
Loading

Depends on: eclipse-omr/omr#6804

@dchopra001 dchopra001 mentioned this pull request Nov 7, 2022
Closed
@dchopra001 dchopra001 marked this pull request as ready for review November 8, 2022 03:22
@dchopra001
Copy link
Contributor Author

dchopra001 commented Nov 8, 2022 •
edited by jdmpapin
Loading

Tagging @jdmpapin @r30shah for review. Please see the dependent OMR PR eclipse-omr/omr#6804 as well.

@dchopra001
Copy link
Contributor Author

See #15855 (comment) for where this issue occurs.

r30shah
r30shah approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@r30shah r30shah left a comment

Choose a reason for hiding this comment

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

LGTM.

@r30shah
Copy link
Contributor

r30shah commented Nov 10, 2022

@jdmpapin As changes are related to Idiom Recognition, can you please review this change? We are looking to get this fixed in 0.36.

@dchopra001 dchopra001 mentioned this pull request Nov 11, 2022
Closed
@jdmpapin jdmpapin changed the title Add routine to disable idiomRecognition's countDecimalDigits transformation Disable countDecimalDigits transformation for AOT compilations Nov 11, 2022
@jdmpapin
Copy link
Contributor

I assume that you've verified that this still fixes #15855, but please confirm

Jenkins test sanity all jdk17

@jdmpapin
Copy link
Contributor

@r30shah, please re-review

@jdmpapin
Copy link
Contributor

Jenkins test sanity all jdk17

r30shah
r30shah approved these changes Nov 11, 2022
View reviewed changes
Copy link
Contributor

@r30shah r30shah left a comment

Choose a reason for hiding this comment

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

@jdmpapin good catch on the part when we would have disabled the transformation and its impact on all compilations. Change looks good to me.

@dchopra001
Disable countDecimalDigits transformation for AOT compilations
b7049bd 
Signed-off-by: Dhruv Chopra <Dhruv.C.Chopra@ibm.com>
@jdmpapin
Copy link
Contributor

Jenkins test sanity all jdk17

@dchopra001
Copy link
Contributor Author

I assume that you've verified that this still fixes #15855, but please confirm

I'm just checking my grinder results. There's some failures but I'm checking to make sure they are dups of another issue.

@dchopra001
Copy link
Contributor Author

In my ~50 job grinder I'm seeing a failure that looks similar to this issue.

Java version:

[2022-11-13T01:01:31.806Z] openjdk version "19.0.1-internal" 2022-10-18
[2022-11-13T01:01:31.806Z] OpenJDK Runtime Environment (build 19.0.1-internal-adhoc.jenkins.BuildJDK19s390xlinuxPersonal)
[2022-11-13T01:01:31.806Z] Eclipse OpenJ9 VM (build disableGenDecimal-b7049bda30c, JRE 19 Linux s390x-64-Bit Compressed References 20221111_58 (JIT enabled, AOT enabled)
[2022-11-13T01:01:31.806Z] OpenJ9   - b7049bda30c
[2022-11-13T01:01:31.806Z] OMR      - 0453d02bff7
[2022-11-13T01:01:31.806Z] JCL      - 110e1e7808e based on jdk-19.0.1+10)

OpenJ9 sha matches.

Failure:

[2022-11-13T01:23:30.099Z] STDOUT:
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 1330 ms (srcMode=0,dstMode=0)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 1078 ms (srcMode=0,dstMode=1)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 114 ms (srcMode=0,dstMode=2)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 1150 ms (srcMode=1,dstMode=0)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 1144 ms (srcMode=1,dstMode=1)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 394 ms (srcMode=1,dstMode=2)
[2022-11-13T01:23:30.099Z] Time needed for 5 threads to deflate/inflate: 1151 ms (srcMode=2,dstMode=0)
[2022-11-13T01:23:30.099Z] STDERR:
[2022-11-13T01:23:30.099Z] JVMJNCK001I JNI check utility installed. Use -Xcheck:jni:help for usage
[2022-11-13T01:23:30.099Z] 000003FF94479E00: Object neither in heap nor stack-allocated in thread MainThread
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	O-Slot=000003FEBC01C8C0
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	O-Slot value=000003FF9440E6B0
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	PC=000003FF194A8718
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	framesWalked=2
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	arg0EA=000003FEBC01C8D8
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	walkSP=000003FEBC01C7B8
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	literals=000003FF9420A3C8
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	jitInfo=000003FF03183BF8
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	method=000003FF94235360 (java/lang/Thread.genThreadName()Ljava/lang/String;) (JIT)
[2022-11-13T01:23:30.099Z] 000003FF94479E00:	stack=000003FEBC017478-000003FEBC01CCB0
[2022-11-13T01:23:30.099Z] 01:23:23.889 0x3febc044f00    j9mm.479    *   ** ASSERTION FAILED ** at /home/jenkins/workspace/Build_JDK19_s390x_linux_Personal/openj9/runtime/gc_glue_java/ScavengerRootScanner.hpp:109: ((MM_StackSlotValidator(MM_StackSlotValidator::NOT_ON_HEAP, *slotPtr, stackLocation, walkState).validate(_env)))

Taking a look to see why we're still seeing an issue here.

@dchopra001
Copy link
Contributor Author

It looks like the above issue is being investigated here: #15251.

I don't think this is the exact same issue. The transformation we disable here is only meant for Z platforms. But as per #15251 this issue is reproducible on multiple platforms.

@dchopra001
Copy link
Contributor Author

The only other failures I saw were:

[2022-11-13T05:01:14.478Z] JavaTest Message: JUnit Failure: testAwaitUninterruptibly_fair(ReentrantLockTest): timed out waiting for thread to enter thread state WAITING
[2022-11-13T05:01:14.478Z] junit.framework.AssertionFailedError: timed out waiting for thread to enter thread state WAITING
[2022-11-13T05:01:14.478Z] 	at junit.framework.Assert.fail(Assert.java:57)
[2022-11-13T05:01:14.478Z] 	at junit.framework.TestCase.fail(TestCase.java:223)
[2022-11-13T05:01:14.478Z] 	at JSR166TestCase.assertThreadBlocks(JSR166TestCase.java:1221)
[2022-11-13T05:01:14.478Z] 	at ReentrantLockTest.testAwaitUninterruptibly(ReentrantLockTest.java:954)
[2022-11-13T05:01:14.478Z] 	at ReentrantLockTest.testAwaitUninterruptibly_fair(ReentrantLockTest.java:922)

These are dups of: #15465

@jdmpapin
Copy link
Contributor

OK, thanks. I think this is a good change in any case

@jdmpapin jdmpapin merged commit d4aba17 into eclipse-openj9:master Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdmpapin jdmpapin jdmpapin approved these changes

@r30shah r30shah r30shah approved these changes

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

Successfully merging this pull request may close these issues.

jdk19 Compiling method: java/lang/Thread.genThreadName() crash vmstate 0005FFFF
3 participants
@dchopra001 @r30shah @jdmpapin