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

Support Java 16 #16268

Open
lucagiac81 opened this issue Jun 15, 2021 · 7 comments
Open

Support Java 16 #16268

lucagiac81 opened this issue Jun 15, 2021 · 7 comments

Comments

@lucagiac81
Copy link

OpenJDK 16 introduces several new features, such as a Vector API, and it’d be beneficial to have those features available in Presto.
Currently, building/launching Presto with OpenJDK 16 causes some errors. Observed errors and proposed solutions are documented below. The errors were observed when building with -DskipTest (tests still need to be examined).
This effort would also help support JDK 17 (next LTS release) in the long run.

spotbugs-maven-plugin

[ERROR] Failed to execute goal com.github.spotbugs:spotbugs-maven-plugin:3.1.10:spotbugs (spotbugs) on project presto-root: Execution spotbugs of goal com.github.spotbugs:spotbugs-maven-plugin:3.1.10:spotbugs failed: Unable to load the mojo 'spotbugs' in the plugin 'com.github.spotbugs:spotbugs-maven-plugin:3.1.10'. A required class is missing: Could not initialize class org.codehaus.groovy.vmplugin.v7.Java7

  • com.github.spotbugs:spotbugs-maven-plugin:3.1.10 is specified in pom.xml of com.facebook.airlift:airbase:102 (parent POM of presto-root).
  • Upgrading to spotbugs-maven-plugin:4.2.2 resolves the error.
  • The latest version of airbase (110) does use spotbugs-maven-plugin:4.2.2.
    • So, updating airbase from 102 to 110 in presto-root would be sufficient.
    • However, there is no artifact for airbase:110 in Maven repository (102 is the latest published). An artifact for airbase:110 should be generated.
  • For testing purposes, I specified spotbugs-maven-plugin:4.2.2 directly in presto-root pom.xml.

provisio-maven-plugin

Several errors are caused by the following change in OpenJDK 16: Strongly Encapsulate JDK Internals by Default
Example of one such error:

[ERROR] Failed to execute goal io.takari.maven.plugins:provisio-maven-plugin:0.1.40:provision (default-provision) on project presto-tpch: Execution default-provision of goal io.takari.maven.plugins:provisio-maven-plugin:0.1.40:provision failed: An API incompatibility was encountered while executing io.takari.maven.plugins:provisio-maven-plugin:0.1.40:provision: java.lang.ExceptionInInitializerError: null

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.Comparator java.util.TreeMap.comparator accessible: module java.base does not "opens java.util" to unnamed module @33423872
    at java.lang.reflect.AccessibleObject.checkCanSetAccessible (AccessibleObject.java:357)
    at java.lang.reflect.AccessibleObject.checkCanSetAccessible (AccessibleObject.java:297)
    at java.lang.reflect.Field.checkCanSetAccessible (Field.java:177)
    at java.lang.reflect.Field.setAccessible (Field.java:171)
    at com.thoughtworks.xstream.core.util.Fields.locate (Fields.java:39)
    at com.thoughtworks.xstream.converters.collections.TreeMapConverter.<clinit> (TreeMapConverter.java:50)
    at com.thoughtworks.xstream.XStream.setupConverters (XStream.java:807)
    at com.thoughtworks.xstream.XStream.<init> (XStream.java:574)
    at com.thoughtworks.xstream.XStream.<init> (XStream.java:496)
    at com.thoughtworks.xstream.XStream.<init> (XStream.java:465)
    at com.thoughtworks.xstream.XStream.<init> (XStream.java:411)
    at com.thoughtworks.xstream.XStream.<init> (XStream.java:350)
    at io.provis.model.io.RuntimeReader.<init> (RuntimeReader.java:52)
    at io.tesla.maven.plugins.provisio.Provisio.parseDescriptor (Provisio.java:89)
    at io.tesla.maven.plugins.provisio.Provisio.findDescriptorsForPackagingTypeInExtensionRealms (Provisio.java:79)
...
  • Updating to provisio-maven-plugin:0.1.56 (latest) does not resolve the error.
  • A workaround to prevent the error above and related ones is to add the following flags to MAVEN_OPTS:
    --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.text=ALL-UNNAMED --add-opens java.base/java.awt.font=ALL-UNNAMED --add-opens java.desktop/java.awt.font=ALL-UNNAMED
  • Alternatively, the following flag works as well
    --illegal-access=permit
  • A more robust solution would be to update XStream and provisio-maven-plugin to be compatible with OpenJDK 16 encapsulation.

Guice

With the two changes above, Presto builds successfully with OpenJDK 16. However, when launching Presto, Guice throws exceptions. For example:

com.google.inject.Guice An exception was caught and reported. Message: java.lang.NoClassDefFoundError: Could not initialize class com.google.inject.internal.cglib.core.$MethodWrapper

com.google.inject.Guice An exception was caught and reported. Message: java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class 
java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain) throws java.lang.ClassFormatError accessible: module java.base does not "opens java.lang" to unnamed module @5fb2de77
  • The error is resolved by updating Guice from 4.2.2 to 5.0.1.
  • This would again be achieved by using airbase:110 as parent POM.
  • For testing purposes, I specified guice:5.0.1 directly in presto-root pom.xml.

With this change Presto launches correctly. I was able to run a few queries with no issues. I still have to run tests and determine if there are more errors to address.

@tdcmeehan
Copy link
Contributor

tdcmeehan commented Aug 5, 2021

A couple of points that come to mind:

  • With Presto-on-Spark, Presto is now a library that is used as a Spark job. Spark 3 only supports Java 11, hence that puts an upper bound on the JVM supported. One possible option to allow us to use Java 16 as a compilation target is to introduce multi-release JARs and separate the implementation into Java 11 and Java 16 versions where the performance will benefit greatly, however the implications of this (such as code duplication) need to be considered.
  • It's likely that further work will be needed to enhance Presto's code generation library to support Java 16 features. In particular, I'd expect we need to revamp our code generation capabilities to be Java 16 compatible, as I'd expect the greatest wins from vectorization to be within heavily used operators which may already use codegen.

@yingsu00
Copy link
Contributor

Thank you @lucagiac81 for the great summary! Very excited to see Java 16 is finally coming.

Agree with @tdcmeehan that we need to enhance Presto's code generation library to support Java 16 features. Currently some of Presto engine is using byte code gen, e.g. projection and aggregation. But the major cost is in scan and Hive readers which are not using byte code generation. There are lots of low hanging fruits there and will benefit users with interactive queries immediately.

@zhengxingmao
Copy link
Contributor

Is there any update so far?
At present, I have investigated two ways to generate CodeGen: one is to use Weld to generate native CodeGen, and the other is to use Java CodeGen completely based on jdk17.
Any better suggestions @lucagiac81.

@lucagiac81
Copy link
Author

I'm currently focusing on filtering in the ORC reader (no bytecode generation).
I noticed that ASM 9.2 supports up to Java 18, and I was able to run Presto with JDK 18 by upgrading from 9.0 to 9.2. For vectorization, we should be able to call the Vector API from the generated bytecode. It may be convenient to create some abstractions to simplify integration and make the code more readable. Are there other limitations that you're concerned about?

@zhengxingmao
Copy link
Contributor

I don't know which is faster using native mode or Java 17 mode, which needs to be verified. You are right. At present, using java 17 + ASM is the most convenient way, and the amount of code modification will be very small.

@mbasmanova
Copy link
Contributor

Wondering what's the status of this issue and related #16486. More context: #22975

It looks like Trino moved to Java 22:

"Trino requires a 64-bit version of Java 22, with a minimum required version of 22.0.0. Earlier versions such as Java 8, Java 11, Java 17 or Java 21 do not work. Newer versions such as Java 23 are not supported – they may work, but are not tested."

CC: @tdcmeehan @amitkdutta

@lucagiac81
Copy link
Author

PR #21670 includes adding support for JDK 21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants