You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by ej...@apache.org on 2012/02/16 23:38:28 UTC

svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Author: ejiang
Date: Thu Feb 16 22:38:27 2012
New Revision: 1245223

URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
Log:
ARIES-817: ASM4 for Java7 support

Added:
    aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
Modified:
    aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
    aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java

Added: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
==============================================================================
--- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java (added)
+++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java Thu Feb 16 22:38:27 2012
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.aries.proxy.impl;
+
+import java.math.BigDecimal;
+
+import org.objectweb.asm.Opcodes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ProxyUtils
+{
+  private static Logger LOGGER = LoggerFactory.getLogger(ProxyUtils.class);
+  /**
+   * Get the java version to be woven at.
+   * @return
+   */
+  public static int getWeavingJavaVersion() {
+    int javaClassVersion = new BigDecimal(System.getProperty("java.class.version")).intValue();
+    if (javaClassVersion >= Opcodes.V1_7) {
+      LOGGER.debug("Weaving to Java 7");
+      return Opcodes.V1_7;
+    } else {
+      LOGGER.debug("Weaving to Java 6");
+      return Opcodes.V1_6;
+    }
+    
+  }
+}

Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
==============================================================================
--- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java (original)
+++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java Thu Feb 16 22:38:27 2012
@@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
 import java.io.IOException;
 import java.lang.reflect.InvocationHandler;
 
+import org.apache.aries.proxy.impl.ProxyUtils;
 import org.objectweb.asm.AnnotationVisitor;
 import org.objectweb.asm.Attribute;
 import org.objectweb.asm.ClassReader;
@@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
       throw new TypeNotPresentException(superclassBinaryName, cnfe);
     }
 
-    // move the existing class name to become the superclass
-    // modify the version of the dynamic subclass to be Java 1.6
-    int newVersion = Opcodes.V1_6;
+
     // keep the same access and signature as the superclass (unless it's abstract)
     // remove all the superclass interfaces because they will be inherited
     // from the superclass anyway
@@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
       //If the super was abstract the subclass should not be!
       access &= ~ACC_ABSTRACT;
     }
-    cv.visit(newVersion, access, newClassName, signature, name, null);
+    cv.visit(ProxyUtils.getWeavingJavaVersion(), access, newClassName, signature, name, null);
 
     // add a private field for the invocation handler
     // this isn't static in case we have multiple instances of the same

Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
==============================================================================
--- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java (original)
+++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java Thu Feb 16 22:38:27 2012
@@ -25,6 +25,7 @@ import java.util.Collection;
 import java.util.List;
 
 import org.apache.aries.proxy.UnableToProxyException;
+import org.apache.aries.proxy.impl.ProxyUtils;
 import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
 import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
 import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
@@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
       i++;
     }
 
-    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
+    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC | ACC_SYNTHETIC, className, null,
         (superclass == null) ? AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
           Type.getInternalName(superclass), interfaceNames);
   }



Re: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Emily Jiang <em...@googlemail.com>.
Hi Tim,

I was confused with your comments that seemed to hint the code changes I
put in removed the support for java5. Hence my previous response. I did
notice the issue of not supporting java5 and wondered why no one ever asked
for this support. I spoke to Rich who put the proxy subclassing originally.
Not supporting java5 is probably a bug.

I have fixed it under the 'Java7 support' jira. Thanks for bringing it up.

Regards,
Emily

On Mon, Feb 20, 2012 at 2:10 PM, Timothy Ward <ti...@apache.org>wrote:

> Emily,
>
> The proxy impl component uses the java-5-parent, so it is definitely
> intended to work with Java 5. The current code will not pass tests using
> Java 5, and so is broken.
>
> This may be a pre-existing issue, and if you'd prefer for me to raise a
> critical JIRA bug for this then I will.
>
> In any event the code needs to be changed, your commit simply flagged it
> up and so I pointed it out. None of the code generated by any of the
> proxying should need Java 6, and I thought you'd be happy to include the
> one-liner in the code you were changing.
>
> Tim
>
> > Date: Sun, 19 Feb 2012 11:53:58 +0000
> > Subject: Re: svn commit: r1245223 - in
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> ProxyUtils.java gen/ProxySubclassAdapter.java
> interfaces/InterfaceCombiningClassAdapter.java
> > From: emijiang6@googlemail.com
> > To: dev@aries.apache.org
> >
> > Hi Tim,
> > In the previous version of the class of ProxySubClassAdapter
> > andInterfaceCombiningClassAdapter.java, both of the weaving is targeting
> to
> > java6. I cannot see how the previous code support java5 or earlier. Any
> > pointer will be apprecated.
> >
> > previous code: adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className,
> > null,...
> >
> > As for the utility class, there are some debates whether to create a
> > utility class or do some workaround to create some irrelavant class
> > dependencies, my view is to create a utility class and gradually the
> class
> > will grow.
> >
> > Regards,
> > Emily
> >
> >
> > On Sun, Feb 19, 2012 at 9:48 AM, Timothy Ward <timothyjward@apache.org
> >wrote:
> >
> > >
> > > Hi Emily,
> > >
> > > Previous releases of the proxy code have absolutely supported Java 5.
> The
> > > latest release of AbstracWovenProxyAdapter contains the following
> constants:
> > >
> > >  public static final int JAVA_CLASS_VERSION = new
> > > BigDecimal(System.getProperty("java.class.version")).intValue();
> > >  public static final boolean IS_AT_LEAST_JAVA_6 = JAVA_CLASS_VERSION >=
> > > Opcodes.V1_6;
> > >
> > >
> > > These should probably have been used rather than creating a Utility
> class
> > > from them that contains exactly that one method. The weaving code can
> read
> > > and weave classes using any bytecode version, so there's no need to
> > > artificially restrict the VMs that this code can be run on. A lot of
> people
> > > still use Java 5, and there's no good reason that this code needs to
> > > require Java 6.
> > >
> > > Regards,
> > >
> > > Tim Ward
> > > -------------------
> > > Apache Aries PMC member & Enterprise OSGi advocate
> > > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > > -------------------
> > >
> > >
> > > > Date: Fri, 17 Feb 2012 16:22:28 +0000
> > > > Subject: Re: svn commit: r1245223 - in
> > >
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > > ProxyUtils.java gen/ProxySubclassAdapter.java
> > > interfaces/InterfaceCombiningClassAdapter.java
> > > > From: emijiang6@googlemail.com
> > > > To: dev@aries.apache.org
> > > >
> > > > Previous, it only supports java 6 (as you can tell from the previous
> code
> > > > visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving
> code
> > > > has been around for a couple of years. No one asks for weaving at
> java5).
> > > >
> > > > For caching, it is a good idea. I have committed the change.
> > > > Thanks
> > > > Emily
> > > >
> > > > On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward <
> timothyjward@apache.org
> > > >wrote:
> > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > Have I missed something here? The weaving code should support Java
> 5 as
> > > > > well as Java 6 and 7, but there's no version code for it in the
> util
> > > that's
> > > > > just been committed.
> > > > >
> > > > > Also this value can be calculated and cached in a static init
> block so
> > > > > that the method can just return a constant value. This may sound
> like
> > > > > overkill, but we've had a number of people ask for every clock
> cycle
> > > we can
> > > > > save!
> > > > >
> > > > > Regards,
> > > > >
> > > > > Tim Ward
> > > > > -------------------
> > > > > Apache Aries PMC member & Enterprise OSGi advocate
> > > > > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > > > > -------------------
> > > > >
> > > > >
> > > > > > Subject: svn commit: r1245223 - in
> > > > >
> > >
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > > > > ProxyUtils.java gen/ProxySubclassAdapter.java
> > > > > interfaces/InterfaceCombiningClassAdapter.java
> > > > > > Date: Thu, 16 Feb 2012 22:38:28 +0000
> > > > > > To: commits@aries.apache.org
> > > > > > From: ejiang@apache.org
> > > > > >
> > > > > > Author: ejiang
> > > > > > Date: Thu Feb 16 22:38:27 2012
> > > > > > New Revision: 1245223
> > > > > >
> > > > > > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> > > > > > Log:
> > > > > > ARIES-817: ASM4 for Java7 support
> > > > > >
> > > > > > Added:
> > > > > >
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > > Modified:
> > > > > >
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > > >
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > > >
> > > > > > Added:
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > > URL:
> > > > >
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > ---
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > (added)
> > > > > > +++
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > Thu Feb 16 22:38:27 2012
> > > > > > @@ -0,0 +1,45 @@
> > > > > > +/*
> > > > > > + * Licensed to the Apache Software Foundation (ASF) under one
> > > > > > + * or more contributor license agreements.  See the NOTICE file
> > > > > > + * distributed with this work for additional information
> > > > > > + * regarding copyright ownership.  The ASF licenses this file
> > > > > > + * to you under the Apache License, Version 2.0 (the
> > > > > > + * "License"); you may not use this file except in compliance
> > > > > > + * with the License.  You may obtain a copy of the License at
> > > > > > + *
> > > > > > + *   http://www.apache.org/licenses/LICENSE-2.0
> > > > > > + *
> > > > > > + * Unless required by applicable law or agreed to in writing,
> > > > > > + * software distributed under the License is distributed on an
> > > > > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > > > > > + * KIND, either express or implied.  See the License for the
> > > > > > + * specific language governing permissions and limitations
> > > > > > + * under the License.
> > > > > > + */
> > > > > > +package org.apache.aries.proxy.impl;
> > > > > > +
> > > > > > +import java.math.BigDecimal;
> > > > > > +
> > > > > > +import org.objectweb.asm.Opcodes;
> > > > > > +import org.slf4j.Logger;
> > > > > > +import org.slf4j.LoggerFactory;
> > > > > > +
> > > > > > +public class ProxyUtils
> > > > > > +{
> > > > > > +  private static Logger LOGGER =
> > > > > LoggerFactory.getLogger(ProxyUtils.class);
> > > > > > +  /**
> > > > > > +   * Get the java version to be woven at.
> > > > > > +   * @return
> > > > > > +   */
> > > > > > +  public static int getWeavingJavaVersion() {
> > > > > > +    int javaClassVersion = new
> > > > > BigDecimal(System.getProperty("java.class.version")).intValue();
> > > > > > +    if (javaClassVersion >= Opcodes.V1_7) {
> > > > > > +      LOGGER.debug("Weaving to Java 7");
> > > > > > +      return Opcodes.V1_7;
> > > > > > +    } else {
> > > > > > +      LOGGER.debug("Weaving to Java 6");
> > > > > > +      return Opcodes.V1_6;
> > > > > > +    }
> > > > > > +
> > > > > > +  }
> > > > > > +}
> > > > > >
> > > > > > Modified:
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > > > URL:
> > > > >
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > ---
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > > (original)
> > > > > > +++
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > > Thu Feb 16 22:38:27 2012
> > > > > > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
> > > > > >  import java.io.IOException;
> > > > > >  import java.lang.reflect.InvocationHandler;
> > > > > >
> > > > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > > > >  import org.objectweb.asm.AnnotationVisitor;
> > > > > >  import org.objectweb.asm.Attribute;
> > > > > >  import org.objectweb.asm.ClassReader;
> > > > > > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
> > > > > >        throw new TypeNotPresentException(superclassBinaryName,
> cnfe);
> > > > > >      }
> > > > > >
> > > > > > -    // move the existing class name to become the superclass
> > > > > > -    // modify the version of the dynamic subclass to be Java 1.6
> > > > > > -    int newVersion = Opcodes.V1_6;
> > > > > > +
> > > > > >      // keep the same access and signature as the superclass
> (unless
> > > > > it's abstract)
> > > > > >      // remove all the superclass interfaces because they will be
> > > > > inherited
> > > > > >      // from the superclass anyway
> > > > > > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
> > > > > >        //If the super was abstract the subclass should not be!
> > > > > >        access &= ~ACC_ABSTRACT;
> > > > > >      }
> > > > > > -    cv.visit(newVersion, access, newClassName, signature, name,
> > > null);
> > > > > > +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access,
> > > newClassName,
> > > > > signature, name, null);
> > > > > >
> > > > > >      // add a private field for the invocation handler
> > > > > >      // this isn't static in case we have multiple instances of
> the
> > > same
> > > > > >
> > > > > > Modified:
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > > > URL:
> > > > >
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > ---
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > > (original)
> > > > > > +++
> > > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > > Thu Feb 16 22:38:27 2012
> > > > > > @@ -25,6 +25,7 @@ import java.util.Collection;
> > > > > >  import java.util.List;
> > > > > >
> > > > > >  import org.apache.aries.proxy.UnableToProxyException;
> > > > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > > > >  import
> org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
> > > > > >  import
> org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
> > > > > >  import
> org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> > > > > > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
> > > > > >        i++;
> > > > > >      }
> > > > > >
> > > > > > -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className,
> null,
> > > > > > +    adapter.visit(ProxyUtils.getWeavingJavaVersion(),
> ACC_PUBLIC |
> > > > > ACC_SYNTHETIC, className, null,
> > > > > >          (superclass == null) ?
> > > > > AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
> > > > > >            Type.getInternalName(superclass), interfaceNames);
> > > > > >    }
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Thanks
> > > > Emily
> > > > =================
> > > > Emily Jiang
> > > > ejiang@apache.org
> > >
> > >
> >
> >
> >
> > --
> > Thanks
> > Emily
> > =================
> > Emily Jiang
> > ejiang@apache.org
>
>



-- 
Thanks
Emily
=================
Emily Jiang
ejiang@apache.org

RE: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Timothy Ward <ti...@apache.org>.
Emily,

The proxy impl component uses the java-5-parent, so it is definitely intended to work with Java 5. The current code will not pass tests using Java 5, and so is broken. 

This may be a pre-existing issue, and if you'd prefer for me to raise a critical JIRA bug for this then I will. 

In any event the code needs to be changed, your commit simply flagged it up and so I pointed it out. None of the code generated by any of the proxying should need Java 6, and I thought you'd be happy to include the one-liner in the code you were changing.

Tim

> Date: Sun, 19 Feb 2012 11:53:58 +0000
> Subject: Re: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java
> From: emijiang6@googlemail.com
> To: dev@aries.apache.org
> 
> Hi Tim,
> In the previous version of the class of ProxySubClassAdapter
> andInterfaceCombiningClassAdapter.java, both of the weaving is targeting to
> java6. I cannot see how the previous code support java5 or earlier. Any
> pointer will be apprecated.
> 
> previous code: adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className,
> null,...
> 
> As for the utility class, there are some debates whether to create a
> utility class or do some workaround to create some irrelavant class
> dependencies, my view is to create a utility class and gradually the class
> will grow.
> 
> Regards,
> Emily
> 
> 
> On Sun, Feb 19, 2012 at 9:48 AM, Timothy Ward <ti...@apache.org>wrote:
> 
> >
> > Hi Emily,
> >
> > Previous releases of the proxy code have absolutely supported Java 5. The
> > latest release of AbstracWovenProxyAdapter contains the following constants:
> >
> >  public static final int JAVA_CLASS_VERSION = new
> > BigDecimal(System.getProperty("java.class.version")).intValue();
> >  public static final boolean IS_AT_LEAST_JAVA_6 = JAVA_CLASS_VERSION >=
> > Opcodes.V1_6;
> >
> >
> > These should probably have been used rather than creating a Utility class
> > from them that contains exactly that one method. The weaving code can read
> > and weave classes using any bytecode version, so there's no need to
> > artificially restrict the VMs that this code can be run on. A lot of people
> > still use Java 5, and there's no good reason that this code needs to
> > require Java 6.
> >
> > Regards,
> >
> > Tim Ward
> > -------------------
> > Apache Aries PMC member & Enterprise OSGi advocate
> > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > -------------------
> >
> >
> > > Date: Fri, 17 Feb 2012 16:22:28 +0000
> > > Subject: Re: svn commit: r1245223 - in
> > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > ProxyUtils.java gen/ProxySubclassAdapter.java
> > interfaces/InterfaceCombiningClassAdapter.java
> > > From: emijiang6@googlemail.com
> > > To: dev@aries.apache.org
> > >
> > > Previous, it only supports java 6 (as you can tell from the previous code
> > > visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving code
> > > has been around for a couple of years. No one asks for weaving at java5).
> > >
> > > For caching, it is a good idea. I have committed the change.
> > > Thanks
> > > Emily
> > >
> > > On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward <timothyjward@apache.org
> > >wrote:
> > >
> > > >
> > > > Hi,
> > > >
> > > > Have I missed something here? The weaving code should support Java 5 as
> > > > well as Java 6 and 7, but there's no version code for it in the util
> > that's
> > > > just been committed.
> > > >
> > > > Also this value can be calculated and cached in a static init block so
> > > > that the method can just return a constant value. This may sound like
> > > > overkill, but we've had a number of people ask for every clock cycle
> > we can
> > > > save!
> > > >
> > > > Regards,
> > > >
> > > > Tim Ward
> > > > -------------------
> > > > Apache Aries PMC member & Enterprise OSGi advocate
> > > > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > > > -------------------
> > > >
> > > >
> > > > > Subject: svn commit: r1245223 - in
> > > >
> > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > > > ProxyUtils.java gen/ProxySubclassAdapter.java
> > > > interfaces/InterfaceCombiningClassAdapter.java
> > > > > Date: Thu, 16 Feb 2012 22:38:28 +0000
> > > > > To: commits@aries.apache.org
> > > > > From: ejiang@apache.org
> > > > >
> > > > > Author: ejiang
> > > > > Date: Thu Feb 16 22:38:27 2012
> > > > > New Revision: 1245223
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> > > > > Log:
> > > > > ARIES-817: ASM4 for Java7 support
> > > > >
> > > > > Added:
> > > > >
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > Modified:
> > > > >
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > >
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > >
> > > > > Added:
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > > URL:
> > > >
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> > > > >
> > > >
> > ==============================================================================
> > > > > ---
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > (added)
> > > > > +++
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > Thu Feb 16 22:38:27 2012
> > > > > @@ -0,0 +1,45 @@
> > > > > +/*
> > > > > + * Licensed to the Apache Software Foundation (ASF) under one
> > > > > + * or more contributor license agreements.  See the NOTICE file
> > > > > + * distributed with this work for additional information
> > > > > + * regarding copyright ownership.  The ASF licenses this file
> > > > > + * to you under the Apache License, Version 2.0 (the
> > > > > + * "License"); you may not use this file except in compliance
> > > > > + * with the License.  You may obtain a copy of the License at
> > > > > + *
> > > > > + *   http://www.apache.org/licenses/LICENSE-2.0
> > > > > + *
> > > > > + * Unless required by applicable law or agreed to in writing,
> > > > > + * software distributed under the License is distributed on an
> > > > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > > > > + * KIND, either express or implied.  See the License for the
> > > > > + * specific language governing permissions and limitations
> > > > > + * under the License.
> > > > > + */
> > > > > +package org.apache.aries.proxy.impl;
> > > > > +
> > > > > +import java.math.BigDecimal;
> > > > > +
> > > > > +import org.objectweb.asm.Opcodes;
> > > > > +import org.slf4j.Logger;
> > > > > +import org.slf4j.LoggerFactory;
> > > > > +
> > > > > +public class ProxyUtils
> > > > > +{
> > > > > +  private static Logger LOGGER =
> > > > LoggerFactory.getLogger(ProxyUtils.class);
> > > > > +  /**
> > > > > +   * Get the java version to be woven at.
> > > > > +   * @return
> > > > > +   */
> > > > > +  public static int getWeavingJavaVersion() {
> > > > > +    int javaClassVersion = new
> > > > BigDecimal(System.getProperty("java.class.version")).intValue();
> > > > > +    if (javaClassVersion >= Opcodes.V1_7) {
> > > > > +      LOGGER.debug("Weaving to Java 7");
> > > > > +      return Opcodes.V1_7;
> > > > > +    } else {
> > > > > +      LOGGER.debug("Weaving to Java 6");
> > > > > +      return Opcodes.V1_6;
> > > > > +    }
> > > > > +
> > > > > +  }
> > > > > +}
> > > > >
> > > > > Modified:
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > > URL:
> > > >
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > > >
> > > >
> > ==============================================================================
> > > > > ---
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > (original)
> > > > > +++
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > Thu Feb 16 22:38:27 2012
> > > > > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
> > > > >  import java.io.IOException;
> > > > >  import java.lang.reflect.InvocationHandler;
> > > > >
> > > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > > >  import org.objectweb.asm.AnnotationVisitor;
> > > > >  import org.objectweb.asm.Attribute;
> > > > >  import org.objectweb.asm.ClassReader;
> > > > > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
> > > > >        throw new TypeNotPresentException(superclassBinaryName, cnfe);
> > > > >      }
> > > > >
> > > > > -    // move the existing class name to become the superclass
> > > > > -    // modify the version of the dynamic subclass to be Java 1.6
> > > > > -    int newVersion = Opcodes.V1_6;
> > > > > +
> > > > >      // keep the same access and signature as the superclass (unless
> > > > it's abstract)
> > > > >      // remove all the superclass interfaces because they will be
> > > > inherited
> > > > >      // from the superclass anyway
> > > > > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
> > > > >        //If the super was abstract the subclass should not be!
> > > > >        access &= ~ACC_ABSTRACT;
> > > > >      }
> > > > > -    cv.visit(newVersion, access, newClassName, signature, name,
> > null);
> > > > > +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access,
> > newClassName,
> > > > signature, name, null);
> > > > >
> > > > >      // add a private field for the invocation handler
> > > > >      // this isn't static in case we have multiple instances of the
> > same
> > > > >
> > > > > Modified:
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > > URL:
> > > >
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > > >
> > > >
> > ==============================================================================
> > > > > ---
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > (original)
> > > > > +++
> > > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > Thu Feb 16 22:38:27 2012
> > > > > @@ -25,6 +25,7 @@ import java.util.Collection;
> > > > >  import java.util.List;
> > > > >
> > > > >  import org.apache.aries.proxy.UnableToProxyException;
> > > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > > >  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
> > > > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
> > > > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> > > > > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
> > > > >        i++;
> > > > >      }
> > > > >
> > > > > -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> > > > > +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC |
> > > > ACC_SYNTHETIC, className, null,
> > > > >          (superclass == null) ?
> > > > AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
> > > > >            Type.getInternalName(superclass), interfaceNames);
> > > > >    }
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Thanks
> > > Emily
> > > =================
> > > Emily Jiang
> > > ejiang@apache.org
> >
> >
> 
> 
> 
> -- 
> Thanks
> Emily
> =================
> Emily Jiang
> ejiang@apache.org
 		 	   		  

Re: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Emily Jiang <em...@googlemail.com>.
Hi Tim,
In the previous version of the class of ProxySubClassAdapter
andInterfaceCombiningClassAdapter.java, both of the weaving is targeting to
java6. I cannot see how the previous code support java5 or earlier. Any
pointer will be apprecated.

previous code: adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className,
null,...

As for the utility class, there are some debates whether to create a
utility class or do some workaround to create some irrelavant class
dependencies, my view is to create a utility class and gradually the class
will grow.

Regards,
Emily


On Sun, Feb 19, 2012 at 9:48 AM, Timothy Ward <ti...@apache.org>wrote:

>
> Hi Emily,
>
> Previous releases of the proxy code have absolutely supported Java 5. The
> latest release of AbstracWovenProxyAdapter contains the following constants:
>
>  public static final int JAVA_CLASS_VERSION = new
> BigDecimal(System.getProperty("java.class.version")).intValue();
>  public static final boolean IS_AT_LEAST_JAVA_6 = JAVA_CLASS_VERSION >=
> Opcodes.V1_6;
>
>
> These should probably have been used rather than creating a Utility class
> from them that contains exactly that one method. The weaving code can read
> and weave classes using any bytecode version, so there's no need to
> artificially restrict the VMs that this code can be run on. A lot of people
> still use Java 5, and there's no good reason that this code needs to
> require Java 6.
>
> Regards,
>
> Tim Ward
> -------------------
> Apache Aries PMC member & Enterprise OSGi advocate
> Enterprise OSGi in Action (http://www.manning.com/cummins)
> -------------------
>
>
> > Date: Fri, 17 Feb 2012 16:22:28 +0000
> > Subject: Re: svn commit: r1245223 - in
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> ProxyUtils.java gen/ProxySubclassAdapter.java
> interfaces/InterfaceCombiningClassAdapter.java
> > From: emijiang6@googlemail.com
> > To: dev@aries.apache.org
> >
> > Previous, it only supports java 6 (as you can tell from the previous code
> > visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving code
> > has been around for a couple of years. No one asks for weaving at java5).
> >
> > For caching, it is a good idea. I have committed the change.
> > Thanks
> > Emily
> >
> > On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward <timothyjward@apache.org
> >wrote:
> >
> > >
> > > Hi,
> > >
> > > Have I missed something here? The weaving code should support Java 5 as
> > > well as Java 6 and 7, but there's no version code for it in the util
> that's
> > > just been committed.
> > >
> > > Also this value can be calculated and cached in a static init block so
> > > that the method can just return a constant value. This may sound like
> > > overkill, but we've had a number of people ask for every clock cycle
> we can
> > > save!
> > >
> > > Regards,
> > >
> > > Tim Ward
> > > -------------------
> > > Apache Aries PMC member & Enterprise OSGi advocate
> > > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > > -------------------
> > >
> > >
> > > > Subject: svn commit: r1245223 - in
> > >
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > > ProxyUtils.java gen/ProxySubclassAdapter.java
> > > interfaces/InterfaceCombiningClassAdapter.java
> > > > Date: Thu, 16 Feb 2012 22:38:28 +0000
> > > > To: commits@aries.apache.org
> > > > From: ejiang@apache.org
> > > >
> > > > Author: ejiang
> > > > Date: Thu Feb 16 22:38:27 2012
> > > > New Revision: 1245223
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> > > > Log:
> > > > ARIES-817: ASM4 for Java7 support
> > > >
> > > > Added:
> > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > Modified:
> > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > >
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > >
> > > > Added:
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> > > >
> > >
> ==============================================================================
> > > > ---
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > (added)
> > > > +++
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > Thu Feb 16 22:38:27 2012
> > > > @@ -0,0 +1,45 @@
> > > > +/*
> > > > + * Licensed to the Apache Software Foundation (ASF) under one
> > > > + * or more contributor license agreements.  See the NOTICE file
> > > > + * distributed with this work for additional information
> > > > + * regarding copyright ownership.  The ASF licenses this file
> > > > + * to you under the Apache License, Version 2.0 (the
> > > > + * "License"); you may not use this file except in compliance
> > > > + * with the License.  You may obtain a copy of the License at
> > > > + *
> > > > + *   http://www.apache.org/licenses/LICENSE-2.0
> > > > + *
> > > > + * Unless required by applicable law or agreed to in writing,
> > > > + * software distributed under the License is distributed on an
> > > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > > > + * KIND, either express or implied.  See the License for the
> > > > + * specific language governing permissions and limitations
> > > > + * under the License.
> > > > + */
> > > > +package org.apache.aries.proxy.impl;
> > > > +
> > > > +import java.math.BigDecimal;
> > > > +
> > > > +import org.objectweb.asm.Opcodes;
> > > > +import org.slf4j.Logger;
> > > > +import org.slf4j.LoggerFactory;
> > > > +
> > > > +public class ProxyUtils
> > > > +{
> > > > +  private static Logger LOGGER =
> > > LoggerFactory.getLogger(ProxyUtils.class);
> > > > +  /**
> > > > +   * Get the java version to be woven at.
> > > > +   * @return
> > > > +   */
> > > > +  public static int getWeavingJavaVersion() {
> > > > +    int javaClassVersion = new
> > > BigDecimal(System.getProperty("java.class.version")).intValue();
> > > > +    if (javaClassVersion >= Opcodes.V1_7) {
> > > > +      LOGGER.debug("Weaving to Java 7");
> > > > +      return Opcodes.V1_7;
> > > > +    } else {
> > > > +      LOGGER.debug("Weaving to Java 6");
> > > > +      return Opcodes.V1_6;
> > > > +    }
> > > > +
> > > > +  }
> > > > +}
> > > >
> > > > Modified:
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > >
> > >
> ==============================================================================
> > > > ---
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > (original)
> > > > +++
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > Thu Feb 16 22:38:27 2012
> > > > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
> > > >  import java.io.IOException;
> > > >  import java.lang.reflect.InvocationHandler;
> > > >
> > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > >  import org.objectweb.asm.AnnotationVisitor;
> > > >  import org.objectweb.asm.Attribute;
> > > >  import org.objectweb.asm.ClassReader;
> > > > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
> > > >        throw new TypeNotPresentException(superclassBinaryName, cnfe);
> > > >      }
> > > >
> > > > -    // move the existing class name to become the superclass
> > > > -    // modify the version of the dynamic subclass to be Java 1.6
> > > > -    int newVersion = Opcodes.V1_6;
> > > > +
> > > >      // keep the same access and signature as the superclass (unless
> > > it's abstract)
> > > >      // remove all the superclass interfaces because they will be
> > > inherited
> > > >      // from the superclass anyway
> > > > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
> > > >        //If the super was abstract the subclass should not be!
> > > >        access &= ~ACC_ABSTRACT;
> > > >      }
> > > > -    cv.visit(newVersion, access, newClassName, signature, name,
> null);
> > > > +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access,
> newClassName,
> > > signature, name, null);
> > > >
> > > >      // add a private field for the invocation handler
> > > >      // this isn't static in case we have multiple instances of the
> same
> > > >
> > > > Modified:
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > > URL:
> > >
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > > >
> > >
> ==============================================================================
> > > > ---
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > (original)
> > > > +++
> > >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > Thu Feb 16 22:38:27 2012
> > > > @@ -25,6 +25,7 @@ import java.util.Collection;
> > > >  import java.util.List;
> > > >
> > > >  import org.apache.aries.proxy.UnableToProxyException;
> > > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > > >  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
> > > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
> > > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> > > > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
> > > >        i++;
> > > >      }
> > > >
> > > > -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> > > > +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC |
> > > ACC_SYNTHETIC, className, null,
> > > >          (superclass == null) ?
> > > AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
> > > >            Type.getInternalName(superclass), interfaceNames);
> > > >    }
> > > >
> > > >
> > >
> > >
> >
> >
> >
> > --
> > Thanks
> > Emily
> > =================
> > Emily Jiang
> > ejiang@apache.org
>
>



-- 
Thanks
Emily
=================
Emily Jiang
ejiang@apache.org

RE: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Timothy Ward <ti...@apache.org>.
Hi Emily,

Previous releases of the proxy code have absolutely supported Java 5. The latest release of AbstracWovenProxyAdapter contains the following constants:

  public static final int JAVA_CLASS_VERSION = new BigDecimal(System.getProperty("java.class.version")).intValue();
  public static final boolean IS_AT_LEAST_JAVA_6 = JAVA_CLASS_VERSION >= Opcodes.V1_6;


These should probably have been used rather than creating a Utility class from them that contains exactly that one method. The weaving code can read and weave classes using any bytecode version, so there's no need to artificially restrict the VMs that this code can be run on. A lot of people still use Java 5, and there's no good reason that this code needs to require Java 6.

Regards,

Tim Ward
-------------------
Apache Aries PMC member & Enterprise OSGi advocate
Enterprise OSGi in Action (http://www.manning.com/cummins)
-------------------


> Date: Fri, 17 Feb 2012 16:22:28 +0000
> Subject: Re: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java
> From: emijiang6@googlemail.com
> To: dev@aries.apache.org
> 
> Previous, it only supports java 6 (as you can tell from the previous code
> visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving code
> has been around for a couple of years. No one asks for weaving at java5).
> 
> For caching, it is a good idea. I have committed the change.
> Thanks
> Emily
> 
> On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward <ti...@apache.org>wrote:
> 
> >
> > Hi,
> >
> > Have I missed something here? The weaving code should support Java 5 as
> > well as Java 6 and 7, but there's no version code for it in the util that's
> > just been committed.
> >
> > Also this value can be calculated and cached in a static init block so
> > that the method can just return a constant value. This may sound like
> > overkill, but we've had a number of people ask for every clock cycle we can
> > save!
> >
> > Regards,
> >
> > Tim Ward
> > -------------------
> > Apache Aries PMC member & Enterprise OSGi advocate
> > Enterprise OSGi in Action (http://www.manning.com/cummins)
> > -------------------
> >
> >
> > > Subject: svn commit: r1245223 - in
> > /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> > ProxyUtils.java gen/ProxySubclassAdapter.java
> > interfaces/InterfaceCombiningClassAdapter.java
> > > Date: Thu, 16 Feb 2012 22:38:28 +0000
> > > To: commits@aries.apache.org
> > > From: ejiang@apache.org
> > >
> > > Author: ejiang
> > > Date: Thu Feb 16 22:38:27 2012
> > > New Revision: 1245223
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> > > Log:
> > > ARIES-817: ASM4 for Java7 support
> > >
> > > Added:
> > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > Modified:
> > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > >
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > >
> > > Added:
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > > URL:
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> > >
> > ==============================================================================
> > > ---
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > (added)
> > > +++
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > Thu Feb 16 22:38:27 2012
> > > @@ -0,0 +1,45 @@
> > > +/*
> > > + * Licensed to the Apache Software Foundation (ASF) under one
> > > + * or more contributor license agreements.  See the NOTICE file
> > > + * distributed with this work for additional information
> > > + * regarding copyright ownership.  The ASF licenses this file
> > > + * to you under the Apache License, Version 2.0 (the
> > > + * "License"); you may not use this file except in compliance
> > > + * with the License.  You may obtain a copy of the License at
> > > + *
> > > + *   http://www.apache.org/licenses/LICENSE-2.0
> > > + *
> > > + * Unless required by applicable law or agreed to in writing,
> > > + * software distributed under the License is distributed on an
> > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > > + * KIND, either express or implied.  See the License for the
> > > + * specific language governing permissions and limitations
> > > + * under the License.
> > > + */
> > > +package org.apache.aries.proxy.impl;
> > > +
> > > +import java.math.BigDecimal;
> > > +
> > > +import org.objectweb.asm.Opcodes;
> > > +import org.slf4j.Logger;
> > > +import org.slf4j.LoggerFactory;
> > > +
> > > +public class ProxyUtils
> > > +{
> > > +  private static Logger LOGGER =
> > LoggerFactory.getLogger(ProxyUtils.class);
> > > +  /**
> > > +   * Get the java version to be woven at.
> > > +   * @return
> > > +   */
> > > +  public static int getWeavingJavaVersion() {
> > > +    int javaClassVersion = new
> > BigDecimal(System.getProperty("java.class.version")).intValue();
> > > +    if (javaClassVersion >= Opcodes.V1_7) {
> > > +      LOGGER.debug("Weaving to Java 7");
> > > +      return Opcodes.V1_7;
> > > +    } else {
> > > +      LOGGER.debug("Weaving to Java 6");
> > > +      return Opcodes.V1_6;
> > > +    }
> > > +
> > > +  }
> > > +}
> > >
> > > Modified:
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > > URL:
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > >
> > ==============================================================================
> > > ---
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > (original)
> > > +++
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > Thu Feb 16 22:38:27 2012
> > > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
> > >  import java.io.IOException;
> > >  import java.lang.reflect.InvocationHandler;
> > >
> > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > >  import org.objectweb.asm.AnnotationVisitor;
> > >  import org.objectweb.asm.Attribute;
> > >  import org.objectweb.asm.ClassReader;
> > > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
> > >        throw new TypeNotPresentException(superclassBinaryName, cnfe);
> > >      }
> > >
> > > -    // move the existing class name to become the superclass
> > > -    // modify the version of the dynamic subclass to be Java 1.6
> > > -    int newVersion = Opcodes.V1_6;
> > > +
> > >      // keep the same access and signature as the superclass (unless
> > it's abstract)
> > >      // remove all the superclass interfaces because they will be
> > inherited
> > >      // from the superclass anyway
> > > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
> > >        //If the super was abstract the subclass should not be!
> > >        access &= ~ACC_ABSTRACT;
> > >      }
> > > -    cv.visit(newVersion, access, newClassName, signature, name, null);
> > > +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access, newClassName,
> > signature, name, null);
> > >
> > >      // add a private field for the invocation handler
> > >      // this isn't static in case we have multiple instances of the same
> > >
> > > Modified:
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > > URL:
> > http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> > >
> > ==============================================================================
> > > ---
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > (original)
> > > +++
> > aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > Thu Feb 16 22:38:27 2012
> > > @@ -25,6 +25,7 @@ import java.util.Collection;
> > >  import java.util.List;
> > >
> > >  import org.apache.aries.proxy.UnableToProxyException;
> > > +import org.apache.aries.proxy.impl.ProxyUtils;
> > >  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
> > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
> > >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> > > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
> > >        i++;
> > >      }
> > >
> > > -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> > > +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC |
> > ACC_SYNTHETIC, className, null,
> > >          (superclass == null) ?
> > AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
> > >            Type.getInternalName(superclass), interfaceNames);
> > >    }
> > >
> > >
> >
> >
> 
> 
> 
> -- 
> Thanks
> Emily
> =================
> Emily Jiang
> ejiang@apache.org
 		 	   		  

Re: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Emily Jiang <em...@googlemail.com>.
Previous, it only supports java 6 (as you can tell from the previous code
visit(Opcodes.V1_6, ...), so I don't think anyone use it (the weving code
has been around for a couple of years. No one asks for weaving at java5).

For caching, it is a good idea. I have committed the change.
Thanks
Emily

On Fri, Feb 17, 2012 at 11:50 AM, Timothy Ward <ti...@apache.org>wrote:

>
> Hi,
>
> Have I missed something here? The weaving code should support Java 5 as
> well as Java 6 and 7, but there's no version code for it in the util that's
> just been committed.
>
> Also this value can be calculated and cached in a static init block so
> that the method can just return a constant value. This may sound like
> overkill, but we've had a number of people ask for every clock cycle we can
> save!
>
> Regards,
>
> Tim Ward
> -------------------
> Apache Aries PMC member & Enterprise OSGi advocate
> Enterprise OSGi in Action (http://www.manning.com/cummins)
> -------------------
>
>
> > Subject: svn commit: r1245223 - in
> /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl:
> ProxyUtils.java gen/ProxySubclassAdapter.java
> interfaces/InterfaceCombiningClassAdapter.java
> > Date: Thu, 16 Feb 2012 22:38:28 +0000
> > To: commits@aries.apache.org
> > From: ejiang@apache.org
> >
> > Author: ejiang
> > Date: Thu Feb 16 22:38:27 2012
> > New Revision: 1245223
> >
> > URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> > Log:
> > ARIES-817: ASM4 for Java7 support
> >
> > Added:
> >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > Modified:
> >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> >
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> >
> > Added:
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> > URL:
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> >
> ==============================================================================
> > ---
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> (added)
> > +++
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> Thu Feb 16 22:38:27 2012
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Licensed to the Apache Software Foundation (ASF) under one
> > + * or more contributor license agreements.  See the NOTICE file
> > + * distributed with this work for additional information
> > + * regarding copyright ownership.  The ASF licenses this file
> > + * to you under the Apache License, Version 2.0 (the
> > + * "License"); you may not use this file except in compliance
> > + * with the License.  You may obtain a copy of the License at
> > + *
> > + *   http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing,
> > + * software distributed under the License is distributed on an
> > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > + * KIND, either express or implied.  See the License for the
> > + * specific language governing permissions and limitations
> > + * under the License.
> > + */
> > +package org.apache.aries.proxy.impl;
> > +
> > +import java.math.BigDecimal;
> > +
> > +import org.objectweb.asm.Opcodes;
> > +import org.slf4j.Logger;
> > +import org.slf4j.LoggerFactory;
> > +
> > +public class ProxyUtils
> > +{
> > +  private static Logger LOGGER =
> LoggerFactory.getLogger(ProxyUtils.class);
> > +  /**
> > +   * Get the java version to be woven at.
> > +   * @return
> > +   */
> > +  public static int getWeavingJavaVersion() {
> > +    int javaClassVersion = new
> BigDecimal(System.getProperty("java.class.version")).intValue();
> > +    if (javaClassVersion >= Opcodes.V1_7) {
> > +      LOGGER.debug("Weaving to Java 7");
> > +      return Opcodes.V1_7;
> > +    } else {
> > +      LOGGER.debug("Weaving to Java 6");
> > +      return Opcodes.V1_6;
> > +    }
> > +
> > +  }
> > +}
> >
> > Modified:
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> > URL:
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> >
> ==============================================================================
> > ---
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> (original)
> > +++
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> Thu Feb 16 22:38:27 2012
> > @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
> >  import java.io.IOException;
> >  import java.lang.reflect.InvocationHandler;
> >
> > +import org.apache.aries.proxy.impl.ProxyUtils;
> >  import org.objectweb.asm.AnnotationVisitor;
> >  import org.objectweb.asm.Attribute;
> >  import org.objectweb.asm.ClassReader;
> > @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
> >        throw new TypeNotPresentException(superclassBinaryName, cnfe);
> >      }
> >
> > -    // move the existing class name to become the superclass
> > -    // modify the version of the dynamic subclass to be Java 1.6
> > -    int newVersion = Opcodes.V1_6;
> > +
> >      // keep the same access and signature as the superclass (unless
> it's abstract)
> >      // remove all the superclass interfaces because they will be
> inherited
> >      // from the superclass anyway
> > @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
> >        //If the super was abstract the subclass should not be!
> >        access &= ~ACC_ABSTRACT;
> >      }
> > -    cv.visit(newVersion, access, newClassName, signature, name, null);
> > +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access, newClassName,
> signature, name, null);
> >
> >      // add a private field for the invocation handler
> >      // this isn't static in case we have multiple instances of the same
> >
> > Modified:
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> > URL:
> http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> >
> ==============================================================================
> > ---
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> (original)
> > +++
> aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> Thu Feb 16 22:38:27 2012
> > @@ -25,6 +25,7 @@ import java.util.Collection;
> >  import java.util.List;
> >
> >  import org.apache.aries.proxy.UnableToProxyException;
> > +import org.apache.aries.proxy.impl.ProxyUtils;
> >  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
> >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
> >  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> > @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
> >        i++;
> >      }
> >
> > -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> > +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC |
> ACC_SYNTHETIC, className, null,
> >          (superclass == null) ?
> AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
> >            Type.getInternalName(superclass), interfaceNames);
> >    }
> >
> >
>
>



-- 
Thanks
Emily
=================
Emily Jiang
ejiang@apache.org

RE: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Timothy Ward <ti...@apache.org>.
Hi,

Have I missed something here? The weaving code should support Java 5 as well as Java 6 and 7, but there's no version code for it in the util that's just been committed.

Also this value can be calculated and cached in a static init block so that the method can just return a constant value. This may sound like overkill, but we've had a number of people ask for every clock cycle we can save!

Regards,

Tim Ward
-------------------
Apache Aries PMC member & Enterprise OSGi advocate
Enterprise OSGi in Action (http://www.manning.com/cummins)
-------------------


> Subject: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java
> Date: Thu, 16 Feb 2012 22:38:28 +0000
> To: commits@aries.apache.org
> From: ejiang@apache.org
> 
> Author: ejiang
> Date: Thu Feb 16 22:38:27 2012
> New Revision: 1245223
> 
> URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> Log:
> ARIES-817: ASM4 for Java7 support
> 
> Added:
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> Modified:
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> 
> Added: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java (added)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java Thu Feb 16 22:38:27 2012
> @@ -0,0 +1,45 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.apache.aries.proxy.impl;
> +
> +import java.math.BigDecimal;
> +
> +import org.objectweb.asm.Opcodes;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +public class ProxyUtils
> +{
> +  private static Logger LOGGER = LoggerFactory.getLogger(ProxyUtils.class);
> +  /**
> +   * Get the java version to be woven at.
> +   * @return
> +   */
> +  public static int getWeavingJavaVersion() {
> +    int javaClassVersion = new BigDecimal(System.getProperty("java.class.version")).intValue();
> +    if (javaClassVersion >= Opcodes.V1_7) {
> +      LOGGER.debug("Weaving to Java 7");
> +      return Opcodes.V1_7;
> +    } else {
> +      LOGGER.debug("Weaving to Java 6");
> +      return Opcodes.V1_6;
> +    }
> +    
> +  }
> +}
> 
> Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java (original)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java Thu Feb 16 22:38:27 2012
> @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
>  import java.io.IOException;
>  import java.lang.reflect.InvocationHandler;
>  
> +import org.apache.aries.proxy.impl.ProxyUtils;
>  import org.objectweb.asm.AnnotationVisitor;
>  import org.objectweb.asm.Attribute;
>  import org.objectweb.asm.ClassReader;
> @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
>        throw new TypeNotPresentException(superclassBinaryName, cnfe);
>      }
>  
> -    // move the existing class name to become the superclass
> -    // modify the version of the dynamic subclass to be Java 1.6
> -    int newVersion = Opcodes.V1_6;
> +
>      // keep the same access and signature as the superclass (unless it's abstract)
>      // remove all the superclass interfaces because they will be inherited
>      // from the superclass anyway
> @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
>        //If the super was abstract the subclass should not be!
>        access &= ~ACC_ABSTRACT;
>      }
> -    cv.visit(newVersion, access, newClassName, signature, name, null);
> +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access, newClassName, signature, name, null);
>  
>      // add a private field for the invocation handler
>      // this isn't static in case we have multiple instances of the same
> 
> Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java (original)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java Thu Feb 16 22:38:27 2012
> @@ -25,6 +25,7 @@ import java.util.Collection;
>  import java.util.List;
>  
>  import org.apache.aries.proxy.UnableToProxyException;
> +import org.apache.aries.proxy.impl.ProxyUtils;
>  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
>  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
>  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
>        i++;
>      }
>  
> -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC | ACC_SYNTHETIC, className, null,
>          (superclass == null) ? AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
>            Type.getInternalName(superclass), interfaceNames);
>    }
> 
> 
 		 	   		  

RE: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java

Posted by Timothy Ward <ti...@apache.org>.
Hi,

Have I missed something here? The weaving code should support Java 5 as well as Java 6 and 7, but there's no version code for it in the util that's just been committed.

Also this value can be calculated and cached in a static init block so that the method can just return a constant value. This may sound like overkill, but we've had a number of people ask for every clock cycle we can save!

Regards,

Tim Ward
-------------------
Apache Aries PMC member & Enterprise OSGi advocate
Enterprise OSGi in Action (http://www.manning.com/cummins)
-------------------


> Subject: svn commit: r1245223 - in /aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl: ProxyUtils.java gen/ProxySubclassAdapter.java interfaces/InterfaceCombiningClassAdapter.java
> Date: Thu, 16 Feb 2012 22:38:28 +0000
> To: commits@aries.apache.org
> From: ejiang@apache.org
> 
> Author: ejiang
> Date: Thu Feb 16 22:38:27 2012
> New Revision: 1245223
> 
> URL: http://svn.apache.org/viewvc?rev=1245223&view=rev
> Log:
> ARIES-817: ASM4 for Java7 support
> 
> Added:
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> Modified:
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
>     aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> 
> Added: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java?rev=1245223&view=auto
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java (added)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/ProxyUtils.java Thu Feb 16 22:38:27 2012
> @@ -0,0 +1,45 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one
> + * or more contributor license agreements.  See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership.  The ASF licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License.  You may obtain a copy of the License at
> + *
> + *   http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied.  See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.apache.aries.proxy.impl;
> +
> +import java.math.BigDecimal;
> +
> +import org.objectweb.asm.Opcodes;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +public class ProxyUtils
> +{
> +  private static Logger LOGGER = LoggerFactory.getLogger(ProxyUtils.class);
> +  /**
> +   * Get the java version to be woven at.
> +   * @return
> +   */
> +  public static int getWeavingJavaVersion() {
> +    int javaClassVersion = new BigDecimal(System.getProperty("java.class.version")).intValue();
> +    if (javaClassVersion >= Opcodes.V1_7) {
> +      LOGGER.debug("Weaving to Java 7");
> +      return Opcodes.V1_7;
> +    } else {
> +      LOGGER.debug("Weaving to Java 6");
> +      return Opcodes.V1_6;
> +    }
> +    
> +  }
> +}
> 
> Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java (original)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/gen/ProxySubclassAdapter.java Thu Feb 16 22:38:27 2012
> @@ -21,6 +21,7 @@ package org.apache.aries.proxy.impl.gen;
>  import java.io.IOException;
>  import java.lang.reflect.InvocationHandler;
>  
> +import org.apache.aries.proxy.impl.ProxyUtils;
>  import org.objectweb.asm.AnnotationVisitor;
>  import org.objectweb.asm.Attribute;
>  import org.objectweb.asm.ClassReader;
> @@ -107,9 +108,7 @@ public class ProxySubclassAdapter extend
>        throw new TypeNotPresentException(superclassBinaryName, cnfe);
>      }
>  
> -    // move the existing class name to become the superclass
> -    // modify the version of the dynamic subclass to be Java 1.6
> -    int newVersion = Opcodes.V1_6;
> +
>      // keep the same access and signature as the superclass (unless it's abstract)
>      // remove all the superclass interfaces because they will be inherited
>      // from the superclass anyway
> @@ -117,7 +116,7 @@ public class ProxySubclassAdapter extend
>        //If the super was abstract the subclass should not be!
>        access &= ~ACC_ABSTRACT;
>      }
> -    cv.visit(newVersion, access, newClassName, signature, name, null);
> +    cv.visit(ProxyUtils.getWeavingJavaVersion(), access, newClassName, signature, name, null);
>  
>      // add a private field for the invocation handler
>      // this isn't static in case we have multiple instances of the same
> 
> Modified: aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java
> URL: http://svn.apache.org/viewvc/aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java?rev=1245223&r1=1245222&r2=1245223&view=diff
> ==============================================================================
> --- aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java (original)
> +++ aries/trunk/proxy/proxy-impl/src/main/java/org/apache/aries/proxy/impl/interfaces/InterfaceCombiningClassAdapter.java Thu Feb 16 22:38:27 2012
> @@ -25,6 +25,7 @@ import java.util.Collection;
>  import java.util.List;
>  
>  import org.apache.aries.proxy.UnableToProxyException;
> +import org.apache.aries.proxy.impl.ProxyUtils;
>  import org.apache.aries.proxy.impl.common.AbstractWovenProxyAdapter;
>  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassVisitor;
>  import org.apache.aries.proxy.impl.common.OSGiFriendlyClassWriter;
> @@ -75,7 +76,7 @@ final class InterfaceCombiningClassAdapt
>        i++;
>      }
>  
> -    adapter.visit(V1_6, ACC_PUBLIC | ACC_SYNTHETIC, className, null,
> +    adapter.visit(ProxyUtils.getWeavingJavaVersion(), ACC_PUBLIC | ACC_SYNTHETIC, className, null,
>          (superclass == null) ? AbstractWovenProxyAdapter.OBJECT_TYPE.getInternalName() :
>            Type.getInternalName(superclass), interfaceNames);
>    }
> 
>