prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
16.08k stars 5.39k forks source link

Support Java 16 #16268

Open lucagiac81 opened 3 years ago

lucagiac81 commented 3 years ago

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

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)
...

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

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 commented 3 years ago

A couple of points that come to mind:

yingsu00 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 4 months ago

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 commented 4 months ago

PR #21670 includes adding support for JDK 21.