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

Use Multi-Release JAR instead of reflection for decompression #25158

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ jobs:
- name: Maven Checks
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
$MAVEN clean verify -B --strict-checksums -V -T 1C -DskipTests -P ci
$MAVEN clean install verify -B --strict-checksums -V -T 1C -DskipTests -P ci
Copy link
Member

Choose a reason for hiding this comment

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

verify is redundant now ..

Suggested change
$MAVEN clean install verify -B --strict-checksums -V -T 1C -DskipTests -P ci
$MAVEN clean install -B --strict-checksums -V -T 1C -DskipTests -P ci

- name: Remove Trino from local Maven repo to avoid caching it
# Avoid caching artifacts built in this job, cache should only include dependencies
if: steps.cache.outputs.cache-hit != 'true' && matrix.cache == 'true'
Expand Down Expand Up @@ -197,7 +197,7 @@ jobs:
run: |
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
# Skip checks, these are run in `maven-checks` job and e.g. checkstyle is expensive.
$MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \
$MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$MAVEN ${MAVEN_TEST} -T 1C clean compile test-compile install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \
$MAVEN ${MAVEN_TEST} -T 1C clean install -DskipTests -Dair.check.skip-all=true ${MAVEN_GIB} -Dgib.buildUpstream=never -P errorprone-compiler \

-pl '!:trino-docs,!:trino-server'

test-jdbc-compatibility:
Expand Down Expand Up @@ -328,7 +328,7 @@ jobs:
- name: Maven Tests
id: tests
run: |
$MAVEN test ${MAVEN_TEST} -pl '
$MAVEN test install ${MAVEN_TEST} -pl '
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$MAVEN test install ${MAVEN_TEST} -pl '
$MAVEN install ${MAVEN_TEST} -pl '

!:trino-base-jdbc,
!:trino-bigquery,
!:trino-cassandra,
Expand Down
12 changes: 12 additions & 0 deletions client/trino-cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,18 @@
</executions>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<archive>
<manifestEntries combine.children="append">
<Multi-Release>true</Multi-Release>
</manifestEntries>
</archive>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
Expand Down
56 changes: 50 additions & 6 deletions client/trino-client/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@
<artifactId>aircompressor</artifactId>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>aircompressor-v3</artifactId>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>units</artifactId>
Expand All @@ -91,12 +96,6 @@
<optional>true</optional>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>aircompressor-v3</artifactId>
<scope>runtime</scope>
</dependency>

<dependency>
<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
Expand Down Expand Up @@ -154,6 +153,51 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<executions>
<execution>
<id>compile-java-11</id>
<goals>
<goal>compile</goal>
</goals>
<phase>compile</phase>
<configuration>
<multiReleaseOutput>true</multiReleaseOutput>
<release>11</release>
<compileSourceRoots>
<compileSourceRoot>${project.basedir}/src/main/java</compileSourceRoot>
</compileSourceRoots>
</configuration>
</execution>
<execution>
<id>compile-java-22</id>
<goals>
<goal>compile</goal>
</goals>
<phase>compile</phase>
<configuration>
<multiReleaseOutput>true</multiReleaseOutput>
<release>22</release>
<compileSourceRoots>
<compileSourceRoot>${project.basedir}/src/main/java22</compileSourceRoot>
</compileSourceRoots>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<archive>
<manifestEntries combine.children="append">
<Multi-Release>true</Multi-Release>
</manifestEntries>
</archive>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,69 +16,19 @@
import io.airlift.compress.lz4.Lz4Decompressor;
import io.airlift.compress.zstd.ZstdDecompressor;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.Optional;

public class DecompressionUtils
{
private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
private static final Optional<MethodHandle> LZ4_DECOMPRESSOR_CONSTRUCTOR =
getDecompressorConstructor("io.airlift.compress.v3.lz4.Lz4NativeDecompressor");
private static final Optional<MethodHandle> ZSTD_DECOMPRESSOR_CONSTRUCTOR =
getDecompressorConstructor("io.airlift.compress.v3.zstd.ZstdNativeDecompressor");

private DecompressionUtils() {}

public static int decompressLZ4(byte[] input, byte[] output)
{
if (LZ4_DECOMPRESSOR_CONSTRUCTOR.isEmpty()) {
Lz4Decompressor decompressor = new Lz4Decompressor();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}

return decompressNative(LZ4_DECOMPRESSOR_CONSTRUCTOR.get(), input, output);
Lz4Decompressor decompressor = new Lz4Decompressor();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}

public static int decompressZstd(byte[] input, byte[] output)
{
if (ZSTD_DECOMPRESSOR_CONSTRUCTOR.isEmpty()) {
ZstdDecompressor decompressor = new ZstdDecompressor();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}

return decompressNative(ZSTD_DECOMPRESSOR_CONSTRUCTOR.get(), input, output);
}

private static int decompressNative(MethodHandle constructor, byte[] input, byte[] output)
{
try {
Object decompressor = constructor.invoke();
// int decompress(byte[] input, int inputOffset, int inputLength, byte[] output, int outputOffset, int maxOutputLength)
MethodHandle decompress = LOOKUP.findVirtual(decompressor.getClass(), "decompress", MethodType.methodType(int.class,
byte[].class, int.class, int.class, byte[].class, int.class, int.class));
return (int) decompress.invoke(decompressor, input, 0, input.length, output, 0, output.length);
}
catch (Throwable e) {
throw new RuntimeException(e);
}
}

private static Optional<MethodHandle> getDecompressorConstructor(String clazzName)
{
try {
Class<?> clazz = Class.forName(clazzName);
MethodHandle constructor = LOOKUP.findConstructor(clazz, MethodType.methodType(void.class));
MethodHandle isEnabled = LOOKUP.findStatic(clazz, "isEnabled", MethodType.methodType(boolean.class));
if (!(boolean) isEnabled.invoke()) {
// isEnabled return true only if the native library was loaded properly and all symbols are resolved
return Optional.empty();
}
return Optional.of(constructor);
}
catch (Throwable e) {
return Optional.empty();
}
ZstdDecompressor decompressor = new ZstdDecompressor();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.trino.client.spooling.encoding;

import io.airlift.compress.v3.lz4.Lz4Decompressor;
import io.airlift.compress.v3.zstd.ZstdDecompressor;

public class DecompressionUtils
{
private DecompressionUtils() {}

public static int decompressLZ4(byte[] input, byte[] output)
{
Lz4Decompressor decompressor = Lz4Decompressor.create();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}

public static int decompressZstd(byte[] input, byte[] output)
{
ZstdDecompressor decompressor = ZstdDecompressor.create();
return decompressor.decompress(input, 0, input.length, output, 0, output.length);
}
}
13 changes: 11 additions & 2 deletions client/trino-jdbc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,17 @@
<propertiesEncoding>ISO-8859-1</propertiesEncoding>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<archive>
<manifestEntries combine.children="append">
<Multi-Release>true</Multi-Release>
</manifestEntries>
</archive>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
Expand Down Expand Up @@ -509,7 +519,6 @@
<exclude>META-INF/proguard/**</exclude>
<exclude>LICENSE</exclude>
<exclude>META-INF/**.kotlin_module</exclude>
<exclude>META-INF/versions/**</exclude>
<exclude>META-INF/NOTICE**</exclude>
<exclude>META-INF/*-NOTICE</exclude>
<exclude>META-INF/*-LICENSE</exclude>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ public void testOpenTelemetryIsNotShaded()

public static boolean isExpectedFile(String filename)
{
return MANIFEST_FILES.contains(filename) || filename.startsWith("io/trino/jdbc") || filename.startsWith("aircompressor/");
return MANIFEST_FILES.contains(filename)
|| filename.startsWith("io/trino/jdbc")
|| filename.startsWith("aircompressor/")
|| filename.startsWith("META-INF/versions/"); // Multi-Release JAR classes
}
}
Loading