[#noissue] Improve immutability of ASMMethodInsnNodeRemapper

pinpoint-apm · Jan 8, 2018 · 1dd09cf · 1dd09cf
1 parent bbc1265
commit 1dd09cf
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 34 deletions.
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMAspectWeaver.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMAspectWeaver.java
@@ -102,9 +102,9 @@ private void copyUtilMethods(final MethodNodes methodNodes, final ASMClassNodeAd
     }
 
     private void copyPointCutMethods(final MethodNodes methodNodes, final ASMClassNodeAdapter classNode) throws InstrumentException {
-        final ASMMethodInsnNodeRemapper remapper = new ASMMethodInsnNodeRemapper();
+        final ASMMethodInsnNodeRemapper.Builder remapBuilder = new ASMMethodInsnNodeRemapper.Builder();
         for (ASMMethodNodeAdapter joinPointMethodNode : methodNodes.jointPoints) {
-            remapper.addFilter(null, joinPointMethodNode.getName(), joinPointMethodNode.getDesc());
+            remapBuilder.addFilter(null, joinPointMethodNode.getName(), joinPointMethodNode.getDesc());
         }
 
         for (ASMMethodNodeAdapter pointCutMethodNode : methodNodes.pointCuts) {
@@ -134,7 +134,8 @@ private void copyPointCutMethods(final MethodNodes methodNodes, final ASMClassNo
             if (newMethodNode == null) {
                 throw new InstrumentException("not found new method. " + classNode.getInternalName() + "." + pointCutMethodNode.getName());
             }
-            remapper.setName(methodName);
+            remapBuilder.setName(methodName);
+            ASMMethodInsnNodeRemapper remapper = remapBuilder.build();
             newMethodNode.remapMethodInsnNode(remapper);
         }
     }

diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMClassNodeAdapter.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMClassNodeAdapter.java
@@ -371,10 +371,11 @@ public void copyMethod(final ASMMethodNodeAdapter methodNode) {
         }
 
         // change local call.
-        final ASMMethodInsnNodeRemapper remapper = new ASMMethodInsnNodeRemapper();
-        remapper.addFilter(methodNode.getDeclaringClassInternalName(), null, null);
-        remapper.setOwner(this.classNode.name);
+        final ASMMethodInsnNodeRemapper.Builder remapBuilder = new ASMMethodInsnNodeRemapper.Builder();
+        remapBuilder.addFilter(methodNode.getDeclaringClassInternalName(), null, null);
+        remapBuilder.setOwner(this.classNode.name);
         // remap method call.
+        final ASMMethodInsnNodeRemapper remapper = remapBuilder.build();
         methodNode.remapMethodInsnNode(remapper);
         // remap desc of this.
         methodNode.remapLocalVariables("this", Type.getObjectType(this.classNode.name).getDescriptor());

diff --git a/...r/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMMethodInsnNodeRemapper.java b/...r/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMMethodInsnNodeRemapper.java
@@ -15,6 +15,7 @@
  */
 package com.navercorp.pinpoint.profiler.instrument;
 
+import com.navercorp.pinpoint.common.util.StringUtils;
 import org.objectweb.asm.tree.MethodInsnNode;
 
 import java.util.ArrayList;
@@ -24,37 +25,29 @@
  * @author jaehong.kim
  */
 public class ASMMethodInsnNodeRemapper {
-    private List<Filter> filters = new ArrayList<Filter>();
-    private String owner;
-    private String name;
-    private String desc;
-
-    public void addFilter(final String ownerClassInternalName, final String name, final String desc) {
-        this.filters.add(new Filter(ownerClassInternalName, name, desc));
-    }
-
-    public void setOwner(final String ownerClassInternalName) {
-        this.owner = ownerClassInternalName;
+    private final Filter[] filters;
+    private final String owner;
+    private final String name;
+    private final String desc;
+
+    private ASMMethodInsnNodeRemapper(Filter[] filters, String owner, String name, String desc) {
+        this.filters = filters;
+        this.owner = owner;
+        this.name = name;
+        this.desc = desc;
     }
 
     private String mapOwner(final String ownerClassInternalName) {
-        return this.owner != null ? this.owner : ownerClassInternalName;
-    }
-
-    public void setName(final String name) {
-        this.name = name;
+        return StringUtils.defaultString(this.owner, ownerClassInternalName);
     }
 
     private String mapName(final String name) {
-        return this.name != null ? this.name : name;
+        return StringUtils.defaultString(this.name, name);
     }
 
-    public void setDesc(final String desc) {
-        this.desc = desc;
-    }
 
     private String mapDesc(final String desc) {
-        return this.desc != null ? this.desc : desc;
+        return StringUtils.defaultString(this.desc, desc);
     }
 
     public void mapping(final MethodInsnNode methodInsnNode) {
@@ -67,6 +60,34 @@ public void mapping(final MethodInsnNode methodInsnNode) {
         }
     }
 
+    public static class Builder {
+        private List<Filter> filters = new ArrayList<Filter>();
+        private String owner;
+        private String name;
+        private String desc;
+
+        public void addFilter(final String ownerClassInternalName, final String name, final String desc) {
+            this.filters.add(new Filter(ownerClassInternalName, name, desc));
+        }
+
+        public void setOwner(final String ownerClassInternalName) {
+            this.owner = ownerClassInternalName;
+        }
+
+        public void setName(final String name) {
+            this.name = name;
+        }
+
+        public void setDesc(final String desc) {
+            this.desc = desc;
+        }
+
+        public ASMMethodInsnNodeRemapper build() {
+            Filter[] copyFilter = this.filters.toArray(new Filter[this.filters.size()]);
+            return new ASMMethodInsnNodeRemapper(copyFilter, owner, name, desc);
+        }
+    }
+
     private static class Filter {
         private final String owner;
         private final String name;

diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMMethodNodeAdapter.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/ASMMethodNodeAdapter.java
@@ -208,10 +208,11 @@ public void rename(final String name) {
             throw new IllegalArgumentException("method name must not be null.");
         }
 
-        final ASMMethodInsnNodeRemapper remapper = new ASMMethodInsnNodeRemapper();
-        remapper.addFilter(this.declaringClassInternalName, this.methodNode.name, this.methodNode.desc);
-        remapper.setName(name);
+        final ASMMethodInsnNodeRemapper.Builder remapBuilder = new ASMMethodInsnNodeRemapper.Builder();
+        remapBuilder.addFilter(this.declaringClassInternalName, this.methodNode.name, this.methodNode.desc);
+        remapBuilder.setName(name);
         // change recursive call.
+        ASMMethodInsnNodeRemapper remapper = remapBuilder.build();
         remapMethodInsnNode(remapper);
 
         // change name.

diff --git a/...m/navercorp/pinpoint/profiler/instrument/ASMMethodNodeAdapterRemapMethodInsnNodeTest.java b/...m/navercorp/pinpoint/profiler/instrument/ASMMethodNodeAdapterRemapMethodInsnNodeTest.java
@@ -44,10 +44,10 @@ public void handle(ClassNode classNode) {
                     if (methodNode.name.equals("getHeader")) {
                         ASMMethodNodeAdapter adapter = new ASMMethodNodeAdapter(classNode.name, methodNode);
 
-                        final ASMMethodInsnNodeRemapper remapper = new ASMMethodInsnNodeRemapper();
-                        remapper.addFilter(null, "__getHeader", methodNode.desc);
-                        remapper.setName("__getHeader_$$pinpoint");
-                        adapter.remapMethodInsnNode(remapper);
+                        final ASMMethodInsnNodeRemapper.Builder remapBuilder = new ASMMethodInsnNodeRemapper.Builder();
+                        remapBuilder.addFilter(null, "__getHeader", methodNode.desc);
+                        remapBuilder.setName("__getHeader_$$pinpoint");
+                        adapter.remapMethodInsnNode(remapBuilder.build());
                     }
                 }
             }