Author: henrib Date: Sun Dec 11 11:49:31 2011 New Revision: 1212995 URL: http://svn.apache.org/viewvc?rev=1212995&view=rev Log: JEXL-123: Refactored Introspector, ClassMap and MethodKey: simplified class, removed method, use lock and ConcurrentHashMap; Removed parser classes from cobertura report; Aligned pom with 2.1;
Removed: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodMap.java Modified: commons/proper/jexl/trunk/pom.xml commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/introspection/MethodKeyTest.java Modified: commons/proper/jexl/trunk/pom.xml URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/pom.xml?rev=1212995&r1=1212994&r2=1212995&view=diff ============================================================================== --- commons/proper/jexl/trunk/pom.xml (original) +++ commons/proper/jexl/trunk/pom.xml Sun Dec 11 11:49:31 2011 @@ -132,39 +132,14 @@ <commons.release.2.binary.suffix /> <commons.jira.id>JEXL</commons.jira.id> <commons.jira.pid>12310479</commons.jira.pid> - <!-- Temp fix until parent POM is updated --> - <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> - <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> </properties> <build> - <!-- temporarily override the parent POM (v 11) until that is updated --> - <resources> - <resource> - <directory>${basedir}</directory> - <targetPath>META-INF</targetPath> - <includes> - <include>NOTICE.txt</include> - <include>LICENSE.txt</include> - </includes> - </resource> - <resource> - <directory>src/main/resources</directory> - </resource> - </resources> <plugins> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> </plugin> - <!-- workaround MRELEASE-424 when publishing --> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-release-plugin</artifactId> - <configuration> - <mavenExecutorId>forked-path</mavenExecutorId> - </configuration> - </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> @@ -203,6 +178,34 @@ </execution> </executions> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-jar-plugin</artifactId> + <executions> + <execution> + <goals> + <goal>test-jar</goal> + </goals> + </execution> + </executions> + </plugin> + <plugin> + <groupId>org.codehaus.mojo</groupId> + <artifactId>cobertura-maven-plugin</artifactId> + <version>2.5.1</version> + <configuration> + <instrumentation> + <ignores> + <ignore>**/generated-sources/**/*</ignore> + </ignores> + <excludes> + <exclude>**/generated-sources/**/*</exclude> + <exclude>org/apache/commons/jexl3/parser/*.class</exclude> + <exclude>org/apache/commons/jexl3/**/*Test.class</exclude> + </excludes> + </instrumentation> + </configuration> + </plugin> </plugins> </build> @@ -229,31 +232,18 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-checkstyle-plugin</artifactId> - <version>2.8</version> + <version>2.7</version> <configuration> <configLocation>${basedir}/src/main/config/checkstyle.xml</configLocation> <excludes>org/apache/commons/jexl3/parser/*.java</excludes> <headerLocation>${basedir}/src/main/config/header.txt</headerLocation> <enableRulesSummary>false</enableRulesSummary> - <inherited>false</inherited> </configuration> </plugin> <plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>cobertura-maven-plugin</artifactId> <version>2.5.1</version> - <configuration> - <instrumentation> - <ignores> - <ignore>**/generated-sources/**/*</ignore> - </ignores> - <excludes> - <exclude>**/generated-sources/**/*</exclude> - <exclude>org/apache/commons/jexl3/parser/*.class</exclude> - <exclude>org/apache/commons/jexl3/**/*Test.class</exclude> - </excludes> - </instrumentation> - </configuration> </plugin> <plugin> <groupId>org.codehaus.mojo</groupId> Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java?rev=1212995&r1=1212994&r2=1212995&view=diff ============================================================================== --- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java (original) +++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java Sun Dec 11 11:49:31 2011 @@ -16,15 +16,21 @@ */ package org.apache.commons.jexl3.internal.introspection; +import org.apache.commons.logging.Log; + import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Modifier; + +import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.apache.commons.logging.Log; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; /** * A cache of introspection information for a specific class instance. @@ -37,9 +43,44 @@ import org.apache.commons.logging.Log; * @since 1.0 */ final class ClassMap { - /** cache of methods. */ - private final MethodCache methodCache; - /** cache of fields. */ + /** + * A method that returns itself used as a marker for cache miss, + * allows the underlying cache map to be strongly typed. + * @return itself as a method + */ + public static Method cacheMiss() { + try { + return ClassMap.class.getMethod("cacheMiss"); + } catch (Exception xio) { + // this really cant make an error... + return null; + } + } + + /** The cache miss marker method. */ + private static final Method CACHE_MISS = cacheMiss(); + /** + * This is the cache to store and look up the method information. + * <p> + * It stores the association between: + * - a key made of a method name & an array of argument types. + * - a method. + * </p> + * <p> + * Since the invocation of the associated method is dynamic, there is no need (nor way) to differentiate between + * foo(int,int) & foo(Integer,Integer) since in practise, only the latter form will be used through a call. + * This of course, applies to all 8 primitive types. + * </p> + * Uses ConcurrentMap since 3.0, marginally faster than 2.1 under contention. + */ + private final ConcurrentMap<MethodKey, Method> byKey = new ConcurrentHashMap<MethodKey, Method>(); + /** + * Keep track of all methods with the same name; this is not modified after creation. + */ + private final Map<String, List<Method>> byName = new HashMap<String, List<Method>>(); + /** + * Cache of fields. + */ private final Map<String, Field> fieldCache; /** @@ -48,21 +89,31 @@ final class ClassMap { * @param aClass the class to deconstruct. * @param log the logger. */ + @SuppressWarnings("LeakingThisInConstructor") ClassMap(Class<?> aClass, Log log) { // eagerly cache methods - methodCache = createMethodCache(aClass, log); + create(this, aClass, log); // eagerly cache public fields - fieldCache = createFieldCache(aClass); + Field[] fields = aClass.getFields(); + if (fields.length > 0) { + Map<String, Field> cache = new HashMap<String, Field>(); + for (Field field : fields) { + if (Modifier.isPublic(field.getModifiers()) && Permissions.allow(field)) { + cache.put(field.getName(), field); + } + } + fieldCache = cache; + } else { + fieldCache = Collections.emptyMap(); + } } /** * Find a Field using its name. - * <p>The clazz parameter <strong>must</strong> be this ClassMap key.</p> - * @param clazz the class to introspect * @param fname the field name * @return A Field object representing the field to invoke or null. */ - Field findField(final Class<?> clazz, final String fname) { + Field getField(final String fname) { return fieldCache.get(fname); } @@ -75,87 +126,129 @@ final class ClassMap { } /** - * Creates a map of all public fields of a given class. - * @param clazz the class to introspect - * @return the map of fields (may be the empty map, can not be null) - */ - private static Map<String, Field> createFieldCache(Class<?> clazz) { - Field[] fields = clazz.getFields(); - if (fields.length > 0) { - Map<String, Field> cache = new HashMap<String, Field>(); - for (Field field : fields) { - if (Modifier.isPublic(field.getModifiers()) && Permissions.allow(field)) { - cache.put(field.getName(), field); - } - } - return cache; - } else { - return Collections.emptyMap(); - } - } - - /** * Gets the methods names cached by this map. * @return the array of method names */ String[] getMethodNames() { - return methodCache.names(); + java.util.Set<String> set = byName.keySet(); + return set.toArray(new String[set.size()]); } /** * Gets all the methods with a given name from this map. * @param methodName the seeked methods name - * @return the array of methods + * @return the array of methods (null or non-empty) */ - Method[] get(final String methodName) { - return methodCache.get(methodName); + Method[] getMethods(final String methodName) { + List<Method> lm = byName.get(methodName); + if (lm != null && !lm.isEmpty()) { + return lm.toArray(new Method[lm.size()]); + } else { + return null; + } } /** * Find a Method using the method name and parameter objects. - * - * @param key the method key + *<p> + * Look in the methodMap for an entry. If found, + * it'll either be a CACHE_MISS, in which case we + * simply give up, or it'll be a Method, in which + * case, we return it. + *</p> + * <p> + * If nothing is found, then we must actually go + * and introspect the method from the MethodMap. + *</p> + * @param methodKey the method key * @return A Method object representing the method to invoke or null. * @throws MethodKey.AmbiguousException When more than one method is a match for the parameters. */ - Method findMethod(final MethodKey key) - throws MethodKey.AmbiguousException { - return methodCache.get(key); + Method getMethod(final MethodKey methodKey) throws MethodKey.AmbiguousException { + // Look up by key + Method cacheEntry = byKey.get(methodKey); + // We looked this up before and failed. + if (cacheEntry == CACHE_MISS) { + return null; + } else if (cacheEntry == null) { + try { + // That one is expensive... + List<Method> methodList = byName.get(methodKey.getMethod()); + if (methodList != null) { + cacheEntry = methodKey.getMostSpecificMethod(methodList); + } + if (cacheEntry == null) { + byKey.put(methodKey, CACHE_MISS); + } else { + byKey.put(methodKey, cacheEntry); + } + } catch (MethodKey.AmbiguousException ae) { + // that's a miss :-) + byKey.put(methodKey, CACHE_MISS); + throw ae; + } + } + + // Yes, this might just be null. + return cacheEntry; } /** * Populate the Map of direct hits. These are taken from all the public methods * that our class, its parents and their implemented interfaces provide. + * @param cache the ClassMap instance we create * @param classToReflect the class to cache * @param log the Log - * @return a newly allocated & filled up cache */ - private static MethodCache createMethodCache(Class<?> classToReflect, Log log) { + private static void create(ClassMap cache, Class<?> classToReflect, Log log) { // // Build a list of all elements in the class hierarchy. This one is bottom-first (i.e. we start // with the actual declaring class and its interfaces and then move up (superclass etc.) until we // hit java.lang.Object. That is important because it will give us the methods of the declaring class // which might in turn be abstract further up the tree. // - // We also ignore all SecurityExceptions that might happen due to SecurityManager restrictions (prominently - // hit with Tomcat 5.5). - // - // We can also omit all that complicated getPublic, getAccessible and upcast logic that the class map had up - // until Velocity 1.4. As we always reflect all elements of the tree (that's what we have a cache for), we will - // hit the public elements sooner or later because we reflect all the public elements anyway. + // We also ignore all SecurityExceptions that might happen due to SecurityManager restrictions. // - // Ah, the miracles of Java for(;;) ... - MethodCache cache = new MethodCache(); for (; classToReflect != null; classToReflect = classToReflect.getSuperclass()) { if (Modifier.isPublic(classToReflect.getModifiers())) { - populateMethodCacheWith(cache, classToReflect, log); + populateWithClass(cache, classToReflect, log); } Class<?>[] interfaces = classToReflect.getInterfaces(); for (int i = 0; i < interfaces.length; i++) { - populateMethodCacheWithInterface(cache, interfaces[i], log); + populateWithInterface(cache, interfaces[i], log); + } + } + // now that we've got all methods keyed in, lets organize them by name + if (!cache.byKey.isEmpty()) { + List<Method> lm = new ArrayList<Method>(cache.byKey.size()); + for (Method method : cache.byKey.values()) { + lm.add(method); + } + // sort all methods by name + Collections.sort(lm, new Comparator<Method>() { + @Override + public int compare(Method o1, Method o2) { + return o1.getName().compareTo(o2.getName()); + } + }); + // put all lists of methods with same name in byName cache + int start = 0; + while (start < lm.size()) { + String name = lm.get(start).getName(); + int end = start + 1; + while (end < lm.size()) { + String walk = lm.get(end).getName(); + if (walk.equals(name)) { + end += 1; + } else { + break; + } + } + List<Method> lmn = lm.subList(start, end); + cache.byName.put(name, lmn); + start = end; } } - return cache; } /** @@ -164,13 +257,13 @@ final class ClassMap { * @param iface the interface to populate the cache from * @param log the Log */ - private static void populateMethodCacheWithInterface(MethodCache cache, Class<?> iface, Log log) { + private static void populateWithInterface(ClassMap cache, Class<?> iface, Log log) { if (Modifier.isPublic(iface.getModifiers())) { - populateMethodCacheWith(cache, iface, log); + populateWithClass(cache, iface, log); } Class<?>[] supers = iface.getInterfaces(); for (int i = 0; i < supers.length; i++) { - populateMethodCacheWithInterface(cache, supers[i], log); + populateWithInterface(cache, supers[i], log); } } @@ -180,13 +273,14 @@ final class ClassMap { * @param clazz the class to populate the cache from * @param log the Log */ - private static void populateMethodCacheWith(MethodCache cache, Class<?> clazz, Log log) { + private static void populateWithClass(ClassMap cache, Class<?> clazz, Log log) { try { Method[] methods = clazz.getDeclaredMethods(); for (int i = 0; i < methods.length; i++) { Method mi = methods[i]; if (Modifier.isPublic(mi.getModifiers()) && Permissions.allow(mi)) { - cache.put(methods[i]); + // add method to byKey cache; do not override + cache.byKey.putIfAbsent(new MethodKey(mi), mi); } } } catch (SecurityException se) { @@ -196,169 +290,4 @@ final class ClassMap { } } } - - /** - * This is the cache to store and look up the method information. - * <p> - * It stores the association between: - * - a key made of a method name & an array of argument types. - * - a method. - * </p> - * <p> - * Since the invocation of the associated method is dynamic, there is no need (nor way) to differentiate between - * foo(int,int) & foo(Integer,Integer) since in practise, only the latter form will be used through a call. - * This of course, applies to all 8 primitive types. - * </p> - */ - static final class MethodCache { - /** - * A method that returns itself used as a marker for cache miss, - * allows the underlying cache map to be strongly typed. - * @return itself as a method - */ - public static Method cacheMiss() { - try { - return MethodCache.class.getMethod("cacheMiss"); - } catch (Exception xio) { - // this really cant make an error... - return null; - } - } - /** The cache miss marker method. */ - private static final Method CACHE_MISS = cacheMiss(); - /** The initial size of the primitive conversion map. */ - private static final int PRIMITIVE_SIZE = 13; - /** The primitive type to class conversion map. */ - private static final Map<Class<?>, Class<?>> PRIMITIVE_TYPES; - - static { - PRIMITIVE_TYPES = new HashMap<Class<?>, Class<?>>(PRIMITIVE_SIZE); - PRIMITIVE_TYPES.put(Boolean.TYPE, Boolean.class); - PRIMITIVE_TYPES.put(Byte.TYPE, Byte.class); - PRIMITIVE_TYPES.put(Character.TYPE, Character.class); - PRIMITIVE_TYPES.put(Double.TYPE, Double.class); - PRIMITIVE_TYPES.put(Float.TYPE, Float.class); - PRIMITIVE_TYPES.put(Integer.TYPE, Integer.class); - PRIMITIVE_TYPES.put(Long.TYPE, Long.class); - PRIMITIVE_TYPES.put(Short.TYPE, Short.class); - } - - /** Converts a primitive type to its corresponding class. - * <p> - * If the argument type is primitive then we want to convert our - * primitive type signature to the corresponding Object type so - * introspection for methods with primitive types will work - * correctly. - * </p> - * @param parm a may-be primitive type class - * @return the equivalent object class - */ - static Class<?> primitiveClass(Class<?> parm) { - // it is marginally faster to get from the map than call isPrimitive... - //if (!parm.isPrimitive()) return parm; - Class<?> prim = PRIMITIVE_TYPES.get(parm); - return prim == null ? parm : prim; - } - /** - * The method cache. - * <p> - * Cache of Methods, or CACHE_MISS, keyed by method - * name and actual arguments used to find it. - * </p> - */ - private final Map<MethodKey, Method> methods = new HashMap<MethodKey, Method>(); - /** - * Map of methods that are searchable according to method parameters to find a match. - */ - private final MethodMap methodMap = new MethodMap(); - - /** - * Find a Method using the method name and parameter objects. - *<p> - * Look in the methodMap for an entry. If found, - * it'll either be a CACHE_MISS, in which case we - * simply give up, or it'll be a Method, in which - * case, we return it. - *</p> - * <p> - * If nothing is found, then we must actually go - * and introspect the method from the MethodMap. - *</p> - * @param methodKey the method key - * @return A Method object representing the method to invoke or null. - * @throws MethodKey.AmbiguousException When more than one method is a match for the parameters. - */ - Method get(final MethodKey methodKey) throws MethodKey.AmbiguousException { - synchronized (methodMap) { - Method cacheEntry = methods.get(methodKey); - // We looked this up before and failed. - if (cacheEntry == CACHE_MISS) { - return null; - } - - if (cacheEntry == null) { - try { - // That one is expensive... - cacheEntry = methodMap.find(methodKey); - if (cacheEntry != null) { - methods.put(methodKey, cacheEntry); - } else { - methods.put(methodKey, CACHE_MISS); - } - } catch (MethodKey.AmbiguousException ae) { - // that's a miss :-) - methods.put(methodKey, CACHE_MISS); - throw ae; - } - } - - // Yes, this might just be null. - return cacheEntry; - } - } - - /** - * Adds a method to the map. - * @param method the method to add - */ - void put(Method method) { - synchronized (methodMap) { - MethodKey methodKey = new MethodKey(method); - // We don't overwrite methods. Especially not if we fill the - // cache from defined class towards java.lang.Object because - // abstract methods in superclasses would else overwrite concrete - // classes further down the hierarchy. - if (methods.get(methodKey) == null) { - methods.put(methodKey, method); - methodMap.add(method); - } - } - } - - /** - * Gets all the method names from this map. - * @return the array of method name - */ - String[] names() { - synchronized (methodMap) { - return methodMap.names(); - } - } - - /** - * Gets all the methods with a given name from this map. - * @param methodName the seeked methods name - * @return the array of methods (null or non-empty) - */ - Method[] get(final String methodName) { - synchronized (methodMap) { - List<Method> lm = methodMap.get(methodName); - if (lm != null && !lm.isEmpty()) { - return lm.toArray(new Method[lm.size()]); - } else { - return null; - } - } - } - } -} \ No newline at end of file +} Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java?rev=1212995&r1=1212994&r2=1212995&view=diff ============================================================================== --- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java (original) +++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java Sun Dec 11 11:49:31 2011 @@ -29,6 +29,9 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + /** * This basic function of this class is to return a Method object for a * particular class given the name of a method and the parameters to the method @@ -51,17 +54,33 @@ import java.util.Map; * @since 1.0 */ public final class Introspector { - /** the logger. */ - protected final Log rlog; /** - * Holds the method maps for the classes we know about, keyed by Class. + * A Constructor get cache-miss. */ - private final Map<Class<?>, ClassMap> classMethodMaps = new HashMap<Class<?>, ClassMap>(); + private static class CacheMiss { + /** The constructor used as cache-miss. */ + @SuppressWarnings("unused") + public CacheMiss() { + } + } + /** The cache-miss marker for the constructors map. */ + private static final Constructor<?> CTOR_MISS = CacheMiss.class.getConstructors()[0]; + + /** the logger. */ + protected final Log rlog; /** * The class loader used to solve constructors if needed. */ private ClassLoader loader; /** + * The read/write lock. + */ + private final ReadWriteLock lock = new ReentrantReadWriteLock(); + /** + * Holds the method maps for the classes we know about, keyed by Class. + */ + private final Map<Class<?>, ClassMap> classMethodMaps = new HashMap<Class<?>, ClassMap>(); + /** * Holds the map of classes ctors we know about as well as unknown ones. */ private final Map<MethodKey, Constructor<?>> constructorsMap = new HashMap<MethodKey, Constructor<?>>(); @@ -91,7 +110,7 @@ public final class Introspector { return null; } } - + /** * Gets a method defined by a class, a name and a set of parameters. * @param c the class @@ -103,6 +122,7 @@ public final class Introspector { public Method getMethod(Class<?> c, String name, Object[] params) { return getMethod(c, new MethodKey(name, params)); } + /** * Gets the method defined by the <code>MethodKey</code> for the class <code>c</code>. * @@ -113,7 +133,7 @@ public final class Introspector { */ public Method getMethod(Class<?> c, MethodKey key) { try { - return getMap(c).findMethod(key); + return getMap(c).getMethod(key); } catch (MethodKey.AmbiguousException xambiguous) { // whoops. Ambiguous. Make a nice log message and return null... if (rlog != null && rlog.isInfoEnabled()) { @@ -133,7 +153,7 @@ public final class Introspector { * @return the desired field or null if it does not exist or is not accessible * */ public Field getField(Class<?> c, String key) { - return getMap(c).findField(c, key); + return getMap(c).getField(key); } /** @@ -173,22 +193,132 @@ public final class Introspector { return null; } ClassMap classMap = getMap(c); - return classMap.get(methodName); + return classMap.getMethods(methodName); } + + /** - * A Constructor get cache-miss. + * Gets the constructor defined by the <code>MethodKey</code>. + * + * @param key Key of the constructor being searched for + * @return The desired constructor object + * or null if no unambiguous constructor could be found through introspection. */ - private static class CacheMiss { - /** The constructor used as cache-miss. */ - @SuppressWarnings("unused") - public CacheMiss() {} + public Constructor<?> getConstructor(final MethodKey key) { + return getConstructor(null, key); + } + + /** + * Gets the constructor defined by the <code>MethodKey</code>. + * @param c the class we want to instantiate + * @param key Key of the constructor being searched for + * @return The desired constructor object + * or null if no unambiguous constructor could be found through introspection. + */ + public Constructor<?> getConstructor(final Class<?> c, final MethodKey key) { + Constructor<?> ctor = null; + try { + lock.readLock().lock(); + ctor = constructorsMap.get(key); + if (ctor != null) { + // miss or not? + return CTOR_MISS.equals(ctor) ? null : ctor; + } + } finally { + lock.readLock().unlock(); + } + // let's introspect... + try { + lock.writeLock().lock(); + // again for kicks + ctor = constructorsMap.get(key); + if (ctor != null) { + // miss or not? + return CTOR_MISS.equals(ctor) ? null : ctor; + } + final String cname = key.getMethod(); + // do we know about this class? + Class<?> clazz = constructibleClasses.get(cname); + try { + // do find the most specific ctor + if (clazz == null) { + if (c != null && c.getName().equals(key.getMethod())) { + clazz = c; + } else { + clazz = loader.loadClass(cname); + } + // add it to list of known loaded classes + constructibleClasses.put(cname, clazz); + } + List<Constructor<?>> l = new LinkedList<Constructor<?>>(); + for (Constructor<?> ictor : clazz.getConstructors()) { + if (Modifier.isPublic(ictor.getModifiers()) && Permissions.allow(ictor)) { + l.add(ictor); + } + } + // try to find one + ctor = key.getMostSpecificConstructor(l); + if (ctor != null) { + constructorsMap.put(key, ctor); + } else { + constructorsMap.put(key, CTOR_MISS); + } + } catch (ClassNotFoundException xnotfound) { + if (rlog != null && rlog.isInfoEnabled()) { + rlog.info("unable to find class: " + + cname + "." + + key.debugString(), xnotfound); + } + ctor = null; + } catch (MethodKey.AmbiguousException xambiguous) { + if (rlog != null && rlog.isInfoEnabled()) { + rlog.info("ambiguous constructor invocation: " + + cname + "." + + key.debugString(), xambiguous); + } + ctor = null; + } + return ctor; + } finally { + lock.writeLock().unlock(); + } } - - /** The cache-miss marker for the constructors map. */ - private static final Constructor<?> CTOR_MISS = CacheMiss.class.getConstructors()[0]; /** + * Gets the ClassMap for a given class. + * @param c the class + * @return the class map + */ + private ClassMap getMap(Class<?> c) { + ClassMap classMap; + try { + lock.readLock().lock(); + classMap = classMethodMaps.get(c); + } finally { + lock.readLock().unlock(); + } + if (classMap == null) { + try { + lock.writeLock().lock(); + // try again + classMap = classMethodMaps.get(c); + if (classMap == null) { + classMap = classMethodMaps.get(c); + if (classMap == null) { + classMap = new ClassMap(c, rlog); + classMethodMaps.put(c, classMap); + } + } + } finally { + lock.writeLock().unlock(); + } + + } + return classMap; + } + + /** * Sets the class loader used to solve constructors. * <p>Also cleans the constructors and methods caches.</p> * @param cloader the class loader; if null, use this instance class loader @@ -246,95 +376,4 @@ public final class Introspector { } return false; } - - /** - * Gets the constructor defined by the <code>MethodKey</code>. - * - * @param key Key of the constructor being searched for - * @return The desired constructor object - * or null if no unambiguous constructor could be found through introspection. - */ - public Constructor<?> getConstructor(final MethodKey key) { - return getConstructor(null, key); - } - - /** - * Gets the constructor defined by the <code>MethodKey</code>. - * @param c the class we want to instantiate - * @param key Key of the constructor being searched for - * @return The desired constructor object - * or null if no unambiguous constructor could be found through introspection. - */ - public Constructor<?> getConstructor(final Class<?> c, final MethodKey key) { - Constructor<?> ctor = null; - synchronized (constructorsMap) { - ctor = constructorsMap.get(key); - // that's a clear miss - if (CTOR_MISS.equals(ctor)) { - return null; - } - // let's introspect... - if (ctor == null) { - final String cname = key.getMethod(); - // do we know about this class? - Class<?> clazz = constructibleClasses.get(cname); - try { - // do find the most specific ctor - if (clazz == null) { - if (c != null && c.getName().equals(key.getMethod())) { - clazz = c; - } else { - clazz = loader.loadClass(cname); - } - // add it to list of known loaded classes - constructibleClasses.put(cname, clazz); - } - List<Constructor<?>> l = new LinkedList<Constructor<?>>(); - for (Constructor<?> ictor : clazz.getConstructors()) { - if (Modifier.isPublic(ictor.getModifiers()) && Permissions.allow(ictor)) { - l.add(ictor); - } - } - // try to find one - ctor = key.getMostSpecificConstructor(l); - if (ctor != null) { - constructorsMap.put(key, ctor); - } else { - constructorsMap.put(key, CTOR_MISS); - } - } catch (ClassNotFoundException xnotfound) { - if (rlog != null && rlog.isInfoEnabled()) { - rlog.info("unable to find class: " - + cname + "." - + key.debugString(), xnotfound); - } - ctor = null; - } catch (MethodKey.AmbiguousException xambiguous) { - if (rlog != null && rlog.isInfoEnabled()) { - rlog.info("ambiguous constructor invocation: " - + cname + "." - + key.debugString(), xambiguous); - } - ctor = null; - } - } - return ctor; - } - } - - /** - * Gets the ClassMap for a given class. - * @param c the class - * @return the class map - */ - private ClassMap getMap(Class<?> c) { - synchronized (classMethodMaps) { - ClassMap classMap = classMethodMaps.get(c); - if (classMap == null) { - classMap = new ClassMap(c, rlog); - classMethodMaps.put(c, classMap); - } - return classMap; - } - } } \ No newline at end of file Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java?rev=1212995&r1=1212994&r2=1212995&view=diff ============================================================================== --- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java (original) +++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java Sun Dec 11 11:49:31 2011 @@ -16,12 +16,15 @@ */ package org.apache.commons.jexl3.internal.introspection; -import java.util.List; -import java.util.LinkedList; -import java.util.Iterator; import java.lang.reflect.Constructor; import java.lang.reflect.Method; + import java.util.Arrays; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; /** * A method key usable by the introspector cache. @@ -43,6 +46,40 @@ import java.util.Arrays; * Roughly 3x faster than string key to access the map & uses less memory. */ public final class MethodKey { + /** The initial size of the primitive conversion map. */ + private static final int PRIMITIVE_SIZE = 13; + /** The primitive type to class conversion map. */ + private static final Map<Class<?>, Class<?>> PRIMITIVE_TYPES; + + static { + PRIMITIVE_TYPES = new HashMap<Class<?>, Class<?>>(PRIMITIVE_SIZE); + PRIMITIVE_TYPES.put(Boolean.TYPE, Boolean.class); + PRIMITIVE_TYPES.put(Byte.TYPE, Byte.class); + PRIMITIVE_TYPES.put(Character.TYPE, Character.class); + PRIMITIVE_TYPES.put(Double.TYPE, Double.class); + PRIMITIVE_TYPES.put(Float.TYPE, Float.class); + PRIMITIVE_TYPES.put(Integer.TYPE, Integer.class); + PRIMITIVE_TYPES.put(Long.TYPE, Long.class); + PRIMITIVE_TYPES.put(Short.TYPE, Short.class); + } + + /** Converts a primitive type to its corresponding class. + * <p> + * If the argument type is primitive then we want to convert our + * primitive type signature to the corresponding Object type so + * introspection for methods with primitive types will work + * correctly. + * </p> + * @param parm a may-be primitive type class + * @return the equivalent object class + */ + static Class<?> primitiveClass(Class<?> parm) { + // it is marginally faster to get from the map than call isPrimitive... + //if (!parm.isPrimitive()) return parm; + Class<?> prim = PRIMITIVE_TYPES.get(parm); + return prim == null ? parm : prim; + } + /** The hash code. */ private final int hashCode; /** The method name. */ @@ -80,7 +117,7 @@ public final class MethodKey { } this.hashCode = hash; } - + /** * Creates a key from a method. * @param aMethod the method to generate the key from. @@ -90,6 +127,14 @@ public final class MethodKey { } /** + * Creates a key from a constructor. + * @param aCtor the constructor to generate the key from. + */ + MethodKey(Constructor<?> aCtor) { + this(aCtor.getDeclaringClass().getName(), aCtor.getParameterTypes()); + } + + /** * Creates a key from a method name and a set of parameters. * @param aMethod the method to generate the key from * @param args the intended method parameters @@ -104,7 +149,7 @@ public final class MethodKey { if (args != null && (size = args.length) > 0) { this.params = new Class<?>[size]; for (int p = 0; p < size; ++p) { - Class<?> parm = ClassMap.MethodCache.primitiveClass(args[p]); + Class<?> parm = primitiveClass(args[p]); hash = (HASH * hash) + parm.hashCode(); this.params[p] = parm; } @@ -192,7 +237,7 @@ public final class MethodKey { public Constructor<?> getMostSpecificConstructor(List<Constructor<?>> methods) { return CONSTRUCTORS.getMostSpecific(methods, params); } - + /** * Determines whether a type represented by a class object is * convertible to another type represented by a class object using a @@ -343,7 +388,6 @@ public final class MethodKey { } return false; } - /** * whether a method/ctor is more specific than a previously compared one. */ @@ -383,7 +427,19 @@ public final class MethodKey { // CSOFF: RedundantThrows /** - * Gets the most specific method that is applicable to actual argument types. + * Gets the most specific method that is applicable to actual argument types.<p> + * Attempts to find the most specific applicable method using the + * algorithm described in the JLS section 15.12.2 (with the exception that it can't + * distinguish a primitive type argument from an object type argument, since in reflection + * primitive type arguments are represented by their object counterparts, so for an argument of + * type (say) java.lang.Integer, it will not be able to decide between a method that takes int and a + * method that takes java.lang.Integer as a parameter. + * </p> + * <p> + * This turns out to be a relatively rare case where this is needed - however, functionality + * like this is needed. + * </p> + * * @param methods a list of methods. * @param classes list of argument types. * @return the most specific method. @@ -629,7 +685,6 @@ public final class MethodKey { return isStrictInvocationConvertible(formal, actual.equals(Void.class) ? null : actual, possibleVarArg); } } - /** * The parameter matching service for methods. */ @@ -639,7 +694,6 @@ public final class MethodKey { return app.getParameterTypes(); } }; - /** * The parameter matching service for constructors. */ Modified: commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/introspection/MethodKeyTest.java URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/introspection/MethodKeyTest.java?rev=1212995&r1=1212994&r2=1212995&view=diff ============================================================================== --- commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/introspection/MethodKeyTest.java (original) +++ commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/introspection/MethodKeyTest.java Sun Dec 11 11:49:31 2011 @@ -15,9 +15,9 @@ * limitations under the License. */ package org.apache.commons.jexl3.internal.introspection; -import org.apache.commons.jexl3.internal.introspection.MethodKey; -import org.apache.commons.jexl3.internal.introspection.ClassMap; + import junit.framework.TestCase; + /** * Checks the CacheMap.MethodKey implementation */ @@ -35,7 +35,6 @@ public class MethodKeyTest extends TestC String.class, java.util.Date.class }; - // A set of instances corresponding to the classes private static final Object[] ARGS = { Boolean.TRUE, @@ -45,11 +44,10 @@ public class MethodKeyTest extends TestC new Float(8f), new Integer(16), new Long(32l), - new Short((short)64), + new Short((short) 64), "foobar", new java.util.Date() }; - // A set of (pseudo) method names private static final String[] METHODS = { "plus", @@ -75,27 +73,26 @@ public class MethodKeyTest extends TestC "applyIt", "invokeIt" }; - /** from key to string */ private static final java.util.Map< MethodKey, String> byKey; /** form string to key */ - private static final java.util.Map<String,MethodKey> byString; + private static final java.util.Map<String, MethodKey> byString; /** the list of keys we generated & test against */ private static final MethodKey[] keyList; - + /** Creates & inserts a key into the byKey & byString map */ private static void setUpKey(String name, Class<?>[] parms) { MethodKey key = new MethodKey(name, parms); String str = key.toString(); byKey.put(key, str); byString.put(str, key); - + } /** Generate a list of method*(prims*), method(prims*, prims*), method*(prims*,prims*,prims*) */ static { byKey = new java.util.HashMap< MethodKey, String>(); - byString = new java.util.HashMap<String,MethodKey>(); + byString = new java.util.HashMap<String, MethodKey>(); for (int m = 0; m < METHODS.length; ++m) { String method = METHODS[m]; for (int p0 = 0; p0 < PRIMS.length; ++p0) { @@ -116,102 +113,105 @@ public class MethodKeyTest extends TestC /** Builds a string key */ String makeStringKey(String method, Class<?>... params) { - StringBuilder builder = new StringBuilder(method); - for(int p = 0; p < params.length; ++p) { - builder.append(ClassMap.MethodCache.primitiveClass(params[p]).getName()); - } - return builder.toString(); + StringBuilder builder = new StringBuilder(method); + for (int p = 0; p < params.length; ++p) { + builder.append(MethodKey.primitiveClass(params[p]).getName()); + } + return builder.toString(); } - + /** Checks that a string key does exist */ void checkStringKey(String method, Class<?>... params) { String key = makeStringKey(method, params); MethodKey out = byString.get(key); assertTrue(out != null); } - + /** Builds a method key */ MethodKey makeKey(String method, Class<?>... params) { return new MethodKey(method, params); } - + /** Checks that a method key exists */ void checkKey(String method, Class<?>... params) { MethodKey key = makeKey(method, params); String out = byKey.get(key); assertTrue(out != null); } - + public void testObjectKey() throws Exception { - for(int k = 0; k < keyList.length; ++k) { + for (int k = 0; k < keyList.length; ++k) { MethodKey ctl = keyList[k]; MethodKey key = makeKey(ctl.getMethod(), ctl.getParameters()); String out = byKey.get(key); assertTrue(out != null); assertTrue(ctl.toString() + " != " + out, ctl.toString().equals(out)); } - + } - + public void testStringKey() throws Exception { - for(int k = 0; k < keyList.length; ++k) { + for (int k = 0; k < keyList.length; ++k) { MethodKey ctl = keyList[k]; String key = makeStringKey(ctl.getMethod(), ctl.getParameters()); MethodKey out = byString.get(key); assertTrue(out != null); assertTrue(ctl.toString() + " != " + key, ctl.equals(out)); } - + } - private static final int LOOP = 3;//00; - + public void testPerfKey() throws Exception { - for(int l = 0; l < LOOP; ++l) - for(int k = 0; k < keyList.length; ++k) { - MethodKey ctl = keyList[k]; - MethodKey key = makeKey(ctl.getMethod(), ctl.getParameters()); - String out = byKey.get(key); - assertTrue(out != null); + for (int l = 0; l < LOOP; ++l) { + for (int k = 0; k < keyList.length; ++k) { + MethodKey ctl = keyList[k]; + MethodKey key = makeKey(ctl.getMethod(), ctl.getParameters()); + String out = byKey.get(key); + assertTrue(out != null); + } } } - + public void testPerfString() throws Exception { - for(int l = 0; l < LOOP; ++l) - for(int k = 0; k < keyList.length; ++k) { - MethodKey ctl = keyList[k]; - String key = makeStringKey(ctl.getMethod(), ctl.getParameters()); - MethodKey out = byString.get(key); - assertTrue(out != null); + for (int l = 0; l < LOOP; ++l) { + for (int k = 0; k < keyList.length; ++k) { + MethodKey ctl = keyList[k]; + String key = makeStringKey(ctl.getMethod(), ctl.getParameters()); + MethodKey out = byString.get(key); + assertTrue(out != null); + } } } - + public void testPerfKey2() throws Exception { - for(int l = 0; l < LOOP; ++l) - for (int m = 0; m < METHODS.length; ++m) { - String method = METHODS[m]; - for (int p0 = 0; p0 < ARGS.length; ++p0) { - checkKey(method, ARGS[p0].getClass()); - for (int p1 = 0; p1 < ARGS.length; ++p1) { - checkKey(method, ARGS[p0].getClass(), ARGS[p1].getClass()); - for (int p2 = 0; p2 < ARGS.length; ++p2) { - checkKey(method, ARGS[p0].getClass(), ARGS[p1].getClass(), ARGS[p2].getClass()); + for (int l = 0; l < LOOP; ++l) { + for (int m = 0; m < METHODS.length; ++m) { + String method = METHODS[m]; + for (int p0 = 0; p0 < ARGS.length; ++p0) { + checkKey(method, ARGS[p0].getClass()); + for (int p1 = 0; p1 < ARGS.length; ++p1) { + checkKey(method, ARGS[p0].getClass(), ARGS[p1].getClass()); + for (int p2 = 0; p2 < ARGS.length; ++p2) { + checkKey(method, ARGS[p0].getClass(), ARGS[p1].getClass(), ARGS[p2].getClass()); + } } } } } } - + public void testPerfStringKey2() throws Exception { - for(int l = 0; l < LOOP; ++l) - for (int m = 0; m < METHODS.length; ++m) { - String method = METHODS[m]; - for (int p0 = 0; p0 < ARGS.length; ++p0) { - checkStringKey(method, ARGS[p0].getClass()); - for (int p1 = 0; p1 < ARGS.length; ++p1) { - checkStringKey(method, ARGS[p0].getClass(), ARGS[p1].getClass()); - for (int p2 = 0; p2 < ARGS.length; ++p2) { - checkStringKey(method, ARGS[p0].getClass(), ARGS[p1].getClass(), ARGS[p2].getClass()); + for (int l = 0; l < LOOP; ++l) { + for (int m = 0; m < METHODS.length; ++m) { + String method = METHODS[m]; + for (int p0 = 0; p0 < ARGS.length; ++p0) { + checkStringKey(method, ARGS[p0].getClass()); + for (int p1 = 0; p1 < ARGS.length; ++p1) { + checkStringKey(method, ARGS[p0].getClass(), ARGS[p1].getClass()); + for (int p2 = 0; p2 < ARGS.length; ++p2) { + checkStringKey(method, ARGS[p0].getClass(), ARGS[p1].getClass(), ARGS[p2].getClass()); + } } } }