From c806eac9cb8963efa49d641ee4f696cea003c0e8 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sat, 29 Apr 2017 19:20:45 +0200 Subject: [PATCH 01/10] Fix wrong delegation to constructors when compiling lambdas with method references to ctors. Also remove the get$lambda factory. --- .../painless/LambdaBootstrap.java | 208 +++++++++--------- .../painless/WriterConstants.java | 1 - .../painless/FunctionRefTests.java | 7 + 3 files changed, 105 insertions(+), 111 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index 746467c454bee..d62c3dc30948e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -69,8 +69,7 @@ * 1. member fields for any captured variables * 2. a constructor that will take in captured variables and assign them to * their respective member fields - * 3. if there are captures, a factory method that will take in captured - * variables and delegate them to the constructor + * 3. a static ctor delegation method, if the lambda function is a ctor. * 4. a method that will load the member fields representing captured variables * and take in any other necessary values based on the arguments passed into the * lambda function/reference method; it will then make a delegated call to the @@ -97,10 +96,6 @@ * this.arg$0 = arg$0; * } * - * public static $$Lambda0 get$Lambda(List arg$0) { - * return $$Lambda0(arg$0); - * } - * * public void accept(Object val$0) { * Painless$Script.lambda$0(this.arg$0, val$0); * } @@ -115,13 +110,14 @@ * } * } * - * Note if the above didn't have a captured variable then - * the factory method get$Lambda would not have been generated. * Also the accept method actually uses an invokedynamic * instruction to call the lambda$0 method so that * {@link MethodHandle#asType} can be used to do the necessary * conversions between argument types without having to hard - * code them. + * code them. For method references to a constructor, a static + * wrapper method is created, that creates a class instance and + * calls the constructor. This method is used by the + * invokedynamic call to initialize the instance. * * When the {@link CallSite} is linked the linked method depends * on whether or not there are captures. If there are no captures @@ -160,6 +156,11 @@ private Capture(int count, Class type) { * for each lambda function/reference class. */ private static final AtomicLong COUNTER = new AtomicLong(0); + + /** + * This method name is used to generate a static wrapper method to handle delegation of ctors. + */ + private static final String DELEGATED_CTOR_WRAPPER_NAME = "delegate$ctor"; /** * Generates a lambda class for a lambda function/method reference @@ -193,7 +194,6 @@ public static CallSite lambdaBootstrap( String delegateMethodName, MethodType delegateMethodType) throws LambdaConversionException { - String factoryMethodName = "get$lambda"; String lambdaClassName = lookup.lookupClass().getName().replace('.', '/') + "$$Lambda" + COUNTER.getAndIncrement(); Type lambdaClassType = Type.getType("L" + lambdaClassName + ";"); @@ -203,12 +203,11 @@ public static CallSite lambdaBootstrap( ClassWriter cw = beginLambdaClass(lambdaClassName, factoryMethodType.returnType().getName()); Capture[] captures = generateCaptureFields(cw, factoryMethodType); - Method constructorMethod = - generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures); - - if (captures.length > 0) { - generateFactoryMethod( - cw, factoryMethodName, factoryMethodType, lambdaClassType, constructorMethod); + generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures); + + if (delegateInvokeType == H_NEWINVOKESPECIAL) { + generateStaticCtorDelegator(cw, delegateClassName, delegateInvokeType, + delegateMethodName, delegateMethodType); } generateInterfaceMethod(cw, factoryMethodType, lambdaClassName, lambdaClassType, @@ -220,7 +219,7 @@ public static CallSite lambdaBootstrap( createLambdaClass((Loader)lookup.lookupClass().getClassLoader(), cw, lambdaClassName); if (captures.length > 0) { - return createCaptureCallSite(lookup, factoryMethodName, factoryMethodType, lambdaClass); + return createCaptureCallSite(lookup, factoryMethodType, lambdaClass); } else { return createNoCaptureCallSite(factoryMethodType, lambdaClass); } @@ -244,7 +243,7 @@ private static void validateTypes(MethodType interfaceMethodType, MethodType del * Creates the {@link ClassWriter} to be used for the lambda class generation. */ private static ClassWriter beginLambdaClass(String lambdaClassName, String lambdaInterface) { - String baseClass = Object.class.getName().replace('.', '/'); + String baseClass = Type.getInternalName(Object.class); lambdaInterface = lambdaInterface.replace('.', '/'); int modifiers = ACC_PUBLIC | ACC_STATIC | ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC; @@ -268,7 +267,7 @@ private static Capture[] generateCaptureFields(ClassWriter cw, MethodType factor for (int captureCount = 0; captureCount < captureTotal; ++captureCount) { captures[captureCount] = new Capture(captureCount, factoryMethodType.parameterType(captureCount)); - int modifiers = ACC_PRIVATE + ACC_FINAL; + int modifiers = ACC_PRIVATE | ACC_FINAL; FieldVisitor fv = cw.visitField( modifiers, captures[captureCount].name, captures[captureCount].desc, null, null); @@ -282,11 +281,8 @@ private static Capture[] generateCaptureFields(ClassWriter cw, MethodType factor * Generates a constructor that will take in captured * arguments if any and store them in their respective * member fields. - * @return The constructor {@link Method} used to - * call this method from a potential factory method - * if there are captured arguments */ - private static Method generateLambdaConstructor( + private static void generateLambdaConstructor( ClassWriter cw, Type lambdaClassType, MethodType factoryMethodType, @@ -315,32 +311,26 @@ private static Method generateLambdaConstructor( constructor.returnValue(); constructor.endMethod(); - - return conMeth; } /** - * Generates a factory method that can be used to create the lambda class - * if there are captured variables. + * Generates a factory method to delegate to constructors using + * {@code INVOKEDYNAMIC} using the {@link #delegateBootstrap} type converter. */ - private static void generateFactoryMethod( - ClassWriter cw, - String factoryMethodName, - MethodType factoryMethodType, - Type lambdaClassType, - Method constructorMethod) { - - String facDesc = factoryMethodType.toMethodDescriptorString(); - Method facMeth = new Method(factoryMethodName, facDesc); - int modifiers = ACC_PUBLIC | ACC_STATIC; - - GeneratorAdapter factory = new GeneratorAdapter(modifiers, facMeth, - cw.visitMethod(modifiers, factoryMethodName, facDesc, null, null)); + private static void generateStaticCtorDelegator(ClassWriter cw, String delegateClassName, int delegateInvokeType, String delegateMethodName, + MethodType delegateMethodType) { + Type delegateType = Type.getObjectType(delegateClassName.replace('.', '/')); + Method wrapperMethod = new Method(DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString()); + Method constructorMethod = new Method(delegateMethodName, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString()); + int modifiers = ACC_PRIVATE | ACC_STATIC; + + GeneratorAdapter factory = new GeneratorAdapter(modifiers, wrapperMethod, + cw.visitMethod(modifiers, DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString(), null, null)); factory.visitCode(); - factory.newInstance(lambdaClassType); + factory.newInstance(delegateType); factory.dup(); factory.loadArgs(); - factory.invokeConstructor(lambdaClassType, constructorMethod); + factory.invokeConstructor(delegateType, constructorMethod); factory.returnValue(); factory.endMethod(); } @@ -370,80 +360,76 @@ private static void generateInterfaceMethod( cw.visitMethod(modifiers, interfaceMethodName, lamDesc, null, null)); iface.visitCode(); - // Handles the case where a reference method refers to a constructor. - // A new instance of the requested type will be created and the - // constructor with no parameters will be called. - // Example: String::new + // Handles the case where a reference method refers to a constructor: + // We convert the constructor call to a static method (that is created separately in this class). + // TODO: Investigate why it is not possible to call INVOKEDYNAMIC with a constant pool HANDLE + // pointing to a constructor!?! + // Example: StringBuilder::new if (delegateInvokeType == H_NEWINVOKESPECIAL) { - String conName = ""; - String conDesc = MethodType.methodType(void.class).toMethodDescriptorString(); - Method conMeth = new Method(conName, conDesc); - Type conType = Type.getType(delegateMethodType.returnType()); - - iface.newInstance(conType); - iface.dup(); - iface.invokeConstructor(conType, conMeth); - } else { - // Loads any captured variables onto the stack. - for (int captureCount = 0; captureCount < captures.length; ++captureCount) { - iface.loadThis(); - iface.getField( - lambdaClassType, captures[captureCount].name, captures[captureCount].type); - } - - // Loads any passed in arguments onto the stack. - iface.loadArgs(); + delegateMethodName = DELEGATED_CTOR_WRAPPER_NAME; + delegateClassName = lambdaClassName; + delegateInvokeType = H_INVOKESTATIC; + } + + // Loads any captured variables onto the stack. + for (int captureCount = 0; captureCount < captures.length; ++captureCount) { + iface.loadThis(); + iface.getField( + lambdaClassType, captures[captureCount].name, captures[captureCount].type); + } - // Handles the case for a lambda function or a static reference method. - // interfaceMethodType and delegateMethodType both have the captured types - // inserted into their type signatures. This later allows the delegate + // Loads any passed in arguments onto the stack. + iface.loadArgs(); + + // Handles the case for a lambda function or a static reference method. + // interfaceMethodType and delegateMethodType both have the captured types + // inserted into their type signatures. This later allows the delegate + // method to be invoked dynamically and have the interface method types + // appropriately converted to the delegate method types. + // Example: Integer::parseInt + // Example: something.each(x -> x + 1) + if (delegateInvokeType == H_INVOKESTATIC) { + interfaceMethodType = + interfaceMethodType.insertParameterTypes(0, factoryMethodType.parameterArray()); + delegateMethodType = + delegateMethodType.insertParameterTypes(0, factoryMethodType.parameterArray()); + } else if (delegateInvokeType == H_INVOKEVIRTUAL || + delegateInvokeType == H_INVOKEINTERFACE) { + // Handles the case for a virtual or interface reference method with no captures. + // delegateMethodType drops the 'this' parameter because it will be re-inserted + // when the method handle for the dynamically invoked delegate method is created. + // Example: Object::toString + if (captures.length == 0) { + Class clazz = delegateMethodType.parameterType(0); + delegateClassName = clazz.getName(); + delegateMethodType = delegateMethodType.dropParameterTypes(0, 1); + // Handles the case for a virtual or interface reference method with 'this' + // captured. interfaceMethodType inserts the 'this' type into its + // method signature. This later allows the delegate // method to be invoked dynamically and have the interface method types // appropriately converted to the delegate method types. - // Example: Integer::parseInt - // Example: something.each(x -> x + 1) - if (delegateInvokeType == H_INVOKESTATIC) { - interfaceMethodType = - interfaceMethodType.insertParameterTypes(0, factoryMethodType.parameterArray()); - delegateMethodType = - delegateMethodType.insertParameterTypes(0, factoryMethodType.parameterArray()); - } else if (delegateInvokeType == H_INVOKEVIRTUAL || - delegateInvokeType == H_INVOKEINTERFACE) { - // Handles the case for a virtual or interface reference method with no captures. - // delegateMethodType drops the 'this' parameter because it will be re-inserted - // when the method handle for the dynamically invoked delegate method is created. - // Example: Object::toString - if (captures.length == 0) { - Class clazz = delegateMethodType.parameterType(0); - delegateClassName = clazz.getName(); - delegateMethodType = delegateMethodType.dropParameterTypes(0, 1); - // Handles the case for a virtual or interface reference method with 'this' - // captured. interfaceMethodType inserts the 'this' type into its - // method signature. This later allows the delegate - // method to be invoked dynamically and have the interface method types - // appropriately converted to the delegate method types. - // Example: something::toString - } else if (captures.length == 1) { - Class clazz = factoryMethodType.parameterType(0); - delegateClassName = clazz.getName(); - interfaceMethodType = interfaceMethodType.insertParameterTypes(0, clazz); - } else { - throw new LambdaConversionException( - "unexpected number of captures [ " + captures.length + "]"); - } + // Example: something::toString + } else if (captures.length == 1) { + Class clazz = factoryMethodType.parameterType(0); + delegateClassName = clazz.getName(); + interfaceMethodType = interfaceMethodType.insertParameterTypes(0, clazz); } else { - throw new IllegalStateException( - "unexpected invocation type [" + delegateInvokeType + "]"); + throw new LambdaConversionException( + "unexpected number of captures [ " + captures.length + "]"); } - - Handle delegateHandle = - new Handle(delegateInvokeType, delegateClassName.replace('.', '/'), - delegateMethodName, delegateMethodType.toMethodDescriptorString(), - delegateInvokeType == H_INVOKEINTERFACE); - iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType - .toMethodDescriptorString()).getDescriptor(), DELEGATE_BOOTSTRAP_HANDLE, - delegateHandle); + } else { + throw new IllegalStateException( + "unexpected invocation type [" + delegateInvokeType + "]"); } + Handle delegateHandle = + new Handle(delegateInvokeType, delegateClassName.replace('.', '/'), + delegateMethodName, delegateMethodType.toMethodDescriptorString(), + delegateInvokeType == H_INVOKEINTERFACE); + iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType + .toMethodDescriptorString()).getDescriptor(), DELEGATE_BOOTSTRAP_HANDLE, + delegateHandle); + iface.returnValue(); iface.endMethod(); } @@ -465,6 +451,8 @@ private static Class createLambdaClass( String lambdaClassName) { byte[] classBytes = cw.toByteArray(); + // DEBUG: + // new ClassReader(classBytes).accept(new TraceClassVisitor(new PrintWriter(System.out)), ClassReader.SKIP_DEBUG); return AccessController.doPrivileged((PrivilegedAction>)() -> loader.defineLambda(lambdaClassName.replace('/', '.'), classBytes)); } @@ -501,13 +489,13 @@ private static CallSite createNoCaptureCallSite( */ private static CallSite createCaptureCallSite( Lookup lookup, - String factoryMethodName, MethodType factoryMethodType, Class lambdaClass) { try { return new ConstantCallSite( - lookup.findStatic(lambdaClass, factoryMethodName, factoryMethodType)); + lookup.findConstructor(lambdaClass, factoryMethodType.changeReturnType(void.class)) + .asType(factoryMethodType)); } catch (NoSuchMethodException | IllegalAccessException exception) { throw new IllegalStateException("unable to create lambda factory class", exception); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java index 2772cdcf27521..6b9a71a752b3b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java @@ -27,7 +27,6 @@ import org.objectweb.asm.commons.Method; import java.lang.invoke.CallSite; -import java.lang.invoke.LambdaMetafactory; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java index 4bd687a72058d..7c49d042108ef 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java @@ -91,6 +91,13 @@ public void testCtorMethodReferenceDef() { "return stats.getSum()")); } + public void testCtorWithParams() { + assertArrayEquals(new Object[] { "foo", "bar" }, + (Object[]) exec("List l = new ArrayList(); l.add('foo'); l.add('bar'); " + + "Stream stream = l.stream().map(StringBuilder::new);" + + "return stream.map(Object::toString).toArray()")); + } + public void testArrayCtorMethodRef() { assertEquals(1.0D, exec("List l = new ArrayList(); l.add(1.0); l.add(2.0); " + From 1befcf7bea6adc09134a0bf8eb1957c53e367081 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sat, 29 Apr 2017 20:18:56 +0200 Subject: [PATCH 02/10] Cleanup code and remove unneeded transformations between binary and internal class names (uses ASM Type class instead) --- .../painless/LambdaBootstrap.java | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index d62c3dc30948e..01e7b09e80809 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -174,7 +174,7 @@ private Capture(int count, Class type) { * @param factoryMethodType The type of method to be linked to this CallSite; note that * captured types are based on the parameters for this method * @param interfaceMethodType The type of method representing the functional interface method - * @param delegateClassName The name of the Painless script class + * @param delegateClassName The name of the class to delegate method call to * @param delegateInvokeType The type of method call to be made * (static, virtual, interface, or constructor) * @param delegateMethodName The name of the method to be called in the Painless script class @@ -194,29 +194,29 @@ public static CallSite lambdaBootstrap( String delegateMethodName, MethodType delegateMethodType) throws LambdaConversionException { - String lambdaClassName = lookup.lookupClass().getName().replace('.', '/') + - "$$Lambda" + COUNTER.getAndIncrement(); - Type lambdaClassType = Type.getType("L" + lambdaClassName + ";"); + String lambdaClassName = Type.getInternalName(lookup.lookupClass()) +"$$Lambda" + COUNTER.getAndIncrement(); + Type lambdaClassType = Type.getObjectType(lambdaClassName); + Type delegateClassType = Type.getObjectType(delegateClassName.replace('.', '/')); validateTypes(interfaceMethodType, delegateMethodType); ClassWriter cw = - beginLambdaClass(lambdaClassName, factoryMethodType.returnType().getName()); + beginLambdaClass(lambdaClassName, factoryMethodType.returnType()); Capture[] captures = generateCaptureFields(cw, factoryMethodType); generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures); if (delegateInvokeType == H_NEWINVOKESPECIAL) { - generateStaticCtorDelegator(cw, delegateClassName, delegateInvokeType, + generateStaticCtorDelegator(cw, delegateClassType, delegateInvokeType, delegateMethodName, delegateMethodType); } - generateInterfaceMethod(cw, factoryMethodType, lambdaClassName, lambdaClassType, - interfaceMethodName, interfaceMethodType, delegateClassName, delegateInvokeType, + generateInterfaceMethod(cw, factoryMethodType, lambdaClassType, interfaceMethodName, + interfaceMethodType, delegateClassType, delegateInvokeType, delegateMethodName, delegateMethodType, captures); endLambdaClass(cw); Class lambdaClass = - createLambdaClass((Loader)lookup.lookupClass().getClassLoader(), cw, lambdaClassName); + createLambdaClass((Loader)lookup.lookupClass().getClassLoader(), cw, lambdaClassType); if (captures.length > 0) { return createCaptureCallSite(lookup, factoryMethodType, lambdaClass); @@ -242,14 +242,13 @@ private static void validateTypes(MethodType interfaceMethodType, MethodType del /** * Creates the {@link ClassWriter} to be used for the lambda class generation. */ - private static ClassWriter beginLambdaClass(String lambdaClassName, String lambdaInterface) { + private static ClassWriter beginLambdaClass(String lambdaClassName, Class lambdaInterface) { String baseClass = Type.getInternalName(Object.class); - lambdaInterface = lambdaInterface.replace('.', '/'); int modifiers = ACC_PUBLIC | ACC_STATIC | ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC; ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); cw.visit(CLASS_VERSION, - modifiers, lambdaClassName, null, baseClass, new String[] {lambdaInterface}); + modifiers, lambdaClassName, null, baseClass, new String[] { Type.getInternalName(lambdaInterface) }); return cw; } @@ -317,9 +316,8 @@ private static void generateLambdaConstructor( * Generates a factory method to delegate to constructors using * {@code INVOKEDYNAMIC} using the {@link #delegateBootstrap} type converter. */ - private static void generateStaticCtorDelegator(ClassWriter cw, String delegateClassName, int delegateInvokeType, String delegateMethodName, + private static void generateStaticCtorDelegator(ClassWriter cw, Type delegateClassType, int delegateInvokeType, String delegateMethodName, MethodType delegateMethodType) { - Type delegateType = Type.getObjectType(delegateClassName.replace('.', '/')); Method wrapperMethod = new Method(DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString()); Method constructorMethod = new Method(delegateMethodName, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString()); int modifiers = ACC_PRIVATE | ACC_STATIC; @@ -327,10 +325,10 @@ private static void generateStaticCtorDelegator(ClassWriter cw, String delegateC GeneratorAdapter factory = new GeneratorAdapter(modifiers, wrapperMethod, cw.visitMethod(modifiers, DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString(), null, null)); factory.visitCode(); - factory.newInstance(delegateType); + factory.newInstance(delegateClassType); factory.dup(); factory.loadArgs(); - factory.invokeConstructor(delegateType, constructorMethod); + factory.invokeConstructor(delegateClassType, constructorMethod); factory.returnValue(); factory.endMethod(); } @@ -341,11 +339,10 @@ private static void generateStaticCtorDelegator(ClassWriter cw, String delegateC private static void generateInterfaceMethod( ClassWriter cw, MethodType factoryMethodType, - String lambdaClassName, Type lambdaClassType, String interfaceMethodName, MethodType interfaceMethodType, - String delegateClassName, + Type delegateClassType, int delegateInvokeType, String delegateMethodName, MethodType delegateMethodType, @@ -353,7 +350,7 @@ private static void generateInterfaceMethod( throws LambdaConversionException { String lamDesc = interfaceMethodType.toMethodDescriptorString(); - Method lamMeth = new Method(lambdaClassName, lamDesc); + Method lamMeth = new Method(lambdaClassType.getInternalName(), lamDesc); int modifiers = ACC_PUBLIC; GeneratorAdapter iface = new GeneratorAdapter(modifiers, lamMeth, @@ -367,7 +364,7 @@ private static void generateInterfaceMethod( // Example: StringBuilder::new if (delegateInvokeType == H_NEWINVOKESPECIAL) { delegateMethodName = DELEGATED_CTOR_WRAPPER_NAME; - delegateClassName = lambdaClassName; + delegateClassType = lambdaClassType; delegateInvokeType = H_INVOKESTATIC; } @@ -401,7 +398,7 @@ private static void generateInterfaceMethod( // Example: Object::toString if (captures.length == 0) { Class clazz = delegateMethodType.parameterType(0); - delegateClassName = clazz.getName(); + delegateClassType = Type.getType(clazz); delegateMethodType = delegateMethodType.dropParameterTypes(0, 1); // Handles the case for a virtual or interface reference method with 'this' // captured. interfaceMethodType inserts the 'this' type into its @@ -411,7 +408,7 @@ private static void generateInterfaceMethod( // Example: something::toString } else if (captures.length == 1) { Class clazz = factoryMethodType.parameterType(0); - delegateClassName = clazz.getName(); + delegateClassType = Type.getType(clazz); interfaceMethodType = interfaceMethodType.insertParameterTypes(0, clazz); } else { throw new LambdaConversionException( @@ -423,7 +420,7 @@ private static void generateInterfaceMethod( } Handle delegateHandle = - new Handle(delegateInvokeType, delegateClassName.replace('.', '/'), + new Handle(delegateInvokeType, delegateClassType.getInternalName(), delegateMethodName, delegateMethodType.toMethodDescriptorString(), delegateInvokeType == H_INVOKEINTERFACE); iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType @@ -448,13 +445,13 @@ private static void endLambdaClass(ClassWriter cw) { private static Class createLambdaClass( Loader loader, ClassWriter cw, - String lambdaClassName) { + Type lambdaClassType) { byte[] classBytes = cw.toByteArray(); // DEBUG: // new ClassReader(classBytes).accept(new TraceClassVisitor(new PrintWriter(System.out)), ClassReader.SKIP_DEBUG); return AccessController.doPrivileged((PrivilegedAction>)() -> - loader.defineLambda(lambdaClassName.replace('/', '.'), classBytes)); + loader.defineLambda(lambdaClassType.getClassName(), classBytes)); } /** From 6c850f9c5b6d34d378f1bd5e8180e28d47dc4317 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sat, 29 Apr 2017 20:52:19 +0200 Subject: [PATCH 03/10] Fix whitespace --- .../java/org/elasticsearch/painless/LambdaBootstrap.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index 01e7b09e80809..848979ca273af 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -194,14 +194,13 @@ public static CallSite lambdaBootstrap( String delegateMethodName, MethodType delegateMethodType) throws LambdaConversionException { - String lambdaClassName = Type.getInternalName(lookup.lookupClass()) +"$$Lambda" + COUNTER.getAndIncrement(); + String lambdaClassName = Type.getInternalName(lookup.lookupClass()) + "$$Lambda" + COUNTER.getAndIncrement(); Type lambdaClassType = Type.getObjectType(lambdaClassName); Type delegateClassType = Type.getObjectType(delegateClassName.replace('.', '/')); validateTypes(interfaceMethodType, delegateMethodType); - ClassWriter cw = - beginLambdaClass(lambdaClassName, factoryMethodType.returnType()); + ClassWriter cw = beginLambdaClass(lambdaClassName, factoryMethodType.returnType()); Capture[] captures = generateCaptureFields(cw, factoryMethodType); generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures); From 20d254b5803e17c3a6a8b24e05af57316808def9 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sat, 29 Apr 2017 23:13:34 +0200 Subject: [PATCH 04/10] Cleanup Exception handling --- .../java/org/elasticsearch/painless/LambdaBootstrap.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index 848979ca273af..9171b711bd563 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -33,7 +33,6 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.concurrent.atomic.AtomicLong; @@ -473,9 +472,7 @@ private static CallSite createNoCaptureCallSite( try { return new ConstantCallSite(MethodHandles.constant( factoryMethodType.returnType(), constructor.newInstance())); - } catch (InstantiationException | - IllegalAccessException | - InvocationTargetException exception) { + } catch (ReflectiveOperationException exception) { throw new IllegalStateException("unable to create lambda class", exception); } } @@ -492,7 +489,7 @@ private static CallSite createCaptureCallSite( return new ConstantCallSite( lookup.findConstructor(lambdaClass, factoryMethodType.changeReturnType(void.class)) .asType(factoryMethodType)); - } catch (NoSuchMethodException | IllegalAccessException exception) { + } catch (ReflectiveOperationException exception) { throw new IllegalStateException("unable to create lambda factory class", exception); } } From ab898aa9f3e53bcd7853256621b385ee2d944542 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sat, 29 Apr 2017 23:54:15 +0200 Subject: [PATCH 05/10] Simplification by moving the type adaption to the outside --- .../painless/LambdaBootstrap.java | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index 9171b711bd563..a39039ce6b8bc 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -203,14 +203,19 @@ public static CallSite lambdaBootstrap( Capture[] captures = generateCaptureFields(cw, factoryMethodType); generateLambdaConstructor(cw, lambdaClassType, factoryMethodType, captures); + // Handles the special case where a method reference refers to a ctor (we need a static wrapper method): if (delegateInvokeType == H_NEWINVOKESPECIAL) { - generateStaticCtorDelegator(cw, delegateClassType, delegateInvokeType, - delegateMethodName, delegateMethodType); + generateStaticCtorDelegator(cw, delegateClassType, delegateMethodName, delegateMethodType); + // replace the delegate with our static wrapper: + delegateMethodName = DELEGATED_CTOR_WRAPPER_NAME; + delegateClassType = lambdaClassType; + delegateInvokeType = H_INVOKESTATIC; } generateInterfaceMethod(cw, factoryMethodType, lambdaClassType, interfaceMethodName, interfaceMethodType, delegateClassType, delegateInvokeType, delegateMethodName, delegateMethodType, captures); + endLambdaClass(cw); Class lambdaClass = @@ -314,7 +319,7 @@ private static void generateLambdaConstructor( * Generates a factory method to delegate to constructors using * {@code INVOKEDYNAMIC} using the {@link #delegateBootstrap} type converter. */ - private static void generateStaticCtorDelegator(ClassWriter cw, Type delegateClassType, int delegateInvokeType, String delegateMethodName, + private static void generateStaticCtorDelegator(ClassWriter cw, Type delegateClassType, String delegateMethodName, MethodType delegateMethodType) { Method wrapperMethod = new Method(DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString()); Method constructorMethod = new Method(delegateMethodName, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString()); @@ -354,17 +359,6 @@ private static void generateInterfaceMethod( GeneratorAdapter iface = new GeneratorAdapter(modifiers, lamMeth, cw.visitMethod(modifiers, interfaceMethodName, lamDesc, null, null)); iface.visitCode(); - - // Handles the case where a reference method refers to a constructor: - // We convert the constructor call to a static method (that is created separately in this class). - // TODO: Investigate why it is not possible to call INVOKEDYNAMIC with a constant pool HANDLE - // pointing to a constructor!?! - // Example: StringBuilder::new - if (delegateInvokeType == H_NEWINVOKESPECIAL) { - delegateMethodName = DELEGATED_CTOR_WRAPPER_NAME; - delegateClassType = lambdaClassType; - delegateInvokeType = H_INVOKESTATIC; - } // Loads any captured variables onto the stack. for (int captureCount = 0; captureCount < captures.length; ++captureCount) { From 2b57e9dfc0296a849f2776ad42831a75d579bdcb Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 30 Apr 2017 01:24:08 +0200 Subject: [PATCH 06/10] Remove STATIC access flag from our Lambda class (not required and also officially not allowed) --- .../main/java/org/elasticsearch/painless/LambdaBootstrap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index a39039ce6b8bc..b2f8e8feca8c6 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -247,7 +247,7 @@ private static void validateTypes(MethodType interfaceMethodType, MethodType del */ private static ClassWriter beginLambdaClass(String lambdaClassName, Class lambdaInterface) { String baseClass = Type.getInternalName(Object.class); - int modifiers = ACC_PUBLIC | ACC_STATIC | ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC; + int modifiers = ACC_PUBLIC | ACC_SUPER | ACC_FINAL | ACC_SYNTHETIC; ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); cw.visit(CLASS_VERSION, From b547fa995e762b9bc3327da26c90ab828adfdc52 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 30 Apr 2017 11:15:52 +0200 Subject: [PATCH 07/10] Move the lambda counter to the classloader, so we have a per-script lambda ID --- .../org/elasticsearch/painless/Compiler.java | 11 +++++++ .../painless/LambdaBootstrap.java | 32 ++++--------------- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index 976ef897aec2f..54c5346c49990 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -30,6 +30,7 @@ import java.security.SecureClassLoader; import java.security.cert.Certificate; import java.util.BitSet; +import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.painless.WriterConstants.CLASS_NAME; @@ -66,6 +67,8 @@ final class Compiler { * A secure class loader used to define Painless scripts. */ static final class Loader extends SecureClassLoader { + private final AtomicInteger lambdaCounter = new AtomicInteger(0); + /** * @param parent The parent ClassLoader. */ @@ -92,6 +95,14 @@ Class defineScript(String name, byte[] bytes) { Class defineLambda(String name, byte[] bytes) { return defineClass(name, bytes, 0, bytes.length); } + + /** + * A counter used to generate a unique name for each lambda + * function/reference class in this classloader. + */ + int newLambdaIdentifier() { + return lambdaCounter.getAndIncrement(); + } } /** diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index b2f8e8feca8c6..9d5bc498d20e6 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -32,10 +32,8 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; -import java.lang.reflect.Constructor; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.concurrent.atomic.AtomicLong; import static java.lang.invoke.MethodHandles.Lookup; import static org.elasticsearch.painless.Compiler.Loader; @@ -150,12 +148,6 @@ private Capture(int count, Class type) { } } - /** - * A counter used to generate a unique name - * for each lambda function/reference class. - */ - private static final AtomicLong COUNTER = new AtomicLong(0); - /** * This method name is used to generate a static wrapper method to handle delegation of ctors. */ @@ -193,7 +185,8 @@ public static CallSite lambdaBootstrap( String delegateMethodName, MethodType delegateMethodType) throws LambdaConversionException { - String lambdaClassName = Type.getInternalName(lookup.lookupClass()) + "$$Lambda" + COUNTER.getAndIncrement(); + Loader loader = (Loader)lookup.lookupClass().getClassLoader(); + String lambdaClassName = Type.getInternalName(lookup.lookupClass()) + "$$Lambda" + loader.newLambdaIdentifier(); Type lambdaClassType = Type.getObjectType(lambdaClassName); Type delegateClassType = Type.getObjectType(delegateClassName.replace('.', '/')); @@ -218,9 +211,7 @@ public static CallSite lambdaBootstrap( endLambdaClass(cw); - Class lambdaClass = - createLambdaClass((Loader)lookup.lookupClass().getClassLoader(), cw, lambdaClassType); - + Class lambdaClass = createLambdaClass(loader, cw, lambdaClassType); if (captures.length > 0) { return createCaptureCallSite(lookup, factoryMethodType, lambdaClass); } else { @@ -453,21 +444,12 @@ private static Class createLambdaClass( private static CallSite createNoCaptureCallSite( MethodType factoryMethodType, Class lambdaClass) { - - Constructor constructor = AccessController.doPrivileged( - (PrivilegedAction>)() -> { - try { - return lambdaClass.getConstructor(); - } catch (NoSuchMethodException nsme) { - throw new IllegalStateException("unable to create lambda class", nsme); - } - }); - + try { return new ConstantCallSite(MethodHandles.constant( - factoryMethodType.returnType(), constructor.newInstance())); + factoryMethodType.returnType(), lambdaClass.getConstructor().newInstance())); } catch (ReflectiveOperationException exception) { - throw new IllegalStateException("unable to create lambda class", exception); + throw new IllegalStateException("unable to instantiate lambda class", exception); } } @@ -484,7 +466,7 @@ private static CallSite createCaptureCallSite( lookup.findConstructor(lambdaClass, factoryMethodType.changeReturnType(void.class)) .asType(factoryMethodType)); } catch (ReflectiveOperationException exception) { - throw new IllegalStateException("unable to create lambda factory class", exception); + throw new IllegalStateException("unable to create lambda class", exception); } } From 3a5abd3fc95f368069b660eebdfdb7263fb4974a Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 30 Apr 2017 11:43:16 +0200 Subject: [PATCH 08/10] Change Codesource of generated lambdas to be consistent --- .../src/main/java/org/elasticsearch/painless/Compiler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java index 54c5346c49990..0d92f109eea44 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java @@ -93,7 +93,7 @@ Class defineScript(String name, byte[] bytes) { * @return A Class object. */ Class defineLambda(String name, byte[] bytes) { - return defineClass(name, bytes, 0, bytes.length); + return defineClass(name, bytes, 0, bytes.length, CODESOURCE); } /** From 6adf56e4ea24aff2b1520783102fffc2d717f1e2 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 30 Apr 2017 13:45:56 +0200 Subject: [PATCH 09/10] whitespace cleanup --- .../main/java/org/elasticsearch/painless/LambdaBootstrap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index 9d5bc498d20e6..cd24ff9425445 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -408,7 +408,7 @@ private static void generateInterfaceMethod( delegateInvokeType == H_INVOKEINTERFACE); iface.invokeDynamic(delegateMethodName, Type.getMethodType(interfaceMethodType .toMethodDescriptorString()).getDescriptor(), DELEGATE_BOOTSTRAP_HANDLE, - delegateHandle); + delegateHandle); iface.returnValue(); iface.endMethod(); From 56535b707c726e4b05d54175886a74140b6a197d Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 1 May 2017 14:58:55 -0700 Subject: [PATCH 10/10] Update LambdaBootstrap.java Split up a line in LambdaBootstrap that is over 140 characters long. --- .../main/java/org/elasticsearch/painless/LambdaBootstrap.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java index cd24ff9425445..2ffe9afadf38a 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/LambdaBootstrap.java @@ -313,7 +313,8 @@ private static void generateLambdaConstructor( private static void generateStaticCtorDelegator(ClassWriter cw, Type delegateClassType, String delegateMethodName, MethodType delegateMethodType) { Method wrapperMethod = new Method(DELEGATED_CTOR_WRAPPER_NAME, delegateMethodType.toMethodDescriptorString()); - Method constructorMethod = new Method(delegateMethodName, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString()); + Method constructorMethod = + new Method(delegateMethodName, delegateMethodType.changeReturnType(void.class).toMethodDescriptorString()); int modifiers = ACC_PRIVATE | ACC_STATIC; GeneratorAdapter factory = new GeneratorAdapter(modifiers, wrapperMethod,