8343377: Performance regression in reflective invocation of native methods

src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java

@@ -46,7 +46,7 @@ class DirectMethodHandleAccessor extends MethodAccessorImpl {
      * Creates a MethodAccessorImpl for a non-native method.
      */
     static MethodAccessorImpl methodAccessor(Method method, MethodHandle target) {
-        assert !Modifier.isNative(method.getModifiers());
+        assert !MethodHandleAccessorFactory.isSignaturePolymorphicMethod(method);
 
         return new DirectMethodHandleAccessor(method, target, false);
     }

src/java.base/share/classes/jdk/internal/reflect/MethodHandleAccessorFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
+import java.lang.invoke.VarHandle;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Executable;
 import java.lang.reflect.Field;
@@ -169,7 +170,7 @@ static FieldAccessorImpl newFieldAccessor(Field field, boolean isReadOnly) {
     }
 
     private static MethodHandle getDirectMethod(Method method, boolean callerSensitive) throws IllegalAccessException {
-        var mtype = methodType(method.getReturnType(), method.getParameterTypes());
+        var mtype = methodType(method.getReturnType(), reflectionFactory.getExecutableSharedParameterTypes(method));
         var isStatic = Modifier.isStatic(method.getModifiers());
         var dmh = isStatic ? JLIA.findStatic(method.getDeclaringClass(), method.getName(), mtype)
                                         : JLIA.findVirtual(method.getDeclaringClass(), method.getName(), mtype);
@@ -191,7 +192,7 @@ private static MethodHandle getDirectMethod(Method method, boolean callerSensiti
     private static MethodHandle findCallerSensitiveAdapter(Method method) throws IllegalAccessException {
         String name = method.getName();
         // append a Class parameter
-        MethodType mtype = methodType(method.getReturnType(), method.getParameterTypes())
+        MethodType mtype = methodType(method.getReturnType(), reflectionFactory.getExecutableSharedParameterTypes(method))
                                 .appendParameterTypes(Class.class);
         boolean isStatic = Modifier.isStatic(method.getModifiers());
 
@@ -303,29 +304,43 @@ static void ensureClassInitialized(Class<?> defc) {
 
     /*
      * Returns true if NativeAccessor should be used.
+     *
+     * Native accessor, i.e. VM reflection implementation, is used if one of
+     * the following conditions is met:
+     * 1. during VM early startup before method handle support is fully initialized
+     * 2. -Djdk.reflect.useNativeAccessorOnly=true is set
+     * 3. a signature polymorphic method
+     * 4. the member takes a variable number of arguments and the last parameter
+     *    is not an array (see details below)
+     * 5. the member's method type has an arity >= 255
+     *
+     * Conditions 3-5 are due to the restrictions of method handles.
+     * Otherwise, direct invocation of method handles is used.
      */
     private static boolean useNativeAccessor(Executable member) {
         if (!VM.isJavaLangInvokeInited())
             return true;
 
-        if (Modifier.isNative(member.getModifiers()))
+        if (ReflectionFactory.useNativeAccessorOnly())  // for testing only
             return true;
 
-        if (ReflectionFactory.useNativeAccessorOnly())  // for testing only
+        // java.lang.invoke cannot find the underlying native stubs of signature
+        // polymorphic methods that core reflection must invoke.
+        // Fall back to use the native implementation instead.
+        if (member instanceof Method method && isSignaturePolymorphicMethod(method))
             return true;
 
-        // MethodHandle::withVarargs on a member with varargs modifier bit set
-        // verifies that the last parameter of the member must be an array type.
-        // The JVMS does not require the last parameter descriptor of the method descriptor
-        // is an array type if the ACC_VARARGS flag is set in the access_flags item.
-        // Hence the reflection implementation does not check the last parameter type
-        // if ACC_VARARGS flag is set.  Workaround this by invoking through
-        // the native accessor.
+        // For members with ACC_VARARGS bit set, MethodHandles produced by lookup
+        // always have variable arity set and hence the last parameter of the member
+        // must be an array type.  Such restriction does not exist in core reflection
+        // and the JVM, which always use fixed-arity invocations.  Fall back to use
+        // the native implementation instead.
         int paramCount = member.getParameterCount();
         if (member.isVarArgs() &&
-                (paramCount == 0 || !(member.getParameterTypes()[paramCount-1].isArray()))) {
+                (paramCount == 0 || !(reflectionFactory.getExecutableSharedParameterTypes(member)[paramCount-1].isArray()))) {
             return true;
         }
+
         // A method handle cannot be created if its type has an arity >= 255
         // as the method handle's invoke method consumes an extra argument
         // of the method handle itself. Fall back to use the native implementation.
@@ -345,7 +360,7 @@ private static boolean useNativeAccessor(Executable member) {
      */
     private static int slotCount(Executable member) {
         int slots = 0;
-        Class<?>[] ptypes = member.getParameterTypes();
+        Class<?>[] ptypes = reflectionFactory.getExecutableSharedParameterTypes(member);
         for (Class<?> ptype : ptypes) {
             if (ptype == double.class || ptype == long.class) {
                 slots++;
@@ -355,6 +370,31 @@ private static int slotCount(Executable member) {
                 (Modifier.isStatic(member.getModifiers()) ? 0 : 1);
     }
 
+    /**
+     * Signature-polymorphic methods.  Lookup has special rules for these methods,
+     * but core reflection must observe them as they are declared, and reflective
+     * invocation must invoke the native method stubs that throw UOE.
+     *
+     * @param method the method to check
+     * @return {@code true} if this method is signature polymorphic
+     * @jls 15.12.3 Compile-Time Step 3: Is the Chosen Method Appropriate?
+     * @jvms 2.9.3 Signature Polymorphic Methods
+     */
+    public static boolean isSignaturePolymorphicMethod(Method method) {
+        // ACC_NATIVE and ACC_VARARGS
+        if (!method.isVarArgs() || !Modifier.isNative(method.getModifiers())) {
+            return false;
+        }
+        // Declared in MethodHandle or VarHandle
+        var declaringClass = method.getDeclaringClass();
+        if (declaringClass != MethodHandle.class && declaringClass != VarHandle.class) {
+            return false;
+        }
+        // Single parameter of declared type Object[]
+        Class<?>[] parameters = reflectionFactory.getExecutableSharedParameterTypes(method);
+        return parameters.length == 1 && parameters[0] == Object[].class;
+    }
+
     /*
      * Delay initializing these static fields until java.lang.invoke is fully initialized.
      */
@@ -363,4 +403,5 @@ static class LazyStaticHolder {
     }
 
     private static final Unsafe UNSAFE = Unsafe.getUnsafe();
+    private static final ReflectionFactory reflectionFactory = ReflectionFactory.getReflectionFactory();
 }

test/micro/org/openjdk/bench/java/lang/reflect/NativeMethodInvoke.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package org.openjdk.bench.java.lang.reflect;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Benchmark for regression in native method invocation.
+ */
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Thread)
+@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
+@Fork(3)
+public class NativeMethodInvoke {
+
+    private Method objectHashCode;
+    private Method threadCurrentThread;
+
+    private Object[] objects;
+
+    @Setup
+    public void setup() throws ReflectiveOperationException {
+        objects = new Object[]{
+                1, 5L,
+                5.6d, 23.11f,
+                Boolean.TRUE, 'd'
+        };
+
+        objectHashCode = Object.class.getDeclaredMethod("hashCode");
+        threadCurrentThread = Thread.class.getDeclaredMethod("currentThread");
+    }
+
+    @Benchmark
+    public void objectHashCode(Blackhole bh) throws ReflectiveOperationException {
+        for (var obj : objects) {
+            bh.consume(objectHashCode.invoke(obj));
+        }
+    }
+
+    @Benchmark
+    public Object threadCurrentThread() throws ReflectiveOperationException {
+        return threadCurrentThread.invoke(null);
+    }
+}