vipx / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Use ASM 4.2 (or 5.0_BETA for Java8 support) #759

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
See http://forge.ow2.org/forum/forum.php?forum_id=2168 for the list of fixes 
and performance improvements

Original issue reported on code.google.com by mccu...@gmail.com on 8 Aug 2013 at 8:46

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
The following CGLIB patch is a pre-req for upgrading to ASM 4.1:

Index: src/proxy/net/sf/cglib/core/DebuggingClassWriter.java
===================================================================
RCS file: 
/cvsroot/cglib/cglib/src/proxy/net/sf/cglib/core/DebuggingClassWriter.java,v
retrieving revision 1.15
diff -u -r1.15 DebuggingClassWriter.java
--- src/proxy/net/sf/cglib/core/DebuggingClassWriter.java   25 May 2012 22:56:16 
-0000   1.15
+++ src/proxy/net/sf/cglib/core/DebuggingClassWriter.java   15 Jan 2013 21:08:02 
-0000
@@ -46,7 +46,7 @@
     }

     public DebuggingClassWriter(int flags) {
-   super(flags, new ClassWriter(flags));
+   super(Opcodes.ASM4, new ClassWriter(flags));
     }

     public void visit(int version,

From http://sourceforge.net/p/cglib/bugs/39/

Original comment by mccu...@gmail.com on 8 Aug 2013 at 9:37

GoogleCodeExporter commented 9 years ago
BTW the following CGLIB patch removes the need to include asm-util when using 
CGLIB by fixing the dynamic lookup for TraceClassVisitor:

diff -wur 3.0/src/proxy/net/sf/cglib/core/DebuggingClassWriter.java 
3.0-patched/src/proxy/net/sf/cglib/core/DebuggingClassWriter.java
--- 3.0/src/proxy/net/sf/cglib/core/DebuggingClassWriter.java   2012-05-25 
13:21:20.000000000 +0100
+++ 
3.0-patched/src/proxy/net/sf/cglib/core/DebuggingClassWriter.java   2013-04-01 
13:31:58.000000000 +0100
@@ -22,13 +22,14 @@
 import org.objectweb.asm.util.TraceClassVisitor;

 import java.io.*;
+import java.lang.reflect.Constructor;

 public class DebuggingClassWriter extends ClassVisitor {

     public static final String DEBUG_LOCATION_PROPERTY = "cglib.debugLocation";

     private static String debugLocation;
-    private static boolean traceEnabled;
+    private static Constructor traceCtor;

     private String className;
     private String superName;
@@ -38,15 +39,15 @@
         if (debugLocation != null) {
             System.err.println("CGLIB debugging enabled, writing to '" + debugLocation + "'");
             try {
-                Class.forName("org.objectweb.asm.util.TraceClassVisitor");
-                traceEnabled = true;
+                Class clazz = 
Class.forName("org.objectweb.asm.util.TraceClassVisitor");
+                traceCtor = clazz.getConstructor(new 
Class[]{ClassVisitor.class, PrintWriter.class});
             } catch (Throwable ignore) {
             }
         }
     }

     public DebuggingClassWriter(int flags) {
-   super(flags, new ClassWriter(flags));
+   super(Opcodes.ASM4, new ClassWriter(flags));
     }

     public void visit(int version,
@@ -89,20 +90,20 @@
                             out.close();
                         }

-                        if (traceEnabled) {
+                        if (traceCtor != null) {
                             file = new File(new File(debugLocation), dirs + ".asm");
                             out = new BufferedOutputStream(new FileOutputStream(file));
                             try {
                                 ClassReader cr = new ClassReader(b);
                                 PrintWriter pw = new PrintWriter(new OutputStreamWriter(out));
-                                TraceClassVisitor tcv = new 
TraceClassVisitor(null, pw);
+                                ClassVisitor tcv = 
(ClassVisitor)traceCtor.newInstance(new Object[]{null, pw});
                                 cr.accept(tcv, 0);
                                 pw.flush();
                             } finally {
                                 out.close();
                             }
                         }
-                    } catch (IOException e) {
+                    } catch (Exception e) {
                         throw new CodeGenerationException(e);
                     }
                 }

From http://sourceforge.net/p/cglib/bugs/42/

Original comment by mccu...@gmail.com on 8 Aug 2013 at 9:42

GoogleCodeExporter commented 9 years ago
With the one-line CGLIB patch from comment #2 the ASM dependency can be bumped 
to 4.1 as follows:

diff --git a/core/pom.xml b/core/pom.xml
index 328af17..52e64e8 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -15,7 +15,7 @@

   <properties>
     <cglib.version>3.0</cglib.version>
-    <asm.version>4.0</asm.version>
+    <asm.version>4.1</asm.version>
   </properties>

   <dependencies>
@@ -35,6 +35,11 @@
       <version>11.0.1</version>
     </dependency>
     <dependency>
+      <groupId>org.ow2.asm</groupId>
+      <artifactId>asm</artifactId>
+      <version>${asm.version}</version>
+    </dependency>
+    <dependency>
       <groupId>cglib</groupId>
       <artifactId>cglib</artifactId>
       <version>${cglib.version}</version>
@@ -220,6 +225,12 @@
          | Mark as optional: embedded by JarJar
         -->
         <dependency>
+          <groupId>org.ow2.asm</groupId>
+          <artifactId>asm</artifactId>
+          <version>${asm.version}</version>
+          <optional>true</optional>
+        </dependency>
+        <dependency>
           <groupId>cglib</groupId>
           <artifactId>cglib</artifactId>
           <version>${cglib.version}</version>

Although as CGLIB will need to be patched then we might as well update the ASM 
dependency there to 4.1, in which case the only change needed in Guice would 
then be:

diff --git a/core/pom.xml b/core/pom.xml
index 328af17..52e64e8 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -15,7 +15,7 @@

   <properties>
     <cglib.version>3.0</cglib.version>
-    <asm.version>4.0</asm.version>
+    <asm.version>4.1</asm.version>
   </properties>

   <dependencies>

Original comment by mccu...@gmail.com on 8 Aug 2013 at 9:45

GoogleCodeExporter commented 9 years ago
FYI, I'm also experimenting with ASM 5.0_BETA (via patched version of cglib 
that has the DebuggingClassWriter fixes) in 
https://github.com/sonatype/sisu-guice/commit/cbd7bef3f1bd1f57cf55e862a0ed1df66c
dd380d to see how Guice behaves with Java8.

Hopefully the main cglib project will eventually pull in the proposed 
DebuggingClassWriter fixes (at minimum http://sourceforge.net/p/cglib/bugs/39/ 
but http://sourceforge.net/p/cglib/bugs/42/ would be nice to have) then I can 
switch back to a mainstream release of cglib :)

Original comment by mccu...@gmail.com on 22 Oct 2013 at 5:45

GoogleCodeExporter commented 9 years ago
I'll take a look at pulling in the cglib changes and cutting a release with 
them.  Probably won't be for a bit though.

Original comment by sberlin on 22 Oct 2013 at 6:15

GoogleCodeExporter commented 9 years ago
Pushed a new cglib (3.1) with the fix, updated guice to use that new cglib plus 
ASM 4.2.  The POMs still need updating, but someone will need to host the cglib 
3.1 somewhere first.

Original comment by sberlin on 7 Dec 2013 at 5:57