changeset 5892:c2506fb31e41

Revamp of SAM method and bridge analysis, for efficiency, correctness, clarity and to prepare for VM fail-over
author Robert Field <Robert.Field@oracle.com>
date Sun, 02 Sep 2012 09:52:16 -0700
parents 6b67e68ac62e
children a3394c993428
files src/share/classes/java/lang/invoke/InnerClassGenerator.java src/share/classes/java/lang/invoke/LambdaMetafactory.java
diffstat 2 files changed, 72 insertions(+), 98 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/java/lang/invoke/InnerClassGenerator.java	Wed Aug 22 17:38:46 2012 -0700
+++ b/src/share/classes/java/lang/invoke/InnerClassGenerator.java	Sun Sep 02 09:52:16 2012 -0700
@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import java.security.ProtectionDomain;
 import java.util.ArrayList;
 import java.util.List;
@@ -61,8 +62,10 @@
     private final Class<?> targetClass;
     private final int implKind;
     private final Class<?> implMethodClass;
+    private final String implMethodClassName;
     private final String implMethodName;
     private final String implMethodDesc;
+    private final Type implMethodType;
     private final boolean implIsInstanceMethod;
     private final ClassWriter cw;
     private final String[] argNames;
@@ -78,8 +81,10 @@
         this.implIsInstanceMethod = implIsInstanceMethod;
         implKind = implInfo.getReferenceKind();
         implMethodClass = implInfo.getDeclaringClass();
+        implMethodClassName = implMethodClass.getName().replace('.', '/');
         implMethodName = implInfo.getName();
         implMethodDesc = implInfo.getMethodType().toMethodDescriptorString();
+        implMethodType = Type.getMethodType(implMethodDesc);
         constructorDesc = constructorType.toMethodDescriptorString();
         lambdaClassName = targetClass.getName().replace('.', '/') + "$$Lambda$" + counter.incrementAndGet();
         cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
@@ -97,9 +102,8 @@
         String samMethodName = samInfo.getName();
         String functionalMethodDesc = functionDescType.toMethodDescriptorString();
  
-        Method[] methods = samClass.getMethods(); // @@@ Use reflection for expedience, but in production should do better
+        Method[] methods = samClass.getMethods(); 
         String samName = samClass.getName().replace('.', '/');
-        // @@@ Class name generation is wrong for many method refs -- use containing class, not target class
         Type samType = Type.getType(samClass);
 
         cw.visit(CLASSFILE_VERSION, ACC_PUBLIC + ACC_SUPER, lambdaClassName, null, NAME_MAGIC_ACCESSOR_IMPL,
@@ -114,27 +118,61 @@
         generateConstructor(constructorDesc);
 
         // Generate methods
-        List<Method> methodsDone = new ArrayList<>();
-        Methods:
+        MethodType samMethodType = samInfo.getMethodType();
+        Class<?>[] samParamTypes = samMethodType.parameterArray();
+        int samParamLength = samParamTypes.length;
+        Class<?> samReturnType = samMethodType.returnType();
+        boolean defaultMethodFound = false;
+        Method samMethod = null;
+        List<Method> methodsFound = new ArrayList<>(methods.length);
+        List<Method> methodsToBridge = new ArrayList<>(methods.length);
+        Class<?> objectClass = Object.class;
+
+        // Find the SAM method and corresponding methods which should be bridged.
+        // SAM method and those to be bridged will have the same name and number of parameters.
+        // Check for default methods (non-abstract), they should not be bridged-over and indicate a complex bridging situation.
         for (Method m : methods) {
-            // @@@ Need a more precise definition of which methods we bridge and which not
-            // This hack approximates it by arity
-            // Also need to exclude default methods that are similarly named; currently using isAbstract to approximate
-            if (!m.getName().equals(samMethodName)
-                || !Modifier.isAbstract(m.getModifiers())
-                || m.getGenericParameterTypes().length != samInfo.getMethodType().parameterCount()
-                || isObjectMethod(m)) {
-                continue Methods;
-            }
-            for (Method md : methodsDone) {
-                if (sameMethod(m, md)) {
-                    continue Methods;
+            if (m.getName().equals(samMethodName) && m.getDeclaringClass() != objectClass) {
+                Class<?>[] mParamTypes = m.getParameterTypes();
+                if (mParamTypes.length == samParamLength) {
+                    if (Modifier.isAbstract(m.getModifiers())) {
+                        // Exclude methods with duplicate signatures
+                        if (!matchesAnyMethod(m, methodsFound)) {
+                            methodsFound.add(m);
+                            if (m.getReturnType().equals(samReturnType) && Arrays.equals(mParamTypes, samParamTypes)) {
+                                // Exact match, this is the SAM method signature
+                                samMethod = m;
+                            } else {
+                                methodsToBridge.add(m);
+                            }
+                        }
+                    } else {
+                        // This is a default method, flag for special processing
+                        defaultMethodFound = true;
+                        // Ignore future matching abstracts.
+                        // Note, due to reabstraction, this is really a punt, hence pass-off to VM
+                        if (!matchesAnyMethod(m, methodsFound)) {
+                            methodsFound.add(m);
+                        }
+                    }
                 }
             }
-            methodsDone.add(m);
+        }
 
-            // @@@ Should make some semblance of attempt to check compatibility of SAM signatures
-            generateForwardingMethod(m, functionalMethodDesc);
+        // Forward the SAM method
+        if (samMethod == null) {
+            throw new LambdaConversionException(String.format("SAM method not found: %s", samMethodType));
+        } else {
+            generateForwardingMethod(samMethod, functionalMethodDesc, false);
+        }
+
+        // Forward the bridges
+        // @@@ Once the VM can do fail-over, uncomment the default method test
+        if (!methodsToBridge.isEmpty() /* && !defaultMethodFound*/) {
+            for (Method m : methodsToBridge) {
+                generateForwardingMethod(m, functionalMethodDesc, true);
+
+            }
         }
 
         if (serializable) {
@@ -147,8 +185,7 @@
     }
 
     private <T> Class<? extends T> defineGeneratedClass(final byte[] classBytes) {
-        // @@@ Get rid of this in production
-        if (System.getProperty("dump.generated") != null) {
+        if (System.getProperty("debug.dump.generated") != null) {
             System.out.printf("Loaded: %s (%d bytes) %n", lambdaClassName, classBytes.length);
             try (FileOutputStream fos = new FileOutputStream(lambdaClassName.replace('/', '.') + ".class")) {
                 fos.write(classBytes);
@@ -162,23 +199,17 @@
         return (Class <? extends T>) unsafe.defineClass(lambdaClassName, classBytes, 0, classBytes.length, loader, pd);
     }
     
-    private static boolean sameMethod(Method m1, Method m2) {
-        Class<?>[] p1 = m1.getParameterTypes();
-        Class<?>[] p2 = m2.getParameterTypes();
-        if (p1.length != p2.length) {
-            return false;
-        }
-        for (int i = 0; i < p1.length; i++) {
-            if (!p1[i].equals(p2[i])) {
-                return false;
+    private static boolean matchesAnyMethod(Method m, List<Method> methods) {
+        Class<?>[] ptypes = m.getParameterTypes();
+        Class<?> rtype = m.getReturnType();
+        for (Method md : methods) {
+            if (md.getReturnType().equals(rtype) && Arrays.equals(ptypes, md.getParameterTypes())) {
+                return true;
             }
         }
-        if (!m1.getReturnType().equals(m2.getReturnType())) {
-            return false;
-        }
-        return true;
+        return false;
     }
-    
+
     private void generateConstructor(String constructorDesc) {
         // Generate constructor
         MethodVisitor ctor = cw.visitMethod(ACC_PUBLIC, NAME_CTOR, constructorDesc, null, null);
@@ -228,21 +259,20 @@
         mv.visitEnd();
     }
 
-    private void generateForwardingMethod(Method m, String functionalMethodDesc) throws InternalError {
+    private void generateForwardingMethod(Method m, String functionalMethodDesc, boolean isBridge) throws InternalError {
         Class<?>[] exceptionTypes = m.getExceptionTypes();
         String[] exceptionNames = new String[exceptionTypes.length];
         for (int i = 0; i < exceptionTypes.length; i++) {
             exceptionNames[i] = exceptionTypes[i].getName().replace('.', '/');
         }
         String methodDescriptor = Type.getMethodDescriptor(m);
-        MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, m.getName(), methodDescriptor, null, exceptionNames);
+        int access = isBridge? ACC_PUBLIC | ACC_BRIDGE : ACC_PUBLIC;
+        MethodVisitor mv = cw.visitMethod(access, m.getName(), methodDescriptor, null, exceptionNames);
         new ForwardingMethodGenerator(mv).generate(m, functionalMethodDesc);
     }
 
     private class ForwardingMethodGenerator extends TypeConvertingMethodAdapter {
 
-        private final String implMethodClassName = implMethodClass.getName().replace('.', '/');
-
         ForwardingMethodGenerator(MethodVisitor mv) {
             super(mv);
         }
@@ -268,7 +298,7 @@
             // @@@ if adapting from non-void to void, need to pop result off stack
             Type samReturnType = Type.getReturnType(m);
             if (!samReturnType.equals(TYPE_VOID)) {
-                convertType(Type.getReturnType(implMethodDesc), samReturnType, samReturnType);
+                convertType(implMethodType.getReturnType(), samReturnType, samReturnType);
             }
             areturn(samReturnType);
 
@@ -289,7 +319,7 @@
                 lvIndex += rcvrType.getSize();
                 convertType(rcvrType, Type.getType(implMethodClass), functionalType);
             }
-            Type[] implArgumentTypes = Type.getArgumentTypes(implMethodDesc);
+            Type[] implArgumentTypes = implMethodType.getArgumentTypes();
             int argOffset = implArgumentTypes.length - samArgumentTypes.length;
             for (int i = samReceiverLength; i < samArgumentTypes.length; i++) {
                 Type argType = samArgumentTypes[i];
@@ -317,61 +347,4 @@
             }
         }
     }
-    
-    private static final Method METHOD_HASH_CODE;
-    private static final Method METHOD_EQUALS;
-    private static final Method METHOD_CLONE;
-    private static final Method METHOD_TO_STRING;
-    private static final Method METHOD_WAIT_1;
-    private static final Method METHOD_WAIT_2;
-    private static final Method METHOD_WAIT_3;
-    private static final Method METHOD_NOTIFY;
-    private static final Method METHOD_NOTIFY_ALL;
-    private static final Method METHOD_GET_CLASS;
-    private static final Method METHOD_FINALIZE;
-
-    static {
-        try {
-            METHOD_HASH_CODE = Object.class.getMethod("hashCode");
-            METHOD_EQUALS = Object.class.getMethod("equals", Object.class);
-            METHOD_CLONE = Object.class.getDeclaredMethod("clone");
-            METHOD_TO_STRING = Object.class.getMethod("toString");
-            METHOD_WAIT_1 = Object.class.getMethod("wait");
-            METHOD_WAIT_2 = Object.class.getMethod("wait", Long.TYPE);
-            METHOD_WAIT_3 = Object.class.getMethod("wait", Long.TYPE, Integer.TYPE);
-            METHOD_NOTIFY = Object.class.getMethod("notify");
-            METHOD_NOTIFY_ALL = Object.class.getMethod("notifyAll");
-            METHOD_GET_CLASS = Object.class.getMethod("getClass");
-            METHOD_FINALIZE = Object.class.getDeclaredMethod("finalize");
-        }
-        catch (NoSuchMethodException e) {
-            throw new Error(e);
-        }
-    }
-
-    static boolean isObjectMethod(Method m) {
-        // @@@ There are more efficient ways to implement this!
-        switch (m.getName()) {
-            case "hashCode":
-                return m.equals(METHOD_HASH_CODE);
-            case "equals":
-                return m.equals(METHOD_EQUALS);
-            case "clone":
-                return m.equals(METHOD_CLONE);
-            case "toString":
-                return m.equals(METHOD_TO_STRING);
-            case "wait":
-                return m.equals(METHOD_WAIT_1) || m.equals(METHOD_WAIT_2) || m.equals(METHOD_WAIT_3);
-            case "notify":
-                return m.equals(METHOD_NOTIFY);
-            case "notifyAll":
-                return m.equals(METHOD_NOTIFY_ALL);
-            case "getClass":
-                return m.equals(METHOD_GET_CLASS);
-            case "finalize":
-                return m.equals(METHOD_FINALIZE);
-            default:
-                return false;
-        }
-    }
 }
--- a/src/share/classes/java/lang/invoke/LambdaMetafactory.java	Wed Aug 22 17:38:46 2012 -0700
+++ b/src/share/classes/java/lang/invoke/LambdaMetafactory.java	Sun Sep 02 09:52:16 2012 -0700
@@ -173,6 +173,7 @@
      * @param implMethod The implementation method which should be called (with suitable adaptation of argument
      *                   types, return types, and adjustment for captured arguments) when methods of the resulting
      *                   functional interface instance are invoked.
+     * @param functionDescType The signature of the SAM method from the target type's perspective
      * @return
      * @throws ReflectiveOperationException
      * @throws LambdaConversionException If any of the metafactory protocol invariants are violated