From c9975504c6e2f5c186eb3aaf4b9c37d4e3631e21 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 11 Feb 2010 10:38:33 +0000 Subject: [PATCH] + improved LocalVariableTableParameterNameDiscoverer discovery and memory usage + added extra tests --- ...lVariableTableParameterNameDiscoverer.java | 316 ++++++++---------- ...ableTableParameterNameDiscovererTests.java | 137 +++++++- 2 files changed, 269 insertions(+), 184 deletions(-) diff --git a/org.springframework.core/src/main/java/org/springframework/core/LocalVariableTableParameterNameDiscoverer.java b/org.springframework.core/src/main/java/org/springframework/core/LocalVariableTableParameterNameDiscoverer.java index 782cbee01b..44a2c01240 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/LocalVariableTableParameterNameDiscoverer.java +++ b/org.springframework.core/src/main/java/org/springframework/core/LocalVariableTableParameterNameDiscoverer.java @@ -16,14 +16,12 @@ package org.springframework.core; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Constructor; import java.lang.reflect.Member; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.util.HashMap; +import java.util.Collections; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -43,225 +41,163 @@ import org.springframework.util.ClassUtils; * null if the class file was compiled without debug information. * *

Uses ObjectWeb's ASM library for analyzing class files. Each discoverer - * instance caches the ASM ClassReader for each introspected Class, in a + * instance caches the ASM discovered information for each introspected Class, in a * thread-safe manner. It is recommended to reuse discoverer instances * as far as possible. * * @author Adrian Colyer * @author Juergen Hoeller + * @author Costin Leau * @since 2.0 */ public class LocalVariableTableParameterNameDiscoverer implements ParameterNameDiscoverer { private static Log logger = LogFactory.getLog(LocalVariableTableParameterNameDiscoverer.class); - private final Map parameterNamesCache = new ConcurrentHashMap(); - - private final Map classReaderCache = new HashMap(); + // the cache uses a nested index (value is a map) to keep the top level cache relatively small in size + private final Map, Map> parameterNamesCache = new ConcurrentHashMap, Map>(); + // marker object for members that have been visited yet no params have been discovered for it + private static final String[] EMPTY_NAMES_ARRAY = new String[0]; + // marker object for classes that do not have any debug info + private static final Map EMPTY_MAP = Collections.emptyMap(); public String[] getParameterNames(Method method) { - String[] paramNames = this.parameterNamesCache.get(method); - if (paramNames == null) { - try { - paramNames = visitMethod(method).getParameterNames(); - if (paramNames != null) { - this.parameterNamesCache.put(method, paramNames); - } - } - catch (IOException ex) { - // We couldn't load the class file, which is not fatal as it - // simply means this method of discovering parameter names won't work. - if (logger.isDebugEnabled()) { - logger.debug("IOException whilst attempting to read '.class' file for class [" + - method.getDeclaringClass().getName() + - "] - unable to determine parameter names for method: " + method, ex); - } - } + Class declaringClass = method.getDeclaringClass(); + Map map = parameterNamesCache.get(declaringClass); + if (map == null) { + // initialize cache + map = cacheClass(declaringClass); } - return paramNames; + if (map != EMPTY_MAP) { + String[] paramNames = map.get(method); + return (paramNames == EMPTY_NAMES_ARRAY ? null : paramNames); + } + return null; } + @SuppressWarnings("unchecked") public String[] getParameterNames(Constructor ctor) { - String[] paramNames = this.parameterNamesCache.get(ctor); - if (paramNames == null) { - try { - paramNames = visitConstructor(ctor).getParameterNames(); - if (paramNames != null) { - this.parameterNamesCache.put(ctor, paramNames); - } - } - catch (IOException ex) { - // We couldn't load the class file, which is not fatal as it - // simply means this method of discovering parameter names won't work. - if (logger.isDebugEnabled()) { - logger.debug("IOException whilst attempting to read '.class' file for class [" + - ctor.getDeclaringClass().getName() + - "] - unable to determine parameter names for constructor: " + ctor, ex); - } - } + Class declaringClass = ctor.getDeclaringClass(); + Map map = parameterNamesCache.get(declaringClass); + if (map == null) { + // initialize cache + map = cacheClass(declaringClass); } - return paramNames; - } - - /** - * Visit the given method and discover its parameter names. - */ - private ParameterNameDiscoveringVisitor visitMethod(Method method) throws IOException { - ClassReader classReader = getClassReader(method.getDeclaringClass()); - FindMethodParameterNamesClassVisitor classVisitor = new FindMethodParameterNamesClassVisitor(method); - classReader.accept(classVisitor, false); - return classVisitor; + String[] paramNames = map.get(ctor); + return (paramNames == EMPTY_NAMES_ARRAY ? null : paramNames); } /** - * Visit the given constructor and discover its parameter names. + * Caches the results of the introspected class. Exceptions will be logged and swallowed. + * + * @param declaringClass class about to be inspected + * @return cached value */ - private ParameterNameDiscoveringVisitor visitConstructor(Constructor ctor) throws IOException { - ClassReader classReader = getClassReader(ctor.getDeclaringClass()); - FindConstructorParameterNamesClassVisitor classVisitor = new FindConstructorParameterNamesClassVisitor(ctor); - classReader.accept(classVisitor, false); - return classVisitor; - } + private Map cacheClass(Class clazz) { + + InputStream is = clazz.getResourceAsStream(ClassUtils.getClassFileName(clazz)); + if (is == null) { + // We couldn't load the class file, which is not fatal as it + // simply means this method of discovering parameter names won't work. + if (logger.isDebugEnabled()) { + logger.debug("Cannot find '.class' file for class [" + clazz + + "] - unable to determine constructors/methods parameter names"); + } + return Collections.emptyMap(); + } - /** - * Obtain a (cached) ClassReader for the given class. - */ - private ClassReader getClassReader(Class clazz) throws IOException { - synchronized (this.classReaderCache) { - ClassReader classReader = (ClassReader) this.classReaderCache.get(clazz); - if (classReader == null) { - InputStream is = clazz.getResourceAsStream(ClassUtils.getClassFileName(clazz)); - if (is == null) { - throw new FileNotFoundException("Class file for class [" + clazz.getName() + "] not found"); - } - try { - classReader = new ClassReader(is); - this.classReaderCache.put(clazz, classReader); - } - finally { - is.close(); - } + try { + ClassReader classReader = new ClassReader(is); + Map map = new ConcurrentHashMap(); + classReader.accept(new ParameterNameDiscoveringVisitor(clazz, map), false); + return map; + } catch (IOException ex) { + if (logger.isDebugEnabled()) { + logger.debug("Exception thrown while reading '.class' file for class [" + clazz + + "] - unable to determine constructors/methods parameter names", ex); + } + } finally { + try { + is.close(); + } catch (IOException ex) { + // ignore } - return classReader; } - } + return Collections.emptyMap(); + } /** - * Helper class that looks for a given member name and descriptor, and then + * Helper class that inspects all methods (constructor included) and then * attempts to find the parameter names for that member. */ - private static abstract class ParameterNameDiscoveringVisitor extends EmptyVisitor { - - private String methodNameToMatch; + private static class ParameterNameDiscoveringVisitor extends EmptyVisitor { - private String descriptorToMatch; + private final Class clazz; + private final Map memberMap; + private static final String STATIC_CLASS_INIT = ""; - private int numParamsExpected; - - /* - * The nth entry contains the slot index of the LVT table entry holding the - * argument name for the nth parameter. - */ - private int[] lvtSlotIndex; - - private String[] parameterNames; - - public ParameterNameDiscoveringVisitor(String name, boolean isStatic, Class[] paramTypes) { - this.methodNameToMatch = name; - this.numParamsExpected = paramTypes.length; - computeLvtSlotIndices(isStatic, paramTypes); - } - - public void setDescriptorToMatch(String descriptor) { - this.descriptorToMatch = descriptor; + public ParameterNameDiscoveringVisitor(Class clazz, Map memberMap) { + this.clazz = clazz; + this.memberMap = memberMap; } @Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { - if (name.equals(this.methodNameToMatch) && desc.equals(this.descriptorToMatch)) { - return new LocalVariableTableVisitor(this, isStatic(access)); - } - else { - // Not interested in this method... - return null; + // exclude synthetic + bridged && static class initialization + if (!isSyntheticOrBridged(access) && !STATIC_CLASS_INIT.equals(name)) { + return new LocalVariableTableVisitor(clazz, memberMap, name, desc, isStatic(access)); } - } - - private boolean isStatic(int access) { - return ((access & Opcodes.ACC_STATIC) > 0); + return null; } - public String[] getParameterNames() { - return this.parameterNames; + private static boolean isSyntheticOrBridged(int access) { + return (((access & Opcodes.ACC_SYNTHETIC) | (access & Opcodes.ACC_BRIDGE)) > 0); } - private void computeLvtSlotIndices(boolean isStatic, Class[] paramTypes) { - this.lvtSlotIndex = new int[paramTypes.length]; - int nextIndex = (isStatic ? 0 : 1); - for (int i = 0; i < paramTypes.length; i++) { - this.lvtSlotIndex[i] = nextIndex; - if (isWideType(paramTypes[i])) { - nextIndex += 2; - } - else { - nextIndex++; - } - } - } - - private boolean isWideType(Class aType) { - return (aType == Long.TYPE || aType == Double.TYPE); - } - } - - - private static class FindMethodParameterNamesClassVisitor extends ParameterNameDiscoveringVisitor { - - public FindMethodParameterNamesClassVisitor(Method method) { - super(method.getName(), Modifier.isStatic(method.getModifiers()), method.getParameterTypes()); - setDescriptorToMatch(Type.getMethodDescriptor(method)); - } - } - - - private static class FindConstructorParameterNamesClassVisitor extends ParameterNameDiscoveringVisitor { - - public FindConstructorParameterNamesClassVisitor(Constructor ctor) { - super("", false, ctor.getParameterTypes()); - Type[] pTypes = new Type[ctor.getParameterTypes().length]; - for (int i = 0; i < pTypes.length; i++) { - pTypes[i] = Type.getType(ctor.getParameterTypes()[i]); - } - setDescriptorToMatch(Type.getMethodDescriptor(Type.VOID_TYPE, pTypes)); + private static boolean isStatic(int access) { + return ((access & Opcodes.ACC_STATIC) > 0); } } - private static class LocalVariableTableVisitor extends EmptyVisitor { - private final ParameterNameDiscoveringVisitor memberVisitor; + private static final String CONSTRUCTOR = ""; + private final Class clazz; + private final Map memberMap; + private final String name; + private final Type[] args; private final boolean isStatic; private String[] parameterNames; - private boolean hasLvtInfo = false; - public LocalVariableTableVisitor(ParameterNameDiscoveringVisitor memberVisitor, boolean isStatic) { - this.memberVisitor = memberVisitor; + /* + * The nth entry contains the slot index of the LVT table entry holding the + * argument name for the nth parameter. + */ + private final int[] lvtSlotIndex; + + public LocalVariableTableVisitor(Class clazz, Map map, String name, String desc, + boolean isStatic) { + this.clazz = clazz; + this.memberMap = map; + this.name = name; + // determine args + args = Type.getArgumentTypes(desc); + this.parameterNames = new String[args.length]; this.isStatic = isStatic; - this.parameterNames = new String[memberVisitor.numParamsExpected]; + this.lvtSlotIndex = computeLvtSlotIndices(isStatic, args); } @Override - public void visitLocalVariable( - String name, String description, String signature, Label start, Label end, int index) { + public void visitLocalVariable(String name, String description, String signature, Label start, Label end, + int index) { this.hasLvtInfo = true; - int[] lvtSlotIndices = this.memberVisitor.lvtSlotIndex; - for (int i = 0; i < lvtSlotIndices.length; i++) { - if (lvtSlotIndices[i] == index) { + for (int i = 0; i < lvtSlotIndex.length; i++) { + if (lvtSlotIndex[i] == index) { this.parameterNames[i] = name; } } @@ -270,13 +206,51 @@ public class LocalVariableTableParameterNameDiscoverer implements ParameterNameD @Override public void visitEnd() { if (this.hasLvtInfo || (this.isStatic && this.parameterNames.length == 0)) { - // visitLocalVariable will never be called for static no args methods - // which doesn't use any local variables. - // This means that hasLvtInfo could be false for that kind of methods - // even if the class has local variable info. - this.memberVisitor.parameterNames = this.parameterNames; + // visitLocalVariable will never be called for static no args methods + // which doesn't use any local variables. + // This means that hasLvtInfo could be false for that kind of methods + // even if the class has local variable info. + memberMap.put(resolveMember(), parameterNames); } } - } -} + private Member resolveMember() { + ClassLoader loader = clazz.getClassLoader(); + Class[] classes = new Class[args.length]; + + // resolve args + for (int i = 0; i < args.length; i++) { + classes[i] = ClassUtils.resolveClassName(args[i].getClassName(), loader); + } + try { + if (CONSTRUCTOR.equals(name)) { + return clazz.getDeclaredConstructor(classes); + } + + return clazz.getDeclaredMethod(name, classes); + } catch (NoSuchMethodException ex) { + throw new IllegalStateException("Method [" + name + + "] was discovered in the .class file but cannot be resolved in the class object", ex); + } + } + + private static int[] computeLvtSlotIndices(boolean isStatic, Type[] paramTypes) { + int[] lvtIndex = new int[paramTypes.length]; + int nextIndex = (isStatic ? 0 : 1); + for (int i = 0; i < paramTypes.length; i++) { + lvtIndex[i] = nextIndex; + if (isWideType(paramTypes[i])) { + nextIndex += 2; + } else { + nextIndex++; + } + } + return lvtIndex; + } + + private static boolean isWideType(Type aType) { + // float is not a wide type + return (aType == Type.LONG_TYPE || aType == Type.DOUBLE_TYPE); + } + } +} \ No newline at end of file diff --git a/org.springframework.core/src/test/java/org/springframework/core/LocalVariableTableParameterNameDiscovererTests.java b/org.springframework.core/src/test/java/org/springframework/core/LocalVariableTableParameterNameDiscovererTests.java index f12d96f130..29e76e699b 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/LocalVariableTableParameterNameDiscovererTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/LocalVariableTableParameterNameDiscovererTests.java @@ -16,8 +16,12 @@ package org.springframework.core; +import java.awt.Component; +import java.io.PrintStream; import java.lang.reflect.Constructor; import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Date; import junit.framework.TestCase; @@ -30,7 +34,6 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { private LocalVariableTableParameterNameDiscoverer discoverer = new LocalVariableTableParameterNameDiscoverer(); - public void testMethodParameterNameDiscoveryNoArgs() throws NoSuchMethodException { Method getName = TestBean.class.getMethod("getName", new Class[0]); String[] names = discoverer.getParameterNames(getName); @@ -39,7 +42,7 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { } public void testMethodParameterNameDiscoveryWithArgs() throws NoSuchMethodException { - Method setName = TestBean.class.getMethod("setName", new Class[]{String.class}); + Method setName = TestBean.class.getMethod("setName", new Class[] { String.class }); String[] names = discoverer.getParameterNames(setName); assertNotNull("should find method info", names); assertEquals("one argument", 1, names.length); @@ -54,7 +57,7 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { } public void testConsParameterNameDiscoveryArgs() throws NoSuchMethodException { - Constructor twoArgCons = TestBean.class.getConstructor(new Class[]{String.class, int.class}); + Constructor twoArgCons = TestBean.class.getConstructor(new Class[] { String.class, int.class }); String[] names = discoverer.getParameterNames(twoArgCons); assertNotNull("should find cons info", names); assertEquals("one argument", 2, names.length); @@ -72,14 +75,14 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { public void testOverloadedStaticMethod() throws Exception { Class clazz = this.getClass(); - Method m1 = clazz.getMethod("staticMethod", new Class[]{Long.TYPE, Long.TYPE}); + Method m1 = clazz.getMethod("staticMethod", new Class[] { Long.TYPE, Long.TYPE }); String[] names = discoverer.getParameterNames(m1); assertNotNull("should find method info", names); assertEquals("two arguments", 2, names.length); assertEquals("x", names[0]); assertEquals("y", names[1]); - Method m2 = clazz.getMethod("staticMethod", new Class[]{Long.TYPE, Long.TYPE, Long.TYPE}); + Method m2 = clazz.getMethod("staticMethod", new Class[] { Long.TYPE, Long.TYPE, Long.TYPE }); names = discoverer.getParameterNames(m2); assertNotNull("should find method info", names); assertEquals("three arguments", 3, names.length); @@ -91,13 +94,13 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { public void testOverloadedStaticMethodInInnerClass() throws Exception { Class clazz = InnerClass.class; - Method m1 = clazz.getMethod("staticMethod", new Class[]{Long.TYPE}); + Method m1 = clazz.getMethod("staticMethod", new Class[] { Long.TYPE }); String[] names = discoverer.getParameterNames(m1); assertNotNull("should find method info", names); assertEquals("one argument", 1, names.length); assertEquals("x", names[0]); - Method m2 = clazz.getMethod("staticMethod", new Class[]{Long.TYPE, Long.TYPE}); + Method m2 = clazz.getMethod("staticMethod", new Class[] { Long.TYPE, Long.TYPE }); names = discoverer.getParameterNames(m2); assertNotNull("should find method info", names); assertEquals("two arguments", 2, names.length); @@ -108,14 +111,14 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { public void testOverloadedMethod() throws Exception { Class clazz = this.getClass(); - Method m1 = clazz.getMethod("instanceMethod", new Class[]{Double.TYPE, Double.TYPE}); + Method m1 = clazz.getMethod("instanceMethod", new Class[] { Double.TYPE, Double.TYPE }); String[] names = discoverer.getParameterNames(m1); assertNotNull("should find method info", names); assertEquals("two arguments", 2, names.length); assertEquals("x", names[0]); assertEquals("y", names[1]); - Method m2 = clazz.getMethod("instanceMethod", new Class[]{Double.TYPE, Double.TYPE, Double.TYPE}); + Method m2 = clazz.getMethod("instanceMethod", new Class[] { Double.TYPE, Double.TYPE, Double.TYPE }); names = discoverer.getParameterNames(m2); assertNotNull("should find method info", names); assertEquals("three arguments", 3, names.length); @@ -127,13 +130,13 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { public void testOverloadedMethodInInnerClass() throws Exception { Class clazz = InnerClass.class; - Method m1 = clazz.getMethod("instanceMethod", new Class[]{String.class}); + Method m1 = clazz.getMethod("instanceMethod", new Class[] { String.class }); String[] names = discoverer.getParameterNames(m1); assertNotNull("should find method info", names); assertEquals("one argument", 1, names.length); assertEquals("aa", names[0]); - Method m2 = clazz.getMethod("instanceMethod", new Class[]{String.class, String.class}); + Method m2 = clazz.getMethod("instanceMethod", new Class[] { String.class, String.class }); names = discoverer.getParameterNames(m2); assertNotNull("should find method info", names); assertEquals("two arguments", 2, names.length); @@ -141,6 +144,71 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { assertEquals("bb", names[1]); } + public void testGenerifiedClass() throws Exception { + Class clazz = GenerifiedClass.class; + + Constructor ctor = clazz.getDeclaredConstructor(Object.class); + String[] names = discoverer.getParameterNames(ctor); + assertEquals(1, names.length); + assertEquals("key", names[0]); + + ctor = clazz.getDeclaredConstructor(Object.class, Object.class); + names = discoverer.getParameterNames(ctor); + assertEquals(2, names.length); + assertEquals("key", names[0]); + assertEquals("value", names[1]); + + Method m = clazz.getMethod("generifiedStaticMethod", Object.class); + names = discoverer.getParameterNames(m); + assertEquals(1, names.length); + assertEquals("param", names[0]); + + m = clazz.getMethod("generifiedMethod", Object.class, long.class, Object.class, Object.class); + names = discoverer.getParameterNames(m); + assertEquals(4, names.length); + assertEquals("param", names[0]); + assertEquals("x", names[1]); + assertEquals("key", names[2]); + assertEquals("value", names[3]); + + m = clazz.getMethod("voidStaticMethod", Object.class, long.class, int.class); + names = discoverer.getParameterNames(m); + assertEquals(3, names.length); + assertEquals("obj", names[0]); + assertEquals("x", names[1]); + assertEquals("i", names[2]); + + m = clazz.getMethod("nonVoidStaticMethod", Object.class, long.class, int.class); + names = discoverer.getParameterNames(m); + assertEquals(3, names.length); + assertEquals("obj", names[0]); + assertEquals("x", names[1]); + assertEquals("i", names[2]); + + m = clazz.getMethod("getDate", null); + names = discoverer.getParameterNames(m); + assertEquals(0, names.length); + } + + public void testMemUsage() throws Exception { + // JDK classes don't have debug information (usually) + Class clazz = Component.class; + String methodName = "list"; + + Method m = clazz.getMethod(methodName, null); + String[] names = discoverer.getParameterNames(m); + assertNull(names); + + m = clazz.getMethod(methodName, PrintStream.class); + names = discoverer.getParameterNames(m); + assertNull(names); + + m = clazz.getMethod(methodName, PrintStream.class, int.class); + names = discoverer.getParameterNames(m); + assertNull(names); + + //System.in.read() + } public static void staticMethodNoLocalVars() { } @@ -165,7 +233,6 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { return u; } - public static class InnerClass { public int waz = 0; @@ -198,4 +265,48 @@ public class LocalVariableTableParameterNameDiscovererTests extends TestCase { } } -} + public static class GenerifiedClass { + private static long date; + + private K key; + private V value; + + static { + // some custom static bloc or + date = new Date().getTime(); + } + + public GenerifiedClass() { + this(null, null); + } + + public GenerifiedClass(K key) { + this(key, null); + } + + public GenerifiedClass(K key, V value) { + this.key = key; + this.value = value; + } + + public static

long generifiedStaticMethod(P param) { + return date; + } + + public

void generifiedMethod(P param, long x, K key, V value) { + // nothing + } + + public static void voidStaticMethod(Object obj, long x, int i) { + // nothing + } + + public static long nonVoidStaticMethod(Object obj, long x, int i) { + return date; + } + + public static long getDate() { + return date; + } + } +} \ No newline at end of file -- GitLab