Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate the need for VM agent #27

Closed
jtulach opened this issue Aug 17, 2014 · 9 comments
Closed

Eliminate the need for VM agent #27

jtulach opened this issue Aug 17, 2014 · 9 comments

Comments

@jtulach
Copy link

jtulach commented Aug 17, 2014

I eliminated the need for VM agent by hooking into internal lambda classes dumping mechanism. I don't know if that is direction you'd like to move, so I am not starting a pull request, rather than recording my work in an issue.


Eliminating need for agent (and thus starting separate VM) by hooking into internal lamda classes dump mechanism. Works on 1.8.0_11-b12

diff --git a/retrolambda-maven-plugin/src/main/java/net/orfjackal/retrolambda/maven/ProcessClassesMojo.java b/retrolambda-maven-plugin/src/main/java/net/orfjackal/retrolambda/maven/ProcessClassesMojo.java
index a945a78..f766043 100644
--- a/retrolambda-maven-plugin/src/main/java/net/orfjackal/retrolambda/maven/ProcessClassesMojo.java
+++ b/retrolambda-maven-plugin/src/main/java/net/orfjackal/retrolambda/maven/ProcessClassesMojo.java
@@ -6,14 +6,18 @@ package net.orfjackal.retrolambda.maven;

 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableMap;
+import java.io.*;
+import java.lang.reflect.Method;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.*;
+import org.apache.maven.artifact.Artifact;
 import org.apache.maven.execution.MavenSession;
 import org.apache.maven.plugin.*;
 import org.apache.maven.plugins.annotations.*;
 import org.apache.maven.project.MavenProject;
 import org.apache.maven.toolchain.*;

-import java.io.*;
-import java.util.*;

 import static org.twdata.maven.mojoexecutor.MojoExecutor.*;

@@ -132,28 +136,29 @@ abstract class ProcessClassesMojo extends AbstractMojo {

     private void processClasses() throws MojoExecutionException {
         String retrolambdaJar = getRetrolambdaJarPath();
-        executeMojo(
-                plugin(groupId("org.apache.maven.plugins"),
-                        artifactId("maven-antrun-plugin"),
-                        version("1.7")),
-                goal("run"),
-                configuration(element(
-                        "target",
-                        element("property",
-                                attributes(attribute("name", "the_classpath"),
-                                        attribute("refid", getClasspathId()))),
-                        element("exec",
-                                attributes(
-                                        attribute("executable", getJavaCommand()),
-                                        attribute("failonerror", "true")),
-                                element("arg", attribute("value", "-Dretrolambda.bytecodeVersion=" + targetBytecodeVersions.get(target))),
-                                element("arg", attribute("value", "-Dretrolambda.inputDir=" + getInputDir().getAbsolutePath())),
-                                element("arg", attribute("value", "-Dretrolambda.outputDir=" + getOutputDir().getAbsolutePath())),
-                                element("arg", attribute("value", "-Dretrolambda.classpath=${the_classpath}")),
-                                element("arg", attribute("value", "-javaagent:" + retrolambdaJar)),
-                                element("arg", attribute("value", "-jar")),
-                                element("arg", attribute("value", retrolambdaJar))))),
-                executionEnvironment(project, session, pluginManager));
+        File jar = new File(retrolambdaJar);
+        
+        try {
+            StringBuilder sb = new StringBuilder();
+            sb.append(getInputDir());
+            for (Artifact a : project.getArtifacts()) {
+                if (a.getFile() != null) {
+                    sb.append(File.pathSeparator);
+                    sb.append(a.getFile());
+                }
+            }
+        
+            URLClassLoader url = new URLClassLoader(new URL[] { jar.toURI().toURL() });
+            Class<?> mainClass = Class.forName("net.orfjackal.retrolambda.Main", true, url);
+            System.setProperty("retrolambda.bytecodeVersion", "" + targetBytecodeVersions.get(target));
+            System.setProperty("retrolambda.inputDir", getInputDir().getAbsolutePath());
+            System.setProperty("retrolambda.outputDir", getOutputDir().getAbsolutePath());
+            System.setProperty("retrolambda.classpath", sb.toString());
+            Method main = mainClass.getMethod("main", String[].class);
+            main.invoke(null, (Object) new String[0]);
+        } catch (Exception ex) {
+            throw new MojoExecutionException("Cannot initialize classloader for " + retrolambdaJar, ex);
+        }
     }

     private String getRetrolambdaJarPath() {
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/Config.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/Config.java
index 7d07447..502716d 100644
--- a/retrolambda/src/main/java/net/orfjackal/retrolambda/Config.java
+++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/Config.java
@@ -43,7 +43,7 @@ public class Config {
     }

     public boolean isFullyConfigured() {
-        return hasAllRequiredProperties() && PreMain.isAgentLoaded();
+        return hasAllRequiredProperties();
     }

     private boolean hasAllRequiredProperties() {
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/LambdaSavingClassFileTransformer.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/LambdaSavingClassFileTransformer.java
index fef4f1d..eda6e2d 100644
--- a/retrolambda/src/main/java/net/orfjackal/retrolambda/LambdaSavingClassFileTransformer.java
+++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/LambdaSavingClassFileTransformer.java
@@ -4,13 +4,28 @@

 package net.orfjackal.retrolambda;

-import org.objectweb.asm.ClassReader;
-
-import java.lang.instrument.*;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.channels.WritableByteChannel;
 import java.nio.file.*;
-import java.security.ProtectionDomain;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.FileAttribute;
+import java.nio.file.attribute.FileAttributeView;
+import java.nio.file.attribute.UserPrincipalLookupService;
+import java.nio.file.spi.FileSystemProvider;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;

-public class LambdaSavingClassFileTransformer implements ClassFileTransformer {
+public class LambdaSavingClassFileTransformer {

     private final Path outputDir;
     private final int targetVersion;
@@ -20,17 +35,35 @@ public class LambdaSavingClassFileTransformer implements ClassFileTransformer {
         this.targetVersion = targetVersion;
     }

-    @Override
-    public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
-        if (className == null) {
-            // Since JDK 8 build b121 or so, lambda classes have a null class name,
-            // but we can read it from the bytecode where the name still exists.
-            className = new ClassReader(classfileBuffer).getClassName();
+    private Field field;
+    void registerDumper() {
+        try {
+            Class<?> dumper = Class.forName("java.lang.invoke.ProxyClassesDumper");
+            Constructor<?> cnstr = dumper.getDeclaredConstructor(Path.class);
+            cnstr.setAccessible(true);
+            Class<?> mf = Class.forName("java.lang.invoke.InnerClassLambdaMetafactory");
+            field = mf.getDeclaredField("dumper");
+            Field m = field.getClass().getDeclaredField("modifiers");
+            m.setAccessible(true);
+            int mod = m.getInt(field);
+            m.setInt(field, mod & ~Modifier.FINAL);
+            field.setAccessible(true);
+            
+            Path p = new VirtualPath("");
+            field.set(null, cnstr.newInstance(p));
+        } catch (Exception ex) {
+            throw new IllegalStateException("Cannot initialize dumper", ex);
         }
-        if (LambdaReifier.isLambdaClassToReify(className)) {
-            reifyLambdaClass(className, classfileBuffer);
+    }
+    
+    void unregisterDumper() {
+        if (field != null) {
+            try {
+                field.set(null, null);
+            } catch (Exception ex) {
+                throw new IllegalArgumentException(ex);
+            }
         }
-        return null;
     }

     private void reifyLambdaClass(String className, byte[] classfileBuffer) {
@@ -47,4 +80,361 @@ public class LambdaSavingClassFileTransformer implements ClassFileTransformer {
             t.printStackTrace(System.out);
         }
     }
+    
+    private final class VirtualProvider extends FileSystemProvider {
+
+        @Override
+        public String getScheme() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public FileSystem newFileSystem(URI uri, Map<String, ?> env) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public FileSystem getFileSystem(URI uri) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getPath(URI uri) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public SeekableByteChannel newByteChannel(Path path, Set<? extends OpenOption> options, FileAttribute<?>... attrs) throws IOException {
+            return new ClassChannel(path);
+        }
+
+        @Override
+        public DirectoryStream<Path> newDirectoryStream(Path dir, DirectoryStream.Filter<? super Path> filter) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void createDirectory(Path dir, FileAttribute<?>... attrs) throws IOException {
+        }
+
+        @Override
+        public void delete(Path path) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void copy(Path source, Path target, CopyOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void move(Path source, Path target, CopyOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean isSameFile(Path path, Path path2) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean isHidden(Path path) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public FileStore getFileStore(Path path) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void checkAccess(Path path, AccessMode... modes) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public <V extends FileAttributeView> V getFileAttributeView(Path path, Class<V> type, LinkOption... options) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public <A extends BasicFileAttributes> A readAttributes(Path path, Class<A> type, LinkOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public void setAttribute(Path path, String attribute, Object value, LinkOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+        
+    }
+    
+    private final class VirtualFS extends FileSystem {
+
+        @Override
+        public FileSystemProvider provider() {
+            return new VirtualProvider();
+        }
+
+        @Override
+        public void close() throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean isOpen() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean isReadOnly() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public String getSeparator() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Iterable<Path> getRootDirectories() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Iterable<FileStore> getFileStores() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Set<String> supportedFileAttributeViews() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getPath(String first, String... more) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public PathMatcher getPathMatcher(String syntaxAndPattern) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public UserPrincipalLookupService getUserPrincipalLookupService() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public WatchService newWatchService() throws IOException {
+            throw new IllegalStateException();
+        }
+        
+    }
+    
+    private final class VirtualPath implements Path {
+        private final String path;
+
+        public VirtualPath(String path) {
+            this.path = path;
+        }
+
+        @Override
+        public FileSystem getFileSystem() {
+            return new VirtualFS();
+        }
+
+        @Override
+        public boolean isAbsolute() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getRoot() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getFileName() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getParent() {
+            return this;
+        }
+
+        @Override
+        public int getNameCount() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path getName(int index) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path subpath(int beginIndex, int endIndex) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean startsWith(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean startsWith(String other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean endsWith(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean endsWith(String other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path normalize() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path resolve(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path resolve(String other) {
+            assert path.isEmpty();
+            return new VirtualPath(other);
+        }
+
+        @Override
+        public Path resolveSibling(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path resolveSibling(String other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path relativize(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public URI toUri() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path toAbsolutePath() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Path toRealPath(LinkOption... options) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public File toFile() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public WatchKey register(WatchService watcher, WatchEvent.Kind<?>[] events, WatchEvent.Modifier... modifiers) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public WatchKey register(WatchService watcher, WatchEvent.Kind<?>... events) throws IOException {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public Iterator<Path> iterator() {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public int compareTo(Path other) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public String toString() {
+            return path;
+        }
+    }
+    
+    private final class ClassChannel implements SeekableByteChannel {
+        private final Path path;
+        private final ByteArrayOutputStream os;
+        private final WritableByteChannel ch;
+
+        public ClassChannel(Path path) {
+            this.path = path;
+            this.os = new ByteArrayOutputStream();
+            this.ch = Channels.newChannel(os);
+        }
+        
+        @Override
+        public int read(ByteBuffer dst) throws IOException {
+            throw new IOException();
+        }
+
+        @Override
+        public int write(ByteBuffer src) throws IOException {
+            return ch.write(src);
+        }
+
+        @Override
+        public long position() throws IOException {
+            throw new IOException();
+        }
+
+        @Override
+        public SeekableByteChannel position(long newPosition) throws IOException {
+            throw new IOException();
+        }
+
+        @Override
+        public long size() throws IOException {
+            throw new IOException();
+        }
+
+        @Override
+        public SeekableByteChannel truncate(long size) throws IOException {
+            throw new IOException();
+        }
+
+        @Override
+        public boolean isOpen() {
+            return true;
+        }
+
+        @Override
+        public void close() throws IOException {
+            String className = path.toString();
+            className = className.substring(0, className.length() - 6);
+            if (LambdaReifier.isLambdaClassToReify(className)) {
+                reifyLambdaClass(className, os.toByteArray());
+            }
+        }
+    } // end of ClassCastException
 }
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/Main.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/Main.java
index feba36e..98d98c0 100644
--- a/retrolambda/src/main/java/net/orfjackal/retrolambda/Main.java
+++ b/retrolambda/src/main/java/net/orfjackal/retrolambda/Main.java
@@ -43,7 +43,11 @@ public class Main {
             visitFiles(inputDir, includedFiles, new BytecodeTransformingFileVisitor(inputDir, outputDir) {
                 @Override
                 protected byte[] transform(byte[] bytecode) {
-                    return LambdaUsageBackporter.transform(bytecode, bytecodeVersion);
+                    final LambdaSavingClassFileTransformer trans = new LambdaSavingClassFileTransformer(outputDir, bytecodeVersion);
+                    trans.registerDumper();
+                    byte[] ret = LambdaUsageBackporter.transform(bytecode, bytecodeVersion);
+                    trans.unregisterDumper();
+                    return ret;
                 }
             });
         } catch (Throwable t) {
diff --git a/retrolambda/src/main/java/net/orfjackal/retrolambda/PreMain.java b/retrolambda/src/main/java/net/orfjackal/retrolambda/PreMain.java
deleted file mode 100644
index ddb1a6d..0000000
--- a/retrolambda/src/main/java/net/orfjackal/retrolambda/PreMain.java
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright © 2013 Esko Luontola <www.orfjackal.net>
-// 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;
-
-import java.lang.instrument.Instrumentation;
-import java.nio.file.Path;
-
-public class PreMain {
-
-    private static boolean agentLoaded = false;
-
-    public static void premain(String agentArgs, Instrumentation inst) {
-        Config config = new Config(System.getProperties());
-        int bytecodeVersion = config.getBytecodeVersion();
-        Path outputDir = config.getOutputDir();
-        inst.addTransformer(new LambdaSavingClassFileTransformer(outputDir, bytecodeVersion));
-        agentLoaded = true;
-    }
-
-    public static boolean isAgentLoaded() {
-        return agentLoaded;
-    }
-}
@luontola
Copy link
Owner

Is there some disadvantage to using the Java agent? For example some project that is unable to use Retrolambda because of it. I can only come up with the disadvantage of requiring the typing of more command line parameters, but invoking Retrolambda should anyways be automated in the build.

I'm worried that relying on the internal dumping mechanism would be more fragile - the implementation details are more likely to change between Java 8 releases. The Java agent mechanism only assumes the naming pattern of the generated classes. The dumping mechanism assumes details of how InnerClassLambdaMetafactory and ProxyClassesDumper have been implemented. (Both of them assume that lamdas are implemented as anonymous classes.)

Regarding this patch, instead of faking the file system, it would be preferrable to override java.lang.invoke.ProxyClassesDumper#dumpClass(String className, final byte[] classBytes). Unfortunately the class is package private and final, so it's not possible with plain Java code; the JVM prevents user code from adding classes to java.* packages, and anyways it's final. It might be possible to extend that class by using sun.misc.Unsafe#defineClass which bypasses the JVM's security checks. (The InnerClassLambdaMetafactory uses Unsafe's defineAnonymousClass and ensureClassInitialized which lets the lambda class call private lambda implementation methods on the class containing the lambda.)

@ntherning
Copy link

This is quite interesting as I'm currently working on something similar to what @jtulach is attempting. In RoboVM we would like to support Lambdas so what I'm doing is to break out the stuff from OpenJDK which is responsible for creating the bytecode for the lambda inner classes at runtime (LambdaMetafactory, InnerClassLambdaMetafactory and AbstractValidatingLambdaMetafactory). I am refactoring those classes to work against something else then java.lang.Class. I've had retrolambda in mind from the start so I'm doing this in a pluggable manner. We use Soot as our underlying provider of class information but you could as well use ASM. Once it works you won't have to run any agent and won't have to run in a Java8 JVM anymore to generate the inner classes.

Would you guys be interested in using this? I haven't published anything yet but once it works I will put it somewhere under the RoboVM organization.

@luontola
Copy link
Owner

Once it works you won't have to run any agent and won't have to run in a Java8 JVM anymore to generate the inner classes.

If somebody is compiling Java 8 code with lambdas, that means that they have JDK 8 on their development machine and they will always be able to run Retrolambda under Java 8 as part of their build.

I don't see any advantage in trying to avoid the Java 8 runtime or Java agents. On the contratry, a third-party mechanism of generating the lamdba classes has the disadvantage that it has not been tested as much as the JDK's implementation and it may have its own bugs. Plus it's more code to maintain.

In RoboVM's case it makes sense to include the lambda reification in the RoboVM compiler, to have one tool that does all the work. But you are anyways generating native code, which is a much bigger problem scope than just backporting lambdas to older Java VMs.

@jtulach
Copy link
Author

jtulach commented Aug 18, 2014

Hello Niklas.
I had @robovm in mind when I started to play with RetroLamda. Btw. my BOF
https://oracleus.activeevents.com/2014/connect/sessionDetail.ww?SESSION_ID=4941
got accepted and I hope we'll meet there and show everyone JDK8 features
are available for both RoboVM as well as Bck2Brwsr VM.

I was glad to find RetroLambda as writing the same from scratch would
prevent me to finish by JavaOne. However right now it is a bit "heavy
weight" and I can only use it in AOT compilation, I cannot use it during
JIT (e.g. in browser):

  • I'd like to avoid using java.lang.invoke classes all together.
  • I need to replace the bytecode generation and emit JavaScript
    The less classes used while doing this the better.

2014-08-17 16:55 GMT+02:00 Niklas Therning notifications@github.com:

This is quite interesting as I'm currently working on something similar to
what @jtulach https://github.com/jtulach is attempting. In RoboVM we
would like to support Lambdas so what I'm doing is to break out the stuff
from OpenJDK which is responsible for creating the bytecode for the lambda
inner classes at runtime (LambdaMetafactory, InnerClassLambdaMetafactory
and AbstractValidatingLambdaMetafactory). I am refactoring those classes
to work against something else then java.lang.Class. I've had retrolambda
in mind from the start so I'm doing this in a pluggable manner. We use
Soot https://github.com/Sable/soot as our underlying provider of class
information but you could as well use ASM. Once it works you won't have to
run any agent and won't have to run in a Java8 JVM anymore to generate the
inner classes.

Would you guys be interested in using this? I haven't published anything
yet but once it works I will put it somewhere under the RoboVM organization.


Reply to this email directly or view it on GitHub
#27 (comment)
.

@jtulach
Copy link
Author

jtulach commented Aug 18, 2014

Hello Esko,
first of all hats-off for great work. As you mentioned using Java agent has
the benefit of simulating as closely as possible what JDK8 does. However I
want to run in more restricted environments: My friends use Javac on Dalvik
and I am running Javac in a browser (see http://dew.apidesign.org/dew/).
There is no Java agent on these systems, of course. As such I seek for
alternatives.

Also I wanted to avoid starting separate VM during the Maven execution.
Such forked process is harder to debug, while now it is just a matter of a
single button click (e.g.
http://wiki.apidesign.org/wiki/Image:DebugMavenBuild.png) to let me find
out what exactly is happening.

Right, the dumping mechanism may more easily go away, while the actual
generated class file format is likely to stay. As such I don't expect you
to accept my patch into main stream. I just recorded it as an option in
case it is found useful in the future.

Subclassing ProxyClassesDumper was the first thing I wanted to try, but
Path was the first type that was not final.

2014-08-17 11:07 GMT+02:00 Esko Luontola notifications@github.com:

Is there some disadvantage to using the Java agent? For example some
project that is unable to use Retrolambda because of it. I can only come up
with the disadvantage of requiring the typing of more command line
parameters, but invoking Retrolambda should anyways be automated in the
build.

I'm worried that relying on the internal dumping mechanism would be more
fragile - the implementation details are more likely to change between Java
8 releases. The Java agent mechanism only assumes the naming pattern of the
generated classes. The dumping mechanism assumes details of how
InnerClassLambdaMetafactory and ProxyClassesDumper have been implemented.
(Both of them assume that lamdas are implemented as anonymous classes.)

Regarding this patch, instead of faking the file system, it would be
preferrable to override java.lang.invoke.ProxyClassesDumper#dumpClass(String
className, final byte[] classBytes). Unfortunately the class is package
private, so it's not possible with plain Java code, since the JVM prevents
user code from adding classes to java.* packages. It might be possible to
extend that class by using sun.misc.Unsafe#defineClass which bypasses the
JVM's security checks. (The InnerClassLambdaMetafactory uses Unsafe's
defineAnonymousClass and ensureClassInitialized which lets the lambda
class call private lambda implementation methods on the class containing
the lambda.)


Reply to this email directly or view it on GitHub
#27 (comment)
.

@luontola
Copy link
Owner

Also I wanted to avoid starting separate VM during the Maven execution.
Such forked process is harder to debug, while now it is just a matter of a
single button click (e.g.
http://wiki.apidesign.org/wiki/Image:DebugMavenBuild.png) to let me find
out what exactly is happening.

That's a good point. If we can avoid forking the process, then it would also make the build faster. The overhead of starting a JVM is about 100-200ms and that is multiplied by the number of modules times two (i.e. main and test classes).

I think that feature should be toggleable, just in case it causes problems for some (e.g. Maven is not run under Java 8 or the dumper implementation changes). I can have a look at it.

@luontola
Copy link
Owner

Please create a pull request where instead of modifying LambdaSavingClassFileTransformer you create a copy of it and change that. In the Maven plugin add a parameter for toggling between the two implementations. In Main you can toggle the implementations based on PreMain.isAgentLoaded().

@jtulach
Copy link
Author

jtulach commented Aug 19, 2014

As requested: #28

@luontola
Copy link
Owner

This has been included in Retrolambda 1.6.0.

It made my other project's build faster - from 52 to 42 seconds. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants