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

Unblock JDK-next (JDK18) acceptance build #13890

Merged
merged 4 commits into from
Nov 10, 2021

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Nov 9, 2021

1. Signature of NativeLibraries.load has changed in JDK18

Update the signature of NativeLibraries.load in native code for JDK18.

2. Set the useDirectMethodHandle Java flag to false

Setting jdk.reflect.useDirectMethodHandle to false disables the new JEP 416
changes where the reflection API is dependent on the MethodHandle API.

This flag will be reenabled once OpenJ9 supports JEP 416.

Related: #13852
Related: #13880

3. Update MHN.getMemberVMInfo to work with MemberName.vminfoIsConsistent

MemberName.vminfoIsConsistent is only used in an assertion to validate the
vmtarget and vmindex elements of MemberName. It relies on MHN.getMemberVMInfo
to retrieve the vmtarget and vmindex elements of MemberName.

For correct validation, MHN.getMemberVMInfo should return 0 for vmindex when
the vTableIndex is 0 and ref kind is invokevirtual or invokeinterface.
Otherwise, it should return -1 for vmindex. MHN.getMemberVMInfo has been
updated to perform this behaviour. This in return fixes the behaviour of
MemberName.vminfoIsConsistent.

Closes: #13877

4. InjectedInvoker behaviour modified in JDK18

In JDK17, InjectedInvoker does not have the NESTMATE attribute. In JDK18,
InjectedInvoker's definition has been updated, and it is given the NESTMATE
attribute. The new changes invalidate the filter that is used to identify
InjectedInvoker classes for modifying its class name. The filter has been
updated to work with JDK18.

Closes: #13878

Co-authored-by: Jason Feng fengj@ca.ibm.com
Co-authored-by: Jack Lu Jack.S.Lu@ibm.com
Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

Update the signature of NativeLibraries.load in native code for JDK18.

Co-authored-by: Jason Feng <fengj@ca.ibm.com>
Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh
Copy link
Contributor Author

babsingh commented Nov 9, 2021

Started Personal build #11090 to validate that the above changes will unblock the JDK-next acceptance build.

fyi @JasonFengJ9 @fengxue-IS @tajila @pshipton @keithc-ca

@pshipton
Copy link
Member

pshipton commented Nov 9, 2021

Started a non-promoting acceptance build with these changes https://openj9-jenkins.osuosl.org/job/Pipeline-OpenJDK-Acceptance/147/

@pshipton
Copy link
Member

pshipton commented Nov 9, 2021

jenkins test sanity zlinux jdk11,jdk17

@pshipton
Copy link
Member

pshipton commented Nov 9, 2021

jenkins compile win32 jdk8

@pshipton pshipton requested a review from 0xdaryl November 9, 2021 21:21
@pshipton
Copy link
Member

All the testing I started looks good. I don't expect we'll need to re-run it all if the only changes are comments and formatting.

Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

I approve the JIT-specific code.

Setting jdk.reflect.useDirectMethodHandle to false disables the new JEP 416
changes where the reflection API is dependent on the MethodHandle API.

This flag will be reenabled once OpenJ9 supports JEP 416.

Related: eclipse-openj9#13852
Related: eclipse-openj9#13880

Co-authored-by: Jason Feng <fengj@ca.ibm.com>
Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Copy link
Contributor Author

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

@JasonFengJ9 @keithc-ca All feedback has been addressed.

runtime/bcutil/ROMClassBuilder.cpp Show resolved Hide resolved
babsingh and others added 2 commits November 10, 2021 00:49
MemberName.vminfoIsConsistent is only used in an assertion to validate the
vmtarget and vmindex elements of MemberName. It relies on MHN.getMemberVMInfo
to retrieve the vmtarget and vmindex elements of MemberName.

For correct validation, MHN.getMemberVMInfo should return 0 for vmindex when
the vTableIndex is 0 and ref kind is invokevirtual or invokeinterface.
Otherwise, it should return -1 for vmindex. MHN.getMemberVMInfo has been
updated to perform this behaviour. This in return fixes the behaviour of
MemberName.vminfoIsConsistent.

Closes: eclipse-openj9#13877

Co-authored-by: Jack Lu <Jack.S.Lu@ibm.com>
Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
In JDK17, InjectedInvoker does not have the NESTMATE attribute. In JDK18,
InjectedInvoker's definition has been updated, and it is given the NESTMATE
attribute. The new changes invalidate the filter that is used to identify
InjectedInvoker classes for modifying its class name. The filter has been
updated to work with JDK18.

Closes: eclipse-openj9#13878

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Copy link
Member

@JasonFengJ9 JasonFengJ9 left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @babsingh

@keithc-ca
Copy link
Contributor

@pshipton
Copy link
Member

@tajila once you approve, pls leave the merging to me. I think we'll need to run the acceptance build again as we've already got new content, and jdknext needs to promote at the same time as this is merged or nightly builds will be broken.

@pshipton pshipton merged commit 4911a34 into eclipse-openj9:master Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants