Bläddra i källkod

Cleaning up some painless code & removing obsoleted functions (#92103)

* Tidy up some lambda/function factory code
* Remove stray arguments from def argument checking
* We now always have the Java9 array length method handle
* Remove pre-java9 string concat code
Simon Cooper 2 år sedan
förälder
incheckning
5236924dc3

+ 14 - 113
modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java

@@ -46,90 +46,6 @@ import static org.elasticsearch.painless.lookup.PainlessLookupUtility.typeToCano
  */
 public final class Def {
 
-    // TODO: Once Java has a factory for those in java.lang.invoke.MethodHandles, use it:
-
-    /** Helper class for isolating MethodHandles and methods to get the length of arrays
-     * (to emulate a "arraystore" bytecode using MethodHandles).
-     * See: https://bugs.openjdk.java.net/browse/JDK-8156915
-     */
-    @SuppressWarnings("unused") // getArrayLength() methods are are actually used, javac just does not know :)
-    private static final class ArrayLengthHelper {
-        private static final MethodHandles.Lookup PRIVATE_METHOD_HANDLES_LOOKUP = MethodHandles.lookup();
-
-        private static final Map<Class<?>, MethodHandle> ARRAY_TYPE_MH_MAPPING = Collections.unmodifiableMap(
-            Stream.of(
-                boolean[].class,
-                byte[].class,
-                short[].class,
-                int[].class,
-                long[].class,
-                char[].class,
-                float[].class,
-                double[].class,
-                Object[].class
-            ).collect(Collectors.toMap(Function.identity(), type -> {
-                try {
-                    return PRIVATE_METHOD_HANDLES_LOOKUP.findStatic(
-                        PRIVATE_METHOD_HANDLES_LOOKUP.lookupClass(),
-                        "getArrayLength",
-                        MethodType.methodType(int.class, type)
-                    );
-                } catch (ReflectiveOperationException e) {
-                    throw new AssertionError(e);
-                }
-            }))
-        );
-
-        private static final MethodHandle OBJECT_ARRAY_MH = ARRAY_TYPE_MH_MAPPING.get(Object[].class);
-
-        static int getArrayLength(final boolean[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final byte[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final short[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final int[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final long[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final char[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final float[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final double[] array) {
-            return array.length;
-        }
-
-        static int getArrayLength(final Object[] array) {
-            return array.length;
-        }
-
-        static MethodHandle arrayLengthGetter(Class<?> arrayType) {
-            if (arrayType.isArray() == false) {
-                throw new IllegalArgumentException("type must be an array");
-            }
-            return (ARRAY_TYPE_MH_MAPPING.containsKey(arrayType))
-                ? ARRAY_TYPE_MH_MAPPING.get(arrayType)
-                : OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
-        }
-
-        private ArrayLengthHelper() {}
-    }
-
     /** pointer to Map.get(Object) */
     private static final MethodHandle MAP_GET;
     /** pointer to Map.put(Object,Object) */
@@ -144,8 +60,8 @@ public final class Def {
     private static final MethodHandle MAP_INDEX_NORMALIZE;
     /** pointer to {@link Def#listIndexNormalize}. */
     private static final MethodHandle LIST_INDEX_NORMALIZE;
-    /** factory for arraylength MethodHandle (intrinsic) from Java 9 (pkg-private for tests) */
-    static final MethodHandle JAVA9_ARRAY_LENGTH_MH_FACTORY;
+    /** factory for arraylength MethodHandle (intrinsic) */
+    private static final MethodHandle ARRAY_LENGTH;
 
     public static final Map<Class<?>, MethodHandle> DEF_TO_BOXED_TYPE_IMPLICIT_CAST;
 
@@ -168,23 +84,14 @@ public final class Def {
                 "listIndexNormalize",
                 MethodType.methodType(int.class, List.class, int.class)
             );
-        } catch (final ReflectiveOperationException roe) {
-            throw new AssertionError(roe);
-        }
-
-        // lookup up the factory for arraylength MethodHandle (intrinsic) from Java 9:
-        // https://bugs.openjdk.java.net/browse/JDK-8156915
-        MethodHandle arrayLengthMHFactory;
-        try {
-            arrayLengthMHFactory = methodHandlesLookup.findStatic(
+            ARRAY_LENGTH = methodHandlesLookup.findStatic(
                 MethodHandles.class,
                 "arrayLength",
                 MethodType.methodType(MethodHandle.class, Class.class)
             );
-        } catch (final ReflectiveOperationException roe) {
-            arrayLengthMHFactory = null;
+        } catch (ReflectiveOperationException roe) {
+            throw new AssertionError(roe);
         }
-        JAVA9_ARRAY_LENGTH_MH_FACTORY = arrayLengthMHFactory;
 
         Map<Class<?>, MethodHandle> defToBoxedTypeImplicitCast = new HashMap<>();
 
@@ -232,15 +139,11 @@ public final class Def {
 
     /** Returns an array length getter MethodHandle for the given array type */
     static MethodHandle arrayLengthGetter(Class<?> arrayType) {
-        if (JAVA9_ARRAY_LENGTH_MH_FACTORY != null) {
-            try {
-                return (MethodHandle) JAVA9_ARRAY_LENGTH_MH_FACTORY.invokeExact(arrayType);
-            } catch (Throwable t) {
-                rethrow(t);
-                throw new AssertionError(t);
-            }
-        } else {
-            return ArrayLengthHelper.arrayLengthGetter(arrayType);
+        try {
+            return (MethodHandle) ARRAY_LENGTH.invokeExact(arrayType);
+        } catch (Throwable t) {
+            rethrow(t);
+            throw new AssertionError(t);
         }
     }
 
@@ -840,9 +743,8 @@ public final class Def {
             if (arrayType.isArray() == false) {
                 throw new IllegalArgumentException("type must be an array");
             }
-            return (ARRAY_TYPE_MH_MAPPING.containsKey(arrayType))
-                ? ARRAY_TYPE_MH_MAPPING.get(arrayType)
-                : OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
+            MethodHandle iterator = ARRAY_TYPE_MH_MAPPING.get(arrayType);
+            return iterator != null ? iterator : OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
         }
 
         private ArrayIteratorHelper() {}
@@ -1616,9 +1518,8 @@ public final class Def {
             if (arrayType.isArray() == false) {
                 throw new IllegalArgumentException("type must be an array");
             }
-            return (ARRAY_TYPE_MH_MAPPING.containsKey(arrayType))
-                ? ARRAY_TYPE_MH_MAPPING.get(arrayType)
-                : OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
+            MethodHandle handle = ARRAY_TYPE_MH_MAPPING.get(arrayType);
+            return handle != null ? handle : OBJECT_ARRAY_MH.asType(OBJECT_ARRAY_MH.type().changeParameterType(0, arrayType));
         }
 
         private ArrayIndexNormalizeHelper() {}

+ 5 - 5
modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java

@@ -382,13 +382,13 @@ public final class DefBootstrap {
                 } else if (type.parameterType(0) != Object.class) {
                     // case 2: only the argument is unknown, just check that
                     MethodType testType = MethodType.methodType(boolean.class, type);
-                    MethodHandle unaryTest = CHECK_RHS.bindTo(clazz0).bindTo(clazz1);
-                    test = unaryTest.asType(testType);
+                    MethodHandle unaryTest = CHECK_RHS.bindTo(clazz1);
+                    test = MethodHandles.dropArguments(unaryTest, 0, Object.class).asType(testType);
                     nullCheck = MethodHandles.dropArguments(NON_NULL, 0, clazz0).asType(testType);
                 } else {
                     // case 3: check both receiver and argument
                     MethodType testType = MethodType.methodType(boolean.class, type);
-                    MethodHandle binaryTest = CHECK_BOTH.bindTo(clazz0).bindTo(clazz1);
+                    MethodHandle binaryTest = MethodHandles.insertArguments(CHECK_BOTH, 0, clazz0, clazz1);
                     test = binaryTest.asType(testType);
                     nullCheck = BOTH_NON_NULL.asType(testType);
                 }
@@ -423,7 +423,7 @@ public final class DefBootstrap {
          * guard method for inline caching: checks the first argument is the same
          * as the cached first argument.
          */
-        static boolean checkRHS(Class<?> left, Class<?> right, Object leftObject, Object rightObject) {
+        static boolean checkRHS(Class<?> right, Object rightObject) {
             return rightObject.getClass() == right;
         }
 
@@ -460,7 +460,7 @@ public final class DefBootstrap {
                 CHECK_RHS = methodHandlesLookup.findStatic(
                     methodHandlesLookup.lookupClass(),
                     "checkRHS",
-                    MethodType.methodType(boolean.class, Class.class, Class.class, Object.class, Object.class)
+                    MethodType.methodType(boolean.class, Class.class, Object.class)
                 );
                 CHECK_BOTH = methodHandlesLookup.findStatic(
                     methodHandlesLookup.lookupClass(),

+ 48 - 67
modules/lang-painless/src/main/java/org/elasticsearch/painless/FunctionRef.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.painless;
 
+import org.elasticsearch.core.Strings;
 import org.elasticsearch.painless.lookup.PainlessConstructor;
 import org.elasticsearch.painless.lookup.PainlessLookup;
 import org.elasticsearch.painless.lookup.PainlessLookupUtility;
@@ -22,7 +23,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.elasticsearch.painless.WriterConstants.CLASS_NAME;
 import static org.objectweb.asm.Opcodes.H_INVOKEINTERFACE;
@@ -73,14 +74,12 @@ public class FunctionRef {
 
             if (interfaceMethod == null) {
                 throw new IllegalArgumentException(
-                    "cannot convert function reference ["
-                        + typeName
-                        + "::"
-                        + methodName
-                        + "] "
-                        + "to a non-functional interface ["
-                        + targetClassName
-                        + "]"
+                    Strings.format(
+                        "cannot convert function reference [%s::%s] to a non-functional interface [%s]",
+                        typeName,
+                        methodName,
+                        targetClassName
+                    )
                 );
             }
 
@@ -110,18 +109,14 @@ public class FunctionRef {
 
                 if (localFunction == null) {
                     throw new IllegalArgumentException(
-                        "function reference [this::"
-                            + localFunctionKey
-                            + "] "
-                            + "matching ["
-                            + targetClassName
-                            + ", "
-                            + interfaceMethodName
-                            + "/"
-                            + interfaceTypeParametersSize
-                            + "] "
-                            + "not found"
-                            + (localFunctionKey.contains("$") ? " due to an incorrect number of arguments" : "")
+                        Strings.format(
+                            "function reference [this::%s] matching [%s, %s/%d] not found%s",
+                            localFunctionKey,
+                            targetClassName,
+                            interfaceMethodName,
+                            interfaceTypeParametersSize,
+                            localFunctionKey.contains("$") ? " due to an incorrect number of arguments" : ""
+                        )
                     );
                 }
 
@@ -144,19 +139,14 @@ public class FunctionRef {
 
                 if (painlessConstructor == null) {
                     throw new IllegalArgumentException(
-                        "function reference ["
-                            + typeName
-                            + "::new/"
-                            + interfaceTypeParametersSize
-                            + "] "
-                            + "matching ["
-                            + targetClassName
-                            + ", "
-                            + interfaceMethodName
-                            + "/"
-                            + interfaceTypeParametersSize
-                            + "] "
-                            + "not found"
+                        Strings.format(
+                            "function reference [%s::new/%d] matching [%s, %s/%d] not found",
+                            typeName,
+                            interfaceTypeParametersSize,
+                            targetClassName,
+                            interfaceMethodName,
+                            interfaceTypeParametersSize
+                        )
                     );
                 }
 
@@ -193,35 +183,25 @@ public class FunctionRef {
 
                     if (painlessMethod == null) {
                         throw new IllegalArgumentException(
-                            "function reference "
-                                + "["
-                                + typeName
-                                + "::"
-                                + methodName
-                                + "/"
-                                + interfaceTypeParametersSize
-                                + "] "
-                                + "matching ["
-                                + targetClassName
-                                + ", "
-                                + interfaceMethodName
-                                + "/"
-                                + interfaceTypeParametersSize
-                                + "] "
-                                + "not found"
+                            Strings.format(
+                                "function reference [%s::%s/%d] matching [%s, %s/%d] not found",
+                                typeName,
+                                methodName,
+                                interfaceTypeParametersSize,
+                                targetClassName,
+                                interfaceMethodName,
+                                interfaceTypeParametersSize
+                            )
                         );
                     }
                 } else if (captured) {
                     throw new IllegalArgumentException(
-                        "cannot use a static method as a function reference "
-                            + "["
-                            + typeName
-                            + "::"
-                            + methodName
-                            + "/"
-                            + interfaceTypeParametersSize
-                            + "] "
-                            + "with a non-static captured variable"
+                        Strings.format(
+                            "cannot use a static method as a function reference [%s::%s/%d] with a non-static captured variable",
+                            typeName,
+                            methodName,
+                            interfaceTypeParametersSize
+                        )
                     );
                 }
 
@@ -360,19 +340,20 @@ public class FunctionRef {
         if (factoryMethodReceiver == null) {
             return factoryMethodType.toMethodDescriptorString();
         }
-        List<Type> arguments = factoryMethodType.parameterList().stream().map(Type::getType).collect(Collectors.toList());
-        arguments.add(0, factoryMethodReceiver);
-        Type[] argArray = new Type[arguments.size()];
-        arguments.toArray(argArray);
-        return Type.getMethodDescriptor(Type.getType(factoryMethodType.returnType()), argArray);
+        Type[] arguments = Stream.concat(Stream.of(factoryMethodReceiver), factoryMethodType.parameterList().stream().map(Type::getType))
+            .toArray(Type[]::new);
+        return Type.getMethodDescriptor(Type.getType(factoryMethodType.returnType()), arguments);
     }
 
     /** Get the factory method type, updating the receiver if {@code factoryMethodReceiverClass} is non-null */
     public Class<?>[] factoryMethodParameters(Class<?> factoryMethodReceiverClass) {
-        List<Class<?>> parameters = new ArrayList<>(factoryMethodType.parameterList());
+        Class<?>[] parameters = factoryMethodType.parameterList().toArray(Class<?>[]::new);
         if (factoryMethodReceiverClass != null) {
-            parameters.add(0, factoryMethodReceiverClass);
+            Class<?>[] withReceiver = new Class<?>[parameters.length + 1];
+            withReceiver[0] = factoryMethodReceiverClass;
+            System.arraycopy(parameters, 0, withReceiver, 1, parameters.length);
+            parameters = withReceiver;
         }
-        return parameters.toArray(new Class<?>[0]);
+        return parameters;
     }
 }

+ 10 - 8
modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java

@@ -24,7 +24,6 @@ import java.lang.invoke.MethodType;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import static java.lang.invoke.MethodHandles.Lookup;
 import static org.elasticsearch.painless.WriterConstants.CLASS_VERSION;
@@ -400,9 +399,9 @@ public final class LambdaBootstrap {
         iface.visitCode();
 
         // Loads any captured variables onto the stack.
-        for (int captureCount = 0; captureCount < captures.length; ++captureCount) {
+        for (Capture capture : captures) {
             iface.loadThis();
-            iface.getField(lambdaClassType, captures[captureCount].name, captures[captureCount].type);
+            iface.getField(lambdaClassType, capture.name, capture.type);
         }
 
         // Loads any passed in arguments onto the stack.
@@ -442,13 +441,16 @@ public final class LambdaBootstrap {
                 delegateClassType = Type.getType(clazz);
 
                 // functionalInterfaceWithCaptures needs to add the receiver and other captures
-                List<Type> parameters = interfaceMethodType.parameterList().stream().map(Type::getType).collect(Collectors.toList());
-                parameters.add(0, delegateClassType);
+                Type[] parameters = new Type[captures.length + interfaceMethodType.parameterList().size()];
+                int p = 0;
+                parameters[p++] = delegateClassType;
                 for (int i = 1; i < captures.length; i++) {
-                    parameters.add(i, captures[i].type);
+                    parameters[p++] = captures[i].type;
+                }
+                for (Class<?> pCls : interfaceMethodType.parameterList()) {
+                    parameters[p++] = Type.getType(pCls);
                 }
-                Type[] parametersArray = parameters.toArray(new Type[0]);
-                functionalInterfaceWithCaptures = Type.getMethodDescriptor(Type.getType(interfaceMethodType.returnType()), parametersArray);
+                functionalInterfaceWithCaptures = Type.getMethodDescriptor(Type.getType(interfaceMethodType.returnType()), parameters);
 
                 // delegateMethod does not need the receiver
                 List<Class<?>> factoryParameters = factoryMethodType.parameterList();

+ 18 - 58
modules/lang-painless/src/main/java/org/elasticsearch/painless/MethodWriter.java

@@ -61,21 +61,10 @@ import static org.elasticsearch.painless.WriterConstants.DEF_TO_P_SHORT_IMPLICIT
 import static org.elasticsearch.painless.WriterConstants.DEF_TO_STRING_EXPLICIT;
 import static org.elasticsearch.painless.WriterConstants.DEF_TO_STRING_IMPLICIT;
 import static org.elasticsearch.painless.WriterConstants.DEF_UTIL_TYPE;
-import static org.elasticsearch.painless.WriterConstants.INDY_STRING_CONCAT_BOOTSTRAP_HANDLE;
 import static org.elasticsearch.painless.WriterConstants.LAMBDA_BOOTSTRAP_HANDLE;
-import static org.elasticsearch.painless.WriterConstants.MAX_INDY_STRING_CONCAT_ARGS;
+import static org.elasticsearch.painless.WriterConstants.MAX_STRING_CONCAT_ARGS;
 import static org.elasticsearch.painless.WriterConstants.PAINLESS_ERROR_TYPE;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_BOOLEAN;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_CHAR;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_DOUBLE;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_FLOAT;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_INT;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_LONG;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_OBJECT;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_APPEND_STRING;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_CONSTRUCTOR;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_TOSTRING;
-import static org.elasticsearch.painless.WriterConstants.STRINGBUILDER_TYPE;
+import static org.elasticsearch.painless.WriterConstants.STRING_CONCAT_BOOTSTRAP_HANDLE;
 import static org.elasticsearch.painless.WriterConstants.STRING_TO_CHAR;
 import static org.elasticsearch.painless.WriterConstants.STRING_TYPE;
 import static org.elasticsearch.painless.WriterConstants.UTILITY_TYPE;
@@ -90,7 +79,7 @@ public final class MethodWriter extends GeneratorAdapter {
     private final BitSet statements;
     private final CompilerSettings settings;
 
-    private final Deque<List<Type>> stringConcatArgs = (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE == null) ? null : new ArrayDeque<>();
+    private final Deque<List<Type>> stringConcatArgs = new ArrayDeque<>();
 
     public MethodWriter(int access, Method method, ClassVisitor cw, BitSet statements, CompilerSettings settings) {
         super(
@@ -266,57 +255,28 @@ public final class MethodWriter extends GeneratorAdapter {
     /** Starts a new string concat.
      * @return the size of arguments pushed to stack (the object that does string concats, e.g. a StringBuilder)
      */
-    public int writeNewStrings() {
-        if (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE != null) {
-            // Java 9+: we just push our argument collector onto deque
-            stringConcatArgs.push(new ArrayList<>());
-            return 0; // nothing added to stack
-        } else {
-            // Java 8: create a StringBuilder in bytecode
-            newInstance(STRINGBUILDER_TYPE);
-            dup();
-            invokeConstructor(STRINGBUILDER_TYPE, STRINGBUILDER_CONSTRUCTOR);
-            return 1; // StringBuilder on stack
-        }
+    public List<Type> writeNewStrings() {
+        List<Type> list = new ArrayList<>();
+        stringConcatArgs.push(list);
+        return list;
     }
 
     public void writeAppendStrings(Class<?> clazz) {
-        if (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE != null) {
-            // Java 9+: record type information
-            stringConcatArgs.peek().add(getType(clazz));
-            // prevent too many concat args.
-            // If there are too many, do the actual concat:
-            if (stringConcatArgs.peek().size() >= MAX_INDY_STRING_CONCAT_ARGS) {
-                writeToStrings();
-                writeNewStrings();
-                // add the return value type as new first param for next concat:
-                stringConcatArgs.peek().add(STRING_TYPE);
-            }
-        } else {
-            // Java 8: push a StringBuilder append
-            if (clazz == boolean.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_BOOLEAN);
-            else if (clazz == char.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_CHAR);
-            else if (clazz == byte.class || clazz == short.class || clazz == int.class) invokeVirtual(
-                STRINGBUILDER_TYPE,
-                STRINGBUILDER_APPEND_INT
-            );
-            else if (clazz == long.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_LONG);
-            else if (clazz == float.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_FLOAT);
-            else if (clazz == double.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_DOUBLE);
-            else if (clazz == String.class) invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_STRING);
-            else invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_APPEND_OBJECT);
+        List<Type> currentConcat = stringConcatArgs.peek();
+        currentConcat.add(getType(clazz));
+        // prevent too many concat args.
+        // If there are too many, do the actual concat:
+        if (currentConcat.size() >= MAX_STRING_CONCAT_ARGS) {
+            writeToStrings();
+            currentConcat = writeNewStrings();
+            // add the return value type as new first param for next concat:
+            currentConcat.add(STRING_TYPE);
         }
     }
 
     public void writeToStrings() {
-        if (INDY_STRING_CONCAT_BOOTSTRAP_HANDLE != null) {
-            // Java 9+: use type information and push invokeDynamic
-            final String desc = Type.getMethodDescriptor(STRING_TYPE, stringConcatArgs.pop().stream().toArray(Type[]::new));
-            invokeDynamic("concat", desc, INDY_STRING_CONCAT_BOOTSTRAP_HANDLE);
-        } else {
-            // Java 8: call toString() on StringBuilder
-            invokeVirtual(STRINGBUILDER_TYPE, STRINGBUILDER_TOSTRING);
-        }
+        final String desc = Type.getMethodDescriptor(STRING_TYPE, stringConcatArgs.pop().toArray(Type[]::new));
+        invokeDynamic("concat", desc, STRING_CONCAT_BOOTSTRAP_HANDLE);
     }
 
     /** Writes a dynamic binary instruction: returnType, lhs, and rhs can be different */

+ 15 - 38
modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java

@@ -17,10 +17,9 @@ import java.lang.invoke.CallSite;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
-import java.util.ArrayList;
+import java.lang.invoke.StringConcatFactory;
 import java.util.Collection;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -163,39 +162,23 @@ public final class WriterConstants {
         false
     );
 
-    /** dynamic invokedynamic bootstrap for indy string concats (Java 9+) */
-    public static final Handle INDY_STRING_CONCAT_BOOTSTRAP_HANDLE;
-    static {
-        Handle bs;
-        try {
-            final Class<?> factory = Class.forName("java.lang.invoke.StringConcatFactory");
-            final String methodName = "makeConcat";
-            final MethodType type = MethodType.methodType(CallSite.class, MethodHandles.Lookup.class, String.class, MethodType.class);
-            // ensure it is there:
-            MethodHandles.publicLookup().findStatic(factory, methodName, type);
-            bs = new Handle(Opcodes.H_INVOKESTATIC, Type.getInternalName(factory), methodName, type.toMethodDescriptorString(), false);
-        } catch (ReflectiveOperationException e) {
-            // not Java 9 - we set it null, so MethodWriter uses StringBuilder:
-            bs = null;
-        }
-        INDY_STRING_CONCAT_BOOTSTRAP_HANDLE = bs;
-    }
+    public static final MethodType MAKE_CONCAT_TYPE = MethodType.methodType(
+        CallSite.class,
+        MethodHandles.Lookup.class,
+        String.class,
+        MethodType.class
+    );
+    public static final Handle STRING_CONCAT_BOOTSTRAP_HANDLE = new Handle(
+        Opcodes.H_INVOKESTATIC,
+        Type.getInternalName(StringConcatFactory.class),
+        "makeConcat",
+        MAKE_CONCAT_TYPE.toMethodDescriptorString(),
+        false
+    );
 
-    public static final int MAX_INDY_STRING_CONCAT_ARGS = 200;
+    public static final int MAX_STRING_CONCAT_ARGS = 200;
 
     public static final Type STRING_TYPE = Type.getType(String.class);
-    public static final Type STRINGBUILDER_TYPE = Type.getType(StringBuilder.class);
-
-    public static final Method STRINGBUILDER_CONSTRUCTOR = getAsmMethod(void.class, CTOR_METHOD_NAME);
-    public static final Method STRINGBUILDER_APPEND_BOOLEAN = getAsmMethod(StringBuilder.class, "append", boolean.class);
-    public static final Method STRINGBUILDER_APPEND_CHAR = getAsmMethod(StringBuilder.class, "append", char.class);
-    public static final Method STRINGBUILDER_APPEND_INT = getAsmMethod(StringBuilder.class, "append", int.class);
-    public static final Method STRINGBUILDER_APPEND_LONG = getAsmMethod(StringBuilder.class, "append", long.class);
-    public static final Method STRINGBUILDER_APPEND_FLOAT = getAsmMethod(StringBuilder.class, "append", float.class);
-    public static final Method STRINGBUILDER_APPEND_DOUBLE = getAsmMethod(StringBuilder.class, "append", double.class);
-    public static final Method STRINGBUILDER_APPEND_STRING = getAsmMethod(StringBuilder.class, "append", String.class);
-    public static final Method STRINGBUILDER_APPEND_OBJECT = getAsmMethod(StringBuilder.class, "append", Object.class);
-    public static final Method STRINGBUILDER_TOSTRING = getAsmMethod(String.class, "toString");
 
     public static final Type OBJECTS_TYPE = Type.getType(Objects.class);
     public static final Method EQUALS = getAsmMethod(boolean.class, "equals", Object.class, Object.class);
@@ -203,12 +186,6 @@ public final class WriterConstants {
     public static final Type COLLECTION_TYPE = Type.getType(Collection.class);
     public static final Method COLLECTION_SIZE = getAsmMethod(int.class, "size");
 
-    public static final Type LIST_TYPE = Type.getType(List.class);
-    public static final Method LIST_ADD = getAsmMethod(boolean.class, "add", Object.class);
-
-    public static final Type ARRAY_LIST_TYPE = Type.getType(ArrayList.class);
-    public static final Method ARRAY_LIST_CTOR_WITH_SIZE = getAsmMethod(void.class, CTOR_METHOD_NAME, int.class);
-
     private static Method getAsmMethod(final Class<?> rtype, final String name, final Class<?>... ptypes) {
         return new Method(name, MethodType.methodType(rtype, ptypes).toMethodDescriptorString());
     }

+ 1 - 1
modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultUserTreeToIRTreePhase.java

@@ -905,7 +905,7 @@ public class DefaultUserTreeToIRTreePhase implements UserTreeVisitor<ScriptScope
                 irCompoundNode = stringConcatenationNode;
 
                 // must handle the StringBuilder case for java version <= 8
-                if (irLoadNode instanceof BinaryImplNode bin && WriterConstants.INDY_STRING_CONCAT_BOOTSTRAP_HANDLE == null) {
+                if (irLoadNode instanceof BinaryImplNode bin && WriterConstants.STRING_CONCAT_BOOTSTRAP_HANDLE == null) {
                     bin.getLeftNode().attachDecoration(new IRDDepth(1));
                 }
                 // handles when the operation is mathematical

+ 0 - 2
modules/lang-painless/src/test/java/org/elasticsearch/painless/ArrayTests.java

@@ -8,7 +8,6 @@
 
 package org.elasticsearch.painless;
 
-import org.apache.lucene.util.Constants;
 import org.hamcrest.Matcher;
 
 import java.lang.invoke.MethodHandle;
@@ -34,7 +33,6 @@ public class ArrayTests extends ArrayLikeObjectTestCase {
     }
 
     public void testArrayLengthHelper() throws Throwable {
-        assertEquals(Constants.JRE_IS_MINIMUM_JAVA9, Def.JAVA9_ARRAY_LENGTH_MH_FACTORY != null);
         assertArrayLength(2, new int[2]);
         assertArrayLength(3, new long[3]);
         assertArrayLength(4, new byte[4]);

+ 4 - 16
modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java

@@ -8,13 +8,11 @@
 
 package org.elasticsearch.painless;
 
-import org.apache.lucene.util.Constants;
-
 import java.util.HashMap;
 import java.util.Map;
 
 import static java.util.Collections.singletonMap;
-import static org.elasticsearch.painless.WriterConstants.MAX_INDY_STRING_CONCAT_ARGS;
+import static org.elasticsearch.painless.WriterConstants.MAX_STRING_CONCAT_ARGS;
 
 public class StringTests extends ScriptTestCase {
 
@@ -65,7 +63,7 @@ public class StringTests extends ScriptTestCase {
     }
 
     public void testAppendMany() {
-        for (int i = MAX_INDY_STRING_CONCAT_ARGS - 5; i < MAX_INDY_STRING_CONCAT_ARGS + 5; i++) {
+        for (int i = MAX_STRING_CONCAT_ARGS - 5; i < MAX_STRING_CONCAT_ARGS + 5; i++) {
             doTestAppendMany(i);
         }
     }
@@ -234,18 +232,14 @@ public class StringTests extends ScriptTestCase {
         assertEquals(rando, exec("params.rando.encodeBase64().decodeBase64()", singletonMap("rando", rando), true));
     }
 
-    public void testJava9ConstantStringConcatBytecode() {
-        assumeTrue("Needs Java 9 to test indified String concat", Constants.JRE_IS_MINIMUM_JAVA9);
-        assertNotNull(WriterConstants.INDY_STRING_CONCAT_BOOTSTRAP_HANDLE);
+    public void testConstantStringConcatBytecode() {
         assertBytecodeExists(
             "String s = \"cat\"; return s + true + 'abc' + null;",
             "INVOKEDYNAMIC concat(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"
         );
     }
 
-    public void testJava9StringConcatBytecode() {
-        assumeTrue("Needs Java 9 to test indified String concat", Constants.JRE_IS_MINIMUM_JAVA9);
-        assertNotNull(WriterConstants.INDY_STRING_CONCAT_BOOTSTRAP_HANDLE);
+    public void testStringConcatBytecode() {
         assertBytecodeExists(
             "String s = \"cat\"; boolean t = true; Object u = null; return s + t + 'abc' + u;",
             "INVOKEDYNAMIC concat(Ljava/lang/String;ZLjava/lang/String;Ljava/lang/Object;)Ljava/lang/String;"
@@ -260,10 +254,4 @@ public class StringTests extends ScriptTestCase {
         assertEquals("" + 2 + null, exec("2 + '' + null"));
         assertEquals("" + null + 2, exec("null + '' + 2"));
     }
-
-    public void testJava9NullStringConcatBytecode() {
-        assumeTrue("Needs Java 9 to test indified String concat", Constants.JRE_IS_MINIMUM_JAVA9);
-        assertNotNull(WriterConstants.INDY_STRING_CONCAT_BOOTSTRAP_HANDLE);
-        assertEquals("" + null + null, exec("'' + null + null"));
-    }
 }