You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2011/12/23 19:54:38 UTC

svn commit: r1222788 - in /tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic: FieldInstrumentation.java FieldInstrumentations.java PlasticClassImpl.java PlasticClassPool.java PlasticFieldImpl.java

Author: hlship
Date: Fri Dec 23 18:54:37 2011
New Revision: 1222788

URL: http://svn.apache.org/viewvc?rev=1222788&view=rev
Log:
TAP5-1801: Allow non-public fields in instrumented classes

Refactor how file instrumentations are stored in the PlasticClass and in the PlasticClassPool

Added:
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentation.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentations.java
Modified:
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java

Added: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentation.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentation.java?rev=1222788&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentation.java (added)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentation.java Fri Dec 23 18:54:37 2011
@@ -0,0 +1,31 @@
+// Copyright 2011 The Apache Software Foundation
+//
+// Licensed 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.tapestry5.internal.plastic;
+
+/**
+ * Tracks the instrumentations of a field, in terms of a method name
+ * and description to invoke instead of reading from or writing to the
+ * field.
+ */
+class FieldInstrumentation
+{
+    final String methodName, methodDescription;
+
+    FieldInstrumentation(String methodName, String methodDescription)
+    {
+        this.methodName = methodName;
+        this.methodDescription = methodDescription;
+    }
+}

Added: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentations.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentations.java?rev=1222788&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentations.java (added)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/FieldInstrumentations.java Fri Dec 23 18:54:37 2011
@@ -0,0 +1,43 @@
+// Copyright 2011 The Apache Software Foundation
+//
+// Licensed 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.tapestry5.internal.plastic;
+
+import java.util.Map;
+
+/**
+ * Tracks field read and write instrumentations for a particular class. A field instrumentation is the replacement
+ * of direct access to an instance field with invocation of a (synthetic) instance method on the same class. In most cases,
+ * the same field will have two instrumentations; one for read, one for write. In some cases, a field will have a single
+ * write instrumentation.
+ */
+class FieldInstrumentations
+{
+    /**
+     * Map field name to a read method.
+     */
+    final Map<String, FieldInstrumentation> read = PlasticInternalUtils.newMap();
+
+    /**
+     * Maps field name to a write method.
+     */
+    final Map<String, FieldInstrumentation> write = PlasticInternalUtils.newMap();
+
+    FieldInstrumentation get(String fieldName, boolean forRead)
+    {
+        Map<String, FieldInstrumentation> map = forRead ? read : write;
+
+        return map.get(fieldName);
+    }
+}

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java?rev=1222788&r1=1222787&r2=1222788&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java Fri Dec 23 18:54:37 2011
@@ -133,17 +133,12 @@ public class PlasticClassImpl extends Lo
     // such methods are not optimized away incorrectly.
     final Set<MethodNode> shimInvokedMethods = PlasticInternalUtils.newSet();
 
-    /**
-     * Maps a field name to a replacement method that should be invoked instead of reading the
-     * field.
-     */
-    private final Map<String, MethodNode> fieldToReadMethod = PlasticInternalUtils.newMap();
 
     /**
-     * Maps a field name to a replacement method that should be invoked instead of writing the
-     * field.
+     * Tracks instrumentations of fields of this class, including private fields which are not published into the
+     * {@link PlasticClassPool}.
      */
-    private final Map<String, MethodNode> fieldToWriteMethod = PlasticInternalUtils.newMap();
+    private final FieldInstrumentations fieldInstrumentations = new FieldInstrumentations();
 
     /**
      * This normal no-arguments constructor, or null. By the end of the transformation
@@ -712,27 +707,16 @@ public class PlasticClassImpl extends Lo
      */
     private void interceptFieldAccess()
     {
-        Set<MethodNode> unusedAccessMethods = PlasticInternalUtils.newSet();
-
-        // Track all the access methods created for the fields.
-
-        unusedAccessMethods.addAll(fieldToReadMethod.values());
-        unusedAccessMethods.addAll(fieldToWriteMethod.values());
-
-        // Keep any field access methods invoked from the shim
-        unusedAccessMethods.removeAll(shimInvokedMethods);
-
         for (MethodNode node : fieldTransformMethods)
         {
             // Intercept field access inside the method, tracking which access methods
             // are actually used by removing them from accessMethods
 
-            interceptFieldAccess(node, unusedAccessMethods);
+            interceptFieldAccess(node);
         }
 
         // Remove all added methods that aren't actually used.
 
-        classNode.methods.removeAll(unusedAccessMethods);
     }
 
     /**
@@ -909,7 +893,7 @@ public class PlasticClassImpl extends Lo
         }
     }
 
-    private void interceptFieldAccess(MethodNode methodNode, Set<MethodNode> unusedAccessMethods)
+    private void interceptFieldAccess(MethodNode methodNode)
     {
         InsnList insns = methodNode.instructions;
 
@@ -922,33 +906,33 @@ public class PlasticClassImpl extends Lo
             int opcode = node.getOpcode();
 
             if (opcode != GETFIELD && opcode != PUTFIELD)
+            {
                 continue;
-
-            // Make sure we're talking about access to a field of this class, not some other
-            // visible field of another class.
+            }
 
             FieldInsnNode fnode = (FieldInsnNode) node;
 
-            if (!fnode.owner.equals(classNode.name))
-                continue;
-
-            Map<String, MethodNode> fieldToMethod = opcode == GETFIELD ? fieldToReadMethod : fieldToWriteMethod;
-
-            MethodNode mn = fieldToMethod.get(fnode.name);
+            FieldInstrumentation instrumentation = findInstrumentations(fnode).get(fnode.name, opcode == GETFIELD);
 
-            if (mn == null)
-                continue;
-
-            String methodDescription = opcode == GETFIELD ? "()" + fnode.desc : "(" + fnode.desc + ")V";
+            if (instrumentation == null) { continue; }
 
             // Replace the field access node with the appropriate method invocation.
 
-            insns.insertBefore(fnode, new MethodInsnNode(INVOKEVIRTUAL, fnode.owner, mn.name, methodDescription));
+            insns.insertBefore(fnode, new MethodInsnNode(INVOKEVIRTUAL, fnode.owner, instrumentation.methodName, instrumentation.methodDescription));
 
             it.remove();
+        }
+    }
 
-            unusedAccessMethods.remove(mn);
+    private FieldInstrumentations findInstrumentations(FieldInsnNode node)
+    {
+        if (node.owner.equals(classNode.name))
+        {
+            return fieldInstrumentations;
         }
+
+        return pool.getFieldInstrumentations(node.owner);
+
     }
 
     String getInstanceContextFieldName()
@@ -1136,28 +1120,27 @@ public class PlasticClassImpl extends Lo
         return this;
     }
 
-    void redirectFieldWrite(String fieldName, MethodNode method)
+    void redirectFieldWrite(String fieldName, boolean privateField, MethodNode method)
     {
-        fieldToWriteMethod.put(fieldName, method);
+        FieldInstrumentation fi = new FieldInstrumentation(method.name, method.desc);
+
+        fieldInstrumentations.write.put(fieldName, fi);
 
-        if (!(proxy || isPrivate(method)))
+        if (!(proxy || privateField))
         {
-            pool.setFieldWriteInstrumentation(classNode.name, fieldName, method.name);
+            pool.setFieldWriteInstrumentation(classNode.name, fieldName, fi);
         }
     }
 
-    void redirectFieldRead(String fieldName, MethodNode method)
+    void redirectFieldRead(String fieldName, boolean privateField, MethodNode method)
     {
-        fieldToReadMethod.put(fieldName, method);
+        FieldInstrumentation fi = new FieldInstrumentation(method.name, method.desc);
+
+        fieldInstrumentations.read.put(fieldName, fi);
 
-        if (! (proxy || isPrivate(method)))
+        if (!(proxy || privateField))
         {
-            pool.setFieldReadInstrumentation(classNode.name, fieldName, method.name);
+            pool.setFieldReadInstrumentation(classNode.name, fieldName, fi);
         }
     }
-
-    private boolean isPrivate(MethodNode method)
-    {
-        return Modifier.isPrivate(method.access);
-    }
 }

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java?rev=1222788&r1=1222787&r2=1222788&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java Fri Dec 23 18:54:37 2011
@@ -84,22 +84,6 @@ public class PlasticClassPool implements
      */
     private final Map<String, BaseClassDef> baseClassDefs = PlasticInternalUtils.newMap();
 
-    /**
-     * Tracks field read and write instrumentations for a particular class.
-     */
-    static class FieldInstrumentations
-    {
-        /**
-         * Map field name to a read method.
-         */
-        final Map<String, String> read = PlasticInternalUtils.newMap();
-
-        /**
-         * Maps field name to a write method.
-         */
-        final Map<String, String> write = PlasticInternalUtils.newMap();
-    }
-
 
     private final Map<String, FieldInstrumentations> instrumentations = PlasticInternalUtils.newMap();
 
@@ -547,12 +531,12 @@ public class PlasticClassPool implements
     }
 
 
-    void setFieldReadInstrumentation(String classInternalName, String fieldName, String methodName)
+    void setFieldReadInstrumentation(String classInternalName, String fieldName, FieldInstrumentation fi)
     {
-        instrumentations.get(classInternalName).read.put(fieldName, methodName);
+        instrumentations.get(classInternalName).read.put(fieldName, fi);
     }
 
-    private FieldInstrumentations getFieldInstrumentation(String classInternalName)
+    FieldInstrumentations getFieldInstrumentations(String classInternalName)
     {
         FieldInstrumentations result = instrumentations.get(classInternalName);
 
@@ -590,29 +574,9 @@ public class PlasticClassPool implements
         return result;
     }
 
-
-    /**
-     * Checks to see if there is an instrumentation for the given class and field.
-     * This may cause the class to be transformed.  Returns the name of the method that replaces
-     * the field read, or null if the field is not instrumented in such a way that the method is required.
-     */
-    String getInstrumentedReadMethod(String classInternalName, String fieldName)
+    void setFieldWriteInstrumentation(String classInternalName, String fieldName, FieldInstrumentation fi)
     {
-        return getFieldInstrumentation(classInternalName).read.get(fieldName);
+        instrumentations.get(classInternalName).write.put(fieldName, fi);
     }
 
-    void setFieldWriteInstrumentation(String classInternalName, String fieldName, String methodName)
-    {
-        instrumentations.get(classInternalName).write.put(fieldName, methodName);
-    }
-
-    /**
-     * Checks to see if there is an instrumentation for the given class and field.
-     * This may cause the class to be transformed.  Returns the name of the method that replaces
-     * the field write, or null if the field is not instrumented in such a way that the method is required.
-     */
-    String getInstrumentedWriteMethod(String classInternalName, String fieldName)
-    {
-        return getFieldInstrumentation(classInternalName).write.get(fieldName);
-    }
 }

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java?rev=1222788&r1=1222787&r2=1222788&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticFieldImpl.java Fri Dec 23 18:54:37 2011
@@ -381,15 +381,22 @@ class PlasticFieldImpl extends PlasticMe
 
         plasticClass.addMethod(setAccess);
 
-        plasticClass.redirectFieldWrite(node.name, setAccess);
+        plasticClass.redirectFieldWrite(node.name, isPrivate(), setAccess);
     }
 
-    /** Determines the access for a method that takes the place of reading from or writing to the field. Escalates
-     *  private fields to package private visibility, but leaves protected and package private alone (public fields
-     *  can not be instrumented).
+    /**
+     * Determines the access for a method that takes the place of reading from or writing to the field. Escalates
+     * private fields to package private visibility, but leaves protected and package private alone (public fields
+     * can not be instrumented).
      */
-    private int accessForMethod() {
-        return (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL | node.access) &  ~Opcodes.ACC_PRIVATE;
+    private int accessForMethod()
+    {
+        return (Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL | node.access) & ~Opcodes.ACC_PRIVATE;
+    }
+
+    private boolean isPrivate()
+    {
+        return Modifier.isPrivate(node.access);
     }
 
     private void replaceFieldReadAccess(String conduitFieldName)
@@ -442,7 +449,7 @@ class PlasticFieldImpl extends PlasticMe
 
         plasticClass.addMethod(getAccess);
 
-        plasticClass.redirectFieldRead(node.name, getAccess);
+        plasticClass.redirectFieldRead(node.name, isPrivate(), getAccess);
     }
 
     private boolean isWriteBehindEnabled()
@@ -477,7 +484,7 @@ class PlasticFieldImpl extends PlasticMe
 
         plasticClass.addMethod(setAccess);
 
-        plasticClass.redirectFieldWrite(node.name, setAccess);
+        plasticClass.redirectFieldWrite(node.name, isPrivate(), setAccess);
 
         node.access |= Opcodes.ACC_FINAL;
     }