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

JDK16 Changes #11322

Merged
merged 3 commits into from
Dec 2, 2020
Merged

JDK16 Changes #11322

merged 3 commits into from
Dec 2, 2020

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Dec 1, 2020

  1. Add String.getBytes for JDK16.

  2. Add Lookup.ORIGINAL and update defineHiddenClassWithClassData for JDK16.

  3. Add implementation of JavaLangAccess.addExports for JDK16.

Closes: #11312

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

@pshipton
Copy link
Member

pshipton commented Dec 1, 2020

jenkins test sanity zlinux jdknext depends ibmruntimes/openj9-openjdk-jdk#openj9-staging

@babsingh
Copy link
Contributor Author

babsingh commented Dec 1, 2020

@pshipton Made a minor correction. The PR builds would need to be relaunched.

@tajila
Copy link
Contributor

tajila commented Dec 1, 2020

jenkins test sanity zlinux jdknext depends ibmruntimes/openj9-openjdk-jdk#openj9-staging

@tajila
Copy link
Contributor

tajila commented Dec 1, 2020

what change was made?

@babsingh
Copy link
Contributor Author

babsingh commented Dec 1, 2020

what change was made?

fixed typo/grammar in String.getBytes description.

@pshipton
Copy link
Member

pshipton commented Dec 1, 2020

FYI new builds aren't necessarily required for changes to comments. The original builds continue on even if you push more changes and they are no longer shown in the PR.

test_insert12 and test_insert3 are failing in JCL_Test, which means we won't automatically promote jdknext even if these changes are merged.

@pshipton
Copy link
Member

pshipton commented Dec 1, 2020

@tajila fyi if you look at the forced-push link in babsingh force-pushed the babsingh:jdk16_comp branch from 277aa79 to a7b6eaf 2 hours ago above, you can see the changes
https://github.com/eclipse/openj9/compare/277aa796c83429afca607c280c1d68aa89051394..a7b6eaf0b7d83ff02fb1644519940e42d15c438e

@keithc-ca
Copy link
Contributor

A recent addition to JavaLangAccess (void addExports(Module module, String packageName)) must be implemented.

@babsingh
Copy link
Contributor Author

babsingh commented Dec 2, 2020

Update:

  1. I will temporarily disable test_insert12 and test_insert3 to enable jdknext builds. Investigating and fixing these failures will take some time.
  2. I will also add JavaLangAccess.addExports to this PR.

@keithc-ca
Copy link
Contributor

Perhaps the failure in test_insert12 was fixed by openjdk?

  • 8257511: JDK-8254082 brings regression to AbstractStringBuilder.insert(int dstOffset, CharSequence s, int start, int end)

See https://github.com/ibmruntimes/openj9-openjdk-jdk/blame/11ff3afa7597ba8742399ddf387ab5e17a9929f8/src/java.base/share/classes/java/lang/AbstractStringBuilder.java#L1720

@babsingh
Copy link
Contributor Author

babsingh commented Dec 2, 2020

Perhaps the failure in test_insert12 was fixed by openjdk?

That looks like the fix. Rebuilding with the latest source to confirm.

@babsingh
Copy link
Contributor Author

babsingh commented Dec 2, 2020

The PR has been updated to include JavaLangAccess.addExports.

The OpenJDK commit mentioned in #11322 (comment) fixes test_insert12 and test_insert3.

@pshipton
Copy link
Member

pshipton commented Dec 2, 2020

jenkins test sanity zlinux jdknext depends ibmruntimes/openj9-openjdk-jdk#openj9-staging

@pshipton
Copy link
Member

pshipton commented Dec 2, 2020

Related: eclipse-openj9#11312

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
The new Lookup.ORIGINAL field (= 0x40) conflicts with
Lookup.INTERNAL_PRIVILEGED. Since INTERNAL_PRIVILEGED is J9 specific, it
is given a different value (= 0x80).

In JDK16, defineHiddenClassWithClassData has changed to a public method;
and the initOption is supplied by the caller instead of it always being
true.

Related:
eclipse-openj9#11312
eclipse-openj9#11321

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

pshipton commented Dec 2, 2020

PR testing passed. The only change was to a comment so it's still valid.

@keithc-ca keithc-ca merged commit 9f11871 into eclipse-openj9:master Dec 2, 2020
@pshipton
Copy link
Member

pshipton commented Dec 3, 2020

@babsingh Seems we should have tested jdk15, as it no longer compiles with this change. The old definition of defineHiddenClassWithClassData() didn't get removed and now there are two.

22:44:42  /home/jenkins/workspace/Build_JDK15_ppc64_aix_Nightly/build/aix-ppc64-server-release/support/j9jcl/java.base/share/classes/java/lang/invoke/MethodHandles.java:2075: error: method defineHiddenClassWithClassData(byte[],Object,ClassOption...) is already defined in class Lookup
22:44:42  		Lookup defineHiddenClassWithClassData(byte[] bytes, Object classData, ClassOption... classOptions) throws IllegalAccessException {

babsingh added a commit to babsingh/openj9 that referenced this pull request Dec 3, 2020
Related: eclipse-openj9#11322

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
@babsingh babsingh deleted the jdk16_comp branch January 29, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jdknext build problems String.getBytes()
4 participants