Reduce redundant code in procedure declarations

trinodb · Aug 8, 2023 · 3c1cc8e · 3c1cc8e
1 parent fb8eabb
commit 3c1cc8e
Show file tree

Hide file tree

Showing 19 changed files with 85 additions and 201 deletions.
diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/Reflection.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/Reflection.java
@@ -0,0 +1,47 @@
+/*
+ * 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.plugin.base.util;
+
+import io.trino.spi.TrinoException;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+
+import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
+
+/**
+ * @apiNote This mirrors {@code io.trino.util.Reflection}.
+ */
+public final class Reflection
+{
+    private Reflection() {}
+
+    /**
+     * Returns a MethodHandle corresponding to the specified method.
+     * <p>
+     * Warning: The way Oracle JVM implements producing MethodHandle for a method involves creating
+     * JNI global weak references. G1 processes such references serially. As a result, calling this
+     * method in a tight loop can create significant GC pressure and significantly increase
+     * application pause time.
+     */
+    public static MethodHandle methodHandle(Class<?> clazz, String name, Class<?>... parameterTypes)
+    {
+        try {
+            return MethodHandles.lookup().unreflect(clazz.getMethod(name, parameterTypes));
+        }
+        catch (IllegalAccessException | NoSuchMethodException e) {
+            throw new TrinoException(GENERIC_INTERNAL_ERROR, e);
+        }
+    }
+}
diff --git a/...se-jdbc/src/main/java/io/trino/plugin/jdbc/procedure/FlushJdbcMetadataCacheProcedure.java b/...se-jdbc/src/main/java/io/trino/plugin/jdbc/procedure/FlushJdbcMetadataCacheProcedure.java
@@ -23,22 +23,13 @@
 import java.lang.invoke.MethodHandle;
 import java.util.Optional;
 
-import static java.lang.invoke.MethodHandles.lookup;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static java.util.Objects.requireNonNull;
 
 public class FlushJdbcMetadataCacheProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle FLUSH_JDBC_METADATA_CACHE;
-
-    static {
-        try {
-            FLUSH_JDBC_METADATA_CACHE = lookup().unreflect(FlushJdbcMetadataCacheProcedure.class.getMethod("flushMetadataCache"));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle FLUSH_JDBC_METADATA_CACHE = methodHandle(FlushJdbcMetadataCacheProcedure.class, "flushMetadataCache");
 
     private final CachingJdbcClient cachingJdbcClient;
     private final Optional<CachingIdentifierMapping> cachingIdentifierMapping;

diff --git a/...ta-lake/src/main/java/io/trino/plugin/deltalake/procedure/DropExtendedStatsProcedure.java b/...ta-lake/src/main/java/io/trino/plugin/deltalake/procedure/DropExtendedStatsProcedure.java
@@ -30,25 +30,16 @@
 import java.util.List;
 
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.spi.StandardErrorCode.INVALID_PROCEDURE_ARGUMENT;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class DropExtendedStatsProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle PROCEDURE_METHOD;
-
-    static {
-        try {
-            PROCEDURE_METHOD = lookup().unreflect(DropExtendedStatsProcedure.class.getMethod("dropStats", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle PROCEDURE_METHOD = methodHandle(DropExtendedStatsProcedure.class, "dropStats", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class);
 
     private final DeltaLakeMetadataFactory metadataFactory;
     private final ExtendedStatisticsAccess statsAccess;

diff --git a/...a-lake/src/main/java/io/trino/plugin/deltalake/procedure/FlushMetadataCacheProcedure.java b/...a-lake/src/main/java/io/trino/plugin/deltalake/procedure/FlushMetadataCacheProcedure.java
@@ -27,9 +27,9 @@
 import java.lang.invoke.MethodHandle;
 import java.util.Optional;
 
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.spi.StandardErrorCode.INVALID_PROCEDURE_ARGUMENT;
 import static io.trino.spi.type.VarcharType.VARCHAR;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class FlushMetadataCacheProcedure
@@ -40,16 +40,7 @@ public class FlushMetadataCacheProcedure
     private static final String PARAM_SCHEMA_NAME = "SCHEMA_NAME";
     private static final String PARAM_TABLE_NAME = "TABLE_NAME";
 
-    private static final MethodHandle FLUSH_METADATA_CACHE;
-
-    static {
-        try {
-            FLUSH_METADATA_CACHE = lookup().unreflect(FlushMetadataCacheProcedure.class.getMethod("flushMetadataCache", String.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle FLUSH_METADATA_CACHE = methodHandle(FlushMetadataCacheProcedure.class, "flushMetadataCache", String.class, String.class);
 
     private final Optional<CachingHiveMetastore> cachingHiveMetastore;
     private final TransactionLogAccess transactionLogAccess;

diff --git a/...-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/RegisterTableProcedure.java b/...-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/RegisterTableProcedure.java
@@ -40,6 +40,7 @@
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.plugin.deltalake.DeltaLakeErrorCode.DELTA_LAKE_FILESYSTEM_ERROR;
 import static io.trino.plugin.deltalake.DeltaLakeErrorCode.DELTA_LAKE_INVALID_TABLE;
 import static io.trino.plugin.deltalake.DeltaLakeMetadata.buildTable;
@@ -49,13 +50,12 @@
 import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class RegisterTableProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle REGISTER_TABLE;
+    private static final MethodHandle REGISTER_TABLE = methodHandle(RegisterTableProcedure.class, "registerTable", ConnectorSession.class, String.class, String.class, String.class);
 
     private static final String PROCEDURE_NAME = "register_table";
     private static final String SYSTEM_SCHEMA = "system";
@@ -64,15 +64,6 @@ public class RegisterTableProcedure
     private static final String TABLE_NAME = "TABLE_NAME";
     private static final String TABLE_LOCATION = "TABLE_LOCATION";
 
-    static {
-        try {
-            REGISTER_TABLE = lookup().unreflect(RegisterTableProcedure.class.getMethod("registerTable", ConnectorSession.class, String.class, String.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
-
     private final DeltaLakeMetadataFactory metadataFactory;
     private final TransactionLogAccess transactionLogAccess;
     private final CachingExtendedStatisticsAccess statisticsAccess;

diff --git a/...elta-lake/src/main/java/io/trino/plugin/deltalake/procedure/UnregisterTableProcedure.java b/...elta-lake/src/main/java/io/trino/plugin/deltalake/procedure/UnregisterTableProcedure.java
@@ -33,30 +33,21 @@
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.spi.type.VarcharType.VARCHAR;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class UnregisterTableProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle UNREGISTER_TABLE;
+    private static final MethodHandle UNREGISTER_TABLE = methodHandle(UnregisterTableProcedure.class, "unregisterTable", ConnectorAccessControl.class, ConnectorSession.class, String.class, String.class);
 
     private static final String PROCEDURE_NAME = "unregister_table";
     private static final String SYSTEM_SCHEMA = "system";
 
     private static final String SCHEMA_NAME = "SCHEMA_NAME";
     private static final String TABLE_NAME = "TABLE_NAME";
 
-    static {
-        try {
-            UNREGISTER_TABLE = lookup().unreflect(UnregisterTableProcedure.class.getMethod("unregisterTable", ConnectorAccessControl.class, ConnectorSession.class, String.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
-
     private final DeltaLakeMetadataFactory metadataFactory;
     private final TransactionLogAccess transactionLogAccess;
     private final CachingExtendedStatisticsAccess statisticsAccess;

diff --git a/...n/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/VacuumProcedure.java b/...n/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/VacuumProcedure.java
@@ -57,6 +57,7 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.plugin.deltalake.DeltaLakeMetadata.MAX_WRITER_VERSION;
 import static io.trino.plugin.deltalake.DeltaLakeMetadata.checkValidTableHandle;
 import static io.trino.plugin.deltalake.DeltaLakeSessionProperties.getVacuumMinRetention;
@@ -65,7 +66,6 @@
 import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Comparator.naturalOrder;
 import static java.util.Objects.requireNonNull;
 
@@ -75,16 +75,7 @@ public class VacuumProcedure
     private static final Logger log = Logger.get(VacuumProcedure.class);
     private static final int DELETE_BATCH_SIZE = 1000;
 
-    private static final MethodHandle VACUUM;
-
-    static {
-        try {
-            VACUUM = lookup().unreflect(VacuumProcedure.class.getMethod("vacuum", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle VACUUM = methodHandle(VacuumProcedure.class, "vacuum", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, String.class);
 
     private final CatalogName catalogName;
     private final TrinoFileSystemFactory fileSystemFactory;

diff --git a/...e/src/main/java/io/trino/plugin/hive/metastore/procedure/FlushMetadataCacheProcedure.java b/...e/src/main/java/io/trino/plugin/hive/metastore/procedure/FlushMetadataCacheProcedure.java
@@ -29,9 +29,9 @@
 import java.util.Optional;
 
 import static com.google.common.base.Preconditions.checkState;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
 
@@ -69,17 +69,8 @@ public class FlushMetadataCacheProcedure
             PARAM_PARTITION_COLUMN.toLowerCase(ENGLISH),
             PARAM_PARTITION_VALUE.toLowerCase(ENGLISH));
 
-    private static final MethodHandle FLUSH_HIVE_METASTORE_CACHE;
-
-    static {
-        try {
-            FLUSH_HIVE_METASTORE_CACHE = lookup().unreflect(FlushMetadataCacheProcedure.class.getMethod(
-                    "flushMetadataCache", String.class, String.class, List.class, List.class, List.class, List.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle FLUSH_HIVE_METASTORE_CACHE = methodHandle(FlushMetadataCacheProcedure.class,
+            "flushMetadataCache", String.class, String.class, List.class, List.class, List.class, List.class);
 
     private final Optional<CachingHiveMetastore> cachingHiveMetastore;
 

diff --git a/...n/java/io/trino/plugin/hive/metastore/recording/WriteHiveMetastoreRecordingProcedure.java b/...n/java/io/trino/plugin/hive/metastore/recording/WriteHiveMetastoreRecordingProcedure.java
@@ -22,22 +22,13 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandle;
 
-import static java.lang.invoke.MethodHandles.lookup;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static java.util.Objects.requireNonNull;
 
 public class WriteHiveMetastoreRecordingProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle WRITE_HIVE_METASTORE_RECORDING;
-
-    static {
-        try {
-            WRITE_HIVE_METASTORE_RECORDING = lookup().unreflect(WriteHiveMetastoreRecordingProcedure.class.getMethod("writeHiveMetastoreRecording"));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle WRITE_HIVE_METASTORE_RECORDING = methodHandle(WriteHiveMetastoreRecordingProcedure.class, "writeHiveMetastoreRecording");
 
     private final RateLimiter rateLimiter = RateLimiter.create(0.2);
     private final HiveMetastoreRecording hiveMetastoreRecording;

diff --git a/...rino-hive/src/main/java/io/trino/plugin/hive/procedure/CreateEmptyPartitionProcedure.java b/...rino-hive/src/main/java/io/trino/plugin/hive/procedure/CreateEmptyPartitionProcedure.java
@@ -45,28 +45,19 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.plugin.hive.util.HiveUtil.makePartName;
 import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS;
 import static io.trino.spi.StandardErrorCode.INVALID_PROCEDURE_ARGUMENT;
 import static io.trino.spi.connector.RetryMode.NO_RETRIES;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class CreateEmptyPartitionProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle CREATE_EMPTY_PARTITION;
-
-    static {
-        try {
-            CREATE_EMPTY_PARTITION = lookup().unreflect(CreateEmptyPartitionProcedure.class.getMethod("createEmptyPartition", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class, List.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle CREATE_EMPTY_PARTITION = methodHandle(CreateEmptyPartitionProcedure.class, "createEmptyPartition", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class, List.class);
 
     private final TransactionalMetadataFactory hiveMetadataFactory;
     private final LocationService locationService;

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/procedure/DropStatsProcedure.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/procedure/DropStatsProcedure.java
@@ -39,12 +39,12 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.plugin.hive.acid.AcidTransaction.NO_ACID_TRANSACTION;
 import static io.trino.plugin.hive.util.HiveUtil.makePartName;
 import static io.trino.spi.StandardErrorCode.INVALID_PROCEDURE_ARGUMENT;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -55,16 +55,7 @@
 public class DropStatsProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle DROP_STATS;
-
-    static {
-        try {
-            DROP_STATS = lookup().unreflect(DropStatsProcedure.class.getMethod("dropStats", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle DROP_STATS = methodHandle(DropStatsProcedure.class, "dropStats", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class);
 
     private final TransactionalMetadataFactory hiveMetadataFactory;
 

diff --git a/...n/trino-hive/src/main/java/io/trino/plugin/hive/procedure/RegisterPartitionProcedure.java b/...n/trino-hive/src/main/java/io/trino/plugin/hive/procedure/RegisterPartitionProcedure.java
@@ -42,6 +42,7 @@
 import java.util.Optional;
 
 import static io.trino.plugin.base.util.Procedures.checkProcedureArgument;
+import static io.trino.plugin.base.util.Reflection.methodHandle;
 import static io.trino.plugin.hive.HiveMetadata.PRESTO_QUERY_ID_NAME;
 import static io.trino.plugin.hive.procedure.Procedures.checkIsPartitionedTable;
 import static io.trino.plugin.hive.procedure.Procedures.checkPartitionColumns;
@@ -51,22 +52,12 @@
 import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
 import static io.trino.spi.type.VarcharType.VARCHAR;
 import static java.lang.String.format;
-import static java.lang.invoke.MethodHandles.lookup;
 import static java.util.Objects.requireNonNull;
 
 public class RegisterPartitionProcedure
         implements Provider<Procedure>
 {
-    private static final MethodHandle REGISTER_PARTITION;
-
-    static {
-        try {
-            REGISTER_PARTITION = lookup().unreflect(RegisterPartitionProcedure.class.getMethod("registerPartition", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class, List.class, String.class));
-        }
-        catch (ReflectiveOperationException e) {
-            throw new AssertionError(e);
-        }
-    }
+    private static final MethodHandle REGISTER_PARTITION = methodHandle(RegisterPartitionProcedure.class, "registerPartition", ConnectorSession.class, ConnectorAccessControl.class, String.class, String.class, List.class, List.class, String.class);
 
     private final boolean allowRegisterPartition;
     private final TransactionalMetadataFactory hiveMetadataFactory;