From 93beb974460e3439b1e71db133930243fb1cac07 Mon Sep 17 00:00:00 2001 From: Jake Wharton Date: Sun, 10 Jan 2016 03:26:57 -0500 Subject: [PATCH] Remove the lamba factory method. Use the static field or constructor directly. --- .../retrolambda/test/LambdaClassesTest.java | 5 +- .../lambdas/BackportLambdaClass.java | 68 +++++++------------ .../lambdas/BackportLambdaInvocations.java | 54 +++++++++++++-- .../retrolambda/lambdas/LambdaClassInfo.java | 40 +++++++++++ .../lambdas/LambdaFactoryMethod.java | 30 -------- .../retrolambda/lambdas/LambdaNaming.java | 3 +- .../retrolambda/lambdas/LambdaReifier.java | 27 ++------ 7 files changed, 122 insertions(+), 105 deletions(-) create mode 100644 retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaClassInfo.java delete mode 100644 retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaFactoryMethod.java diff --git a/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaClassesTest.java b/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaClassesTest.java index 7fdfd93f..ac1cba32 100644 --- a/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaClassesTest.java +++ b/end-to-end-tests/src/test/java/net/orfjackal/retrolambda/test/LambdaClassesTest.java @@ -4,6 +4,7 @@ package net.orfjackal.retrolambda.test; +import com.google.common.collect.ImmutableSet; import org.junit.Test; import java.lang.reflect.Method; @@ -46,7 +47,7 @@ private Dummy2() { @Test public void capturing_lambda_classes_contain_no_unnecessary_methods() throws ClassNotFoundException { - Set expected = new HashSet<>(Arrays.asList("lambdaFactory$", "run")); + Set expected = ImmutableSet.of("run"); Class lambdaClass = Class.forName(Capturing.class.getName() + "$$Lambda$1"); @@ -67,7 +68,7 @@ private Capturing() { @Test public void non_capturing_lambda_classes_contain_no_unnecessary_methods() throws ClassNotFoundException { - Set expected = new HashSet<>(Arrays.asList("lambdaFactory$", "run")); + Set expected = ImmutableSet.of("run"); Class lambdaClass = Class.forName(NonCapturing.class.getName() + "$$Lambda$1"); diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaClass.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaClass.java index 4522c13e..4b834e39 100644 --- a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaClass.java +++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaClass.java @@ -14,10 +14,8 @@ public class BackportLambdaClass extends ClassVisitor { private static final String JAVA_LANG_OBJECT = "java/lang/Object"; private String lambdaClass; - private Type constructor; private Handle implMethod; private Handle accessMethod; - private LambdaFactoryMethod factoryMethod; public BackportLambdaClass(ClassVisitor next) { super(ASM5, next); @@ -26,10 +24,8 @@ public BackportLambdaClass(ClassVisitor next) { @Override public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { lambdaClass = name; - LambdaReifier.setLambdaClass(lambdaClass); implMethod = LambdaReifier.getLambdaImplMethod(); accessMethod = LambdaReifier.getLambdaAccessMethod(); - factoryMethod = LambdaReifier.getLambdaFactoryMethod(); if (superName.equals(LambdaNaming.MAGIC_LAMBDA_IMPL)) { superName = JAVA_LANG_OBJECT; @@ -40,12 +36,32 @@ public void visit(int version, int access, String name, String signature, String @Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { if (name.equals("")) { - constructor = Type.getMethodType(desc); + Type constructor = Type.getMethodType(desc); + int argumentCount = constructor.getArgumentTypes().length; + + String referenceName; + String referenceDesc; + if (argumentCount == 0) { + // Add the static field and static initializer block to keep a singleton instance. + makeSingleton(); + + referenceName = SINGLETON_FIELD_NAME; + referenceDesc = singletonFieldDesc(); + } else { + // Make the constructor visible + access &= ~ACC_PRIVATE; + + referenceName = ""; + referenceDesc = desc; + } + + LambdaClassInfo info = new LambdaClassInfo(lambdaClass, referenceName, referenceDesc, argumentCount); + LambdaReifier.setClassInfo(info); } if (LambdaNaming.isSerializationHook(access, name, desc)) { return null; // remove serialization hooks; we serialize lambda instances as-is } - if (LambdaNaming.isPlatformFactoryMethod(access, name, desc, factoryMethod.getDesc())) { + if (LambdaNaming.isPlatformFactoryMethod(access, name)) { return null; // remove the JVM's factory method which will not be unused } MethodVisitor next = super.visitMethod(access, name, desc, signature, exceptions); @@ -54,17 +70,8 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si return next; } - @Override - public void visitEnd() { - if (isStateless()) { - makeSingleton(); - } - generateFactoryMethod(); - super.visitEnd(); - } - private void makeSingleton() { - FieldVisitor fv = super.visitField(ACC_PRIVATE | ACC_STATIC | ACC_FINAL, + FieldVisitor fv = super.visitField(ACC_STATIC | ACC_FINAL, SINGLETON_FIELD_NAME, singletonFieldDesc(), null, null); fv.visitEnd(); @@ -79,39 +86,10 @@ private void makeSingleton() { mv.visitEnd(); } - private void generateFactoryMethod() { - MethodVisitor mv = cv.visitMethod(ACC_PUBLIC | ACC_STATIC, - factoryMethod.getName(), factoryMethod.getDesc(), null, null); - mv.visitCode(); - - if (isStateless()) { - mv.visitFieldInsn(GETSTATIC, lambdaClass, SINGLETON_FIELD_NAME, singletonFieldDesc()); - mv.visitInsn(ARETURN); - - } else { - mv.visitTypeInsn(NEW, lambdaClass); - mv.visitInsn(DUP); - int varIndex = 0; - for (Type type : constructor.getArgumentTypes()) { - mv.visitVarInsn(type.getOpcode(ILOAD), varIndex); - varIndex += type.getSize(); - } - mv.visitMethodInsn(INVOKESPECIAL, lambdaClass, "", constructor.getDescriptor(), false); - mv.visitInsn(ARETURN); - } - - mv.visitMaxs(-1, -1); // rely on ClassWriter.COMPUTE_MAXS - mv.visitEnd(); - } - private String singletonFieldDesc() { return "L" + lambdaClass + ";"; } - private boolean isStateless() { - return constructor.getArgumentTypes().length == 0; - } - private static class RemoveMagicLambdaConstructorCall extends MethodVisitor { diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java index 17ee2cd6..1e779b1a 100644 --- a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java +++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/BackportLambdaInvocations.java @@ -6,6 +6,7 @@ import net.orfjackal.retrolambda.util.Bytecode; import org.objectweb.asm.*; +import org.objectweb.asm.tree.*; import java.lang.reflect.Field; import java.util.*; @@ -51,7 +52,8 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si if (LambdaNaming.isDeserializationHook(access, name, desc)) { return null; // remove serialization hooks; we serialize lambda instances as-is } - return new InvokeDynamicInsnConverter(super.visitMethod(access, name, desc, signature, exceptions)); + MethodVisitor next = super.visitMethod(access, name, desc, signature, exceptions); + return new InvokeDynamicInsnConverter(access, name, desc, signature, exceptions, next); } Handle getLambdaAccessMethod(Handle implMethod) { @@ -96,10 +98,13 @@ public void visitEnd() { } - private class InvokeDynamicInsnConverter extends MethodVisitor { + private class InvokeDynamicInsnConverter extends MethodNode { + private final MethodVisitor next; - public InvokeDynamicInsnConverter(MethodVisitor next) { - super(ASM5, next); + public InvokeDynamicInsnConverter(int access, String name, String desc, String signature, String[] exceptions, + MethodVisitor next) { + super(ASM5, access, name, desc, signature, exceptions); + this.next = next; } @Override @@ -116,9 +121,46 @@ private void backportLambda(String invokedName, Type invokedType, Handle bsm, Ob Handle implMethod = (Handle) bsmArgs[1]; Handle accessMethod = getLambdaAccessMethod(implMethod); - LambdaFactoryMethod factory = LambdaReifier.reifyLambdaClass(implMethod, accessMethod, + LambdaClassInfo info = LambdaReifier.reifyLambdaClass(implMethod, accessMethod, invoker, invokedName, invokedType, bsm, bsmArgs); - super.visitMethodInsn(INVOKESTATIC, factory.getOwner(), factory.getName(), factory.getDesc(), false); + + if (info.isStateless()) { + super.visitFieldInsn(GETSTATIC, info.getLambdaClass(), info.getReferenceName(), + info.getReferenceDesc()); + return; + } + + // At this point we know that the lambda is capturing and will require load bytecodes for its arguments. + // Since these must occur after the new/dup bytecodes, find the first load instruction and place the + // new/dup bytecode before it. + AbstractInsnNode firstLoad = null; + for (int i = instructions.size() - 1, seen = 0; i >= 0; i--) { + AbstractInsnNode instruction = instructions.get(i); + int opcode = instruction.getOpcode(); + if (opcode == ALOAD || opcode == ILOAD || opcode == LLOAD || opcode == FLOAD || opcode == DLOAD || opcode == GETSTATIC) { + seen++; + if (seen == info.getArgumentCount()) { + firstLoad = instruction; + break; + } + } + } + if (firstLoad == null) { + throw new IllegalStateException( + "Unable to find expected load instruction count. Please report this as a bug."); + } + + instructions.insertBefore(firstLoad, new TypeInsnNode(NEW, info.getLambdaClass())); + instructions.insertBefore(firstLoad, new InsnNode(DUP)); + + super.visitMethodInsn(INVOKESPECIAL, info.getLambdaClass(), info.getReferenceName(), + info.getReferenceDesc(), false); + } + + @Override + public void visitEnd() { + // Forward all recorded instructions to the delegate method visitor. + accept(next); } } diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaClassInfo.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaClassInfo.java new file mode 100644 index 00000000..9a0ae5d7 --- /dev/null +++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaClassInfo.java @@ -0,0 +1,40 @@ +// Copyright © 2013-2014 Esko Luontola +// This software is released under the Apache License 2.0. +// The license text is at http://www.apache.org/licenses/LICENSE-2.0 + +package net.orfjackal.retrolambda.lambdas; + +public class LambdaClassInfo { + + private final String lambdaClass; + private final String referenceName; + private final String referenceDesc; + private final int argumentCount; + + public LambdaClassInfo(String lambdaClass, String referenceName, String referenceDesc, int argumentCount) { + this.lambdaClass = lambdaClass; + this.referenceName = referenceName; + this.referenceDesc = referenceDesc; + this.argumentCount = argumentCount; + } + + public boolean isStateless() { + return getArgumentCount() == 0; + } + + public String getLambdaClass() { + return lambdaClass; + } + + public String getReferenceName() { + return referenceName; + } + + public String getReferenceDesc() { + return referenceDesc; + } + + public int getArgumentCount() { + return argumentCount; + } +} diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaFactoryMethod.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaFactoryMethod.java deleted file mode 100644 index c4d771fc..00000000 --- a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaFactoryMethod.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright © 2013-2014 Esko Luontola -// This software is released under the Apache License 2.0. -// The license text is at http://www.apache.org/licenses/LICENSE-2.0 - -package net.orfjackal.retrolambda.lambdas; - -import org.objectweb.asm.Type; - -public class LambdaFactoryMethod { - - private final String owner; - private final String desc; - - public LambdaFactoryMethod(String lambdaClass, Type invokedType) { - owner = lambdaClass; - desc = invokedType.getDescriptor(); - } - - public String getOwner() { - return owner; - } - - public String getName() { - return "lambdaFactory$"; - } - - public String getDesc() { - return desc; - } -} diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaNaming.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaNaming.java index 13d15dc3..ec55fa3a 100644 --- a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaNaming.java +++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaNaming.java @@ -32,9 +32,8 @@ public static boolean isDeserializationHook(int access, String name, String desc && Flags.hasFlag(access, ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC); } - public static boolean isPlatformFactoryMethod(int access, String name, String desc, String targetDesc) { + public static boolean isPlatformFactoryMethod(int access, String name) { return name.equals("get$Lambda") - && desc.equals(targetDesc) && Flags.hasFlag(access, ACC_PRIVATE | ACC_STATIC); } } diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaReifier.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaReifier.java index b5094c04..51d52647 100644 --- a/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaReifier.java +++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/lambdas/LambdaReifier.java @@ -20,23 +20,21 @@ public class LambdaReifier { private static final BlockingDeque currentLambdaImplMethod = new LinkedBlockingDeque<>(1); private static final BlockingDeque currentLambdaAccessMethod = new LinkedBlockingDeque<>(1); private static final BlockingDeque> currentInvoker = new LinkedBlockingDeque<>(1); - private static final BlockingDeque currentInvokedType = new LinkedBlockingDeque<>(1); - private static final BlockingDeque currentLambdaClass = new LinkedBlockingDeque<>(1); + private static final BlockingDeque currentClassInfo = new LinkedBlockingDeque<>(1); - public static LambdaFactoryMethod reifyLambdaClass(Handle lambdaImplMethod, Handle lambdaAccessMethod, - Class invoker, String invokedName, Type invokedType, Handle bsm, Object[] bsmArgs) { + public static LambdaClassInfo reifyLambdaClass(Handle lambdaImplMethod, Handle lambdaAccessMethod, + Class invoker, String invokedName, Type invokedType, Handle bsm, Object[] bsmArgs) { try { setLambdaImplMethod(lambdaImplMethod); setLambdaAccessMethod(lambdaAccessMethod); setInvoker(invoker); - setInvokedType(invokedType); // Causes the lambda class to be loaded. Retrolambda's Java agent // will detect it, save it to a file and tell us (via the globals // in this class) that what the name of the lambda class was. callBootstrapMethod(invoker, invokedName, invokedType, bsm, bsmArgs); - return getLambdaFactoryMethod(); + return currentClassInfo.getFirst(); } catch (Throwable t) { throw new RuntimeException("Failed to backport lambda or method reference: " + lambdaImplMethod, t); @@ -57,12 +55,8 @@ private static void setInvoker(Class lambdaInvoker) { currentInvoker.push(lambdaInvoker); } - private static void setInvokedType(Type invokedType) { - currentInvokedType.push(invokedType); - } - - public static void setLambdaClass(String lambdaClass) { - currentLambdaClass.push(lambdaClass); + public static void setClassInfo(LambdaClassInfo info) { + currentClassInfo.push(info); } public static boolean isLambdaClassToReify(String className) { @@ -80,18 +74,11 @@ public static Handle getLambdaAccessMethod() { return currentLambdaAccessMethod.getFirst(); } - public static LambdaFactoryMethod getLambdaFactoryMethod() { - String lambdaClass = currentLambdaClass.getFirst(); - Type invokedType = currentInvokedType.getFirst(); - return new LambdaFactoryMethod(lambdaClass, invokedType); - } - private static void resetGlobals() { currentLambdaImplMethod.clear(); currentLambdaAccessMethod.clear(); currentInvoker.clear(); - currentInvokedType.clear(); - currentLambdaClass.clear(); + currentClassInfo.clear(); } private static CallSite callBootstrapMethod(Class invoker, String invokedName, Type invokedType, Handle bsm, Object[] bsmArgs) throws Throwable {