You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@velocity.apache.org by nb...@apache.org on 2008/07/25 20:21:31 UTC
svn commit: r679872 - in /velocity/engine/trunk: ./
src/java/org/apache/velocity/runtime/
src/java/org/apache/velocity/runtime/directive/
src/java/org/apache/velocity/runtime/resource/
src/java/org/apache/velocity/util/introspection/
Author: nbubna
Date: Fri Jul 25 11:21:31 2008
New Revision: 679872
URL: http://svn.apache.org/viewvc?rev=679872&view=rev
Log:
sync pom commons-lang version with build.properties
Modified:
velocity/engine/trunk/pom.xml
velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java
velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java
velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java
Modified: velocity/engine/trunk/pom.xml
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/pom.xml?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/pom.xml (original)
+++ velocity/engine/trunk/pom.xml Fri Jul 25 11:21:31 2008
@@ -138,7 +138,7 @@
<dependency>
<groupId>commons-lang</groupId>
<artifactId>commons-lang</artifactId>
- <version>2.1</version>
+ <version>2.4</version>
</dependency>
<dependency>
<groupId>oro</groupId>
Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java Fri Jul 25 11:21:31 2008
@@ -94,17 +94,6 @@
*/
private Map libModMap;
- /*
- * Map for holding macro execution information. An executing macro will
- * be identified by the pair <tamplateName, macroName>.
- */
- private final Map templateMap;
-
- /*
- * Private variable for holding the allowed max calling depth.
- */
- private int maxCallingDepth;
-
/**
* C'tor for the VelociMacro factory.
*
@@ -122,96 +111,8 @@
*/
libModMap = new HashMap();
vmManager = new VelocimacroManager(rsvc);
- templateMap = new HashMap();
}
- /**
- * This method is called before a macro is rendered. This method
- * checks whether a macro call is within the allowed calling depth.
- * If the macro call exceeds the allowed calling depth it will throw
- * an exception.
- *
- * @param macroName name of the macro
- * @param templateName name of the template file containing the macro
- * @throws MacroOverflowException if the number of macro calls exceeds the specified value
- */
- public void startMacroRendering(String macroName, String templateName)
- throws MacroOverflowException
- {
- maxCallingDepth = rsvc.getInt(
- RuntimeConstants.VM_MAX_DEPTH);
-
- /*
- * If this property is set to 0 or minus value we do not keep track
- * of the macro execution
- */
- if (maxCallingDepth > 0)
- {
- synchronized(templateMap)
- {
- Stack macroStack = (Stack)templateMap.get(templateName);
- if (macroStack != null)
- {
- /*
- * If the macro stack size is larger than or equal to
- * maxCallingDepth allowed throw an exception
- */
- if (macroStack.size() >= maxCallingDepth)
- {
- log.error("Max calling depth exceded in Template:" +
- templateName + "and Macro:" + macroName);
-
- String message = "Exceed maximum " + maxCallingDepth +
- " macro calls. Call Stack:";
- /*
- * Construct the message from the stack
- */
- for (int i = 0; i < macroStack.size() - 1; i++)
- {
- message += macroStack.get(i) + "->";
- }
- message += macroStack.peek();
-
- /*
- Clean up the template map
- */
- templateMap.remove(templateName);
- throw new MacroOverflowException(message);
- }
- macroStack.push(macroName);
- }
- else
- {
- macroStack = new Stack();
- macroStack.push(macroName);
- templateMap.put(templateName, macroStack);
- }
- }
- }
- }
-
- /**
- * This method is called when a macro finishes rendering. Clears the state
- * saved about the macro call.
- *
- * @param macroName name of the macro
- * @param templateName template name containg the macro
- */
- public void endMacroRendering(String macroName, String templateName)
- {
- String name = null;
- Stack macroStack = (Stack)templateMap.get(templateName);
- if (macroStack != null && macroStack.size() > 0)
- {
- macroStack.pop();
- }
- if (macroStack != null && macroStack.size() == 0)
- {
- templateMap.remove(templateName);
- }
- }
-
-
/**
* initialize the factory - setup all permissions
* load all global libraries.
Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java Fri Jul 25 11:21:31 2008
@@ -19,6 +19,7 @@
* under the License.
*/
+import org.apache.commons.lang.text.StrBuilder;
import org.apache.velocity.context.InternalContextAdapter;
import org.apache.velocity.runtime.parser.node.Node;
import org.apache.velocity.runtime.parser.Token;
@@ -124,26 +125,35 @@
this.context = context;
this.node = node;
- StringBuffer buffer = new StringBuffer();
Token t = node.getFirstToken();
- /**
- * Retrieve the literal text
- */
- while (t != null && t != node.getLastToken())
+ if (t == node.getLastToken())
{
- buffer.append(t.image);
- t = t.next;
+ literal = t.image;
}
-
- if (t != null)
+ else
{
- buffer.append(t.image);
+ // guessing that most macros are much longer than
+ // the 32 char default capacity. let's guess 4x bigger :)
+ StrBuilder text = new StrBuilder(128);
+ /**
+ * Retrieve the literal text
+ */
+ while (t != null && t != node.getLastToken())
+ {
+ text.append(t.image);
+ t = t.next;
+ }
+ if (t != null)
+ {
+ text.append(t.image);
+ }
+
+ /**
+ * Store the literal text
+ */
+ literal = text.toString();
}
- /**
- * Store the literal text
- */
- literal = buffer.toString();
}
/**
Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java Fri Jul 25 11:21:31 2008
@@ -85,6 +85,11 @@
protected Object data = null;
/**
+ * Resource type (RESOURCE_TEMPLATE or RESOURCE_CONTENT)
+ */
+ protected int type;
+
+ /**
* Default constructor
*/
public Resource()
@@ -267,4 +272,20 @@
{
return data;
}
+
+ /**
+ * Sets the type of this Resource (RESOURCE_TEMPLATE or RESOURCE_CONTENT)
+ */
+ public void setType(int type)
+ {
+ this.type = type;
+ }
+
+ /**
+ * @return type code of the Resource
+ */
+ public int getType()
+ {
+ return type;
+ }
}
Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java Fri Jul 25 11:21:31 2008
@@ -99,11 +99,11 @@
public synchronized void initialize(final RuntimeServices rsvc)
throws Exception
{
- if (isInit)
- {
- log.error("Re-initialization of ResourceLoader attempted!");
- return;
- }
+ if (isInit)
+ {
+ log.error("Re-initialization of ResourceLoader attempted!");
+ return;
+ }
ResourceLoader resourceLoader = null;
@@ -259,6 +259,9 @@
* Gets the named resource. Returned class type corresponds to specified type (i.e. <code>Template</code> to <code>
* RESOURCE_TEMPLATE</code>).
*
+ * This method is now unsynchronized which requires that ResourceCache
+ * implementations be thread safe (as the default is).
+ *
* @param resourceName The name of the resource to retrieve.
* @param resourceType The type of resource (<code>RESOURCE_TEMPLATE</code>, <code>RESOURCE_CONTENT</code>, etc.).
* @param encoding The character encoding to use.
@@ -269,7 +272,7 @@
* @throws ParseErrorException if template cannot be parsed due to syntax (or other) error.
* @throws Exception if a problem in parse
*/
- public synchronized Resource getResource(final String resourceName, final int resourceType, final String encoding)
+ public Resource getResource(final String resourceName, final int resourceType, final String encoding)
throws ResourceNotFoundException,
ParseErrorException,
Exception
@@ -289,13 +292,29 @@
if (resource != null)
{
- /*
- * refresh the resource
- */
-
try
{
- refreshResource(resource, encoding);
+ // avoids additional method call to refreshResource
+ if (resource.requiresChecking())
+ {
+ /*
+ * both loadResource() and refreshResource() now return
+ * a new Resource instance when they are called
+ * (put in the cache when appropriate) in order to allow
+ * several threads to parse the same template simultaneously.
+ * It is redundant work and will cause more garbage collection but the
+ * benefit is that it allows concurrent parsing and processing
+ * without race conditions when multiple requests try to
+ * refresh/load the same template at the same time.
+ *
+ * Another alternative is to limit template parsing/retrieval
+ * so that only one thread can parse each template at a time
+ * but that creates a scalability bottleneck.
+ *
+ * See VELOCITY-606, VELOCITY-595 and VELOCITY-24
+ */
+ resource = refreshResource(resource, encoding);
+ }
}
catch (ResourceNotFoundException rnfe)
{
@@ -316,7 +335,7 @@
}
catch (RuntimeException re)
{
- throw re;
+ throw re;
}
catch (Exception e)
{
@@ -330,8 +349,7 @@
{
/*
* it's not in the cache, so load it.
- */
-
+ */
resource = loadResource(resourceName, resourceType, encoding);
if (resource.getResourceLoader().isCachingOn())
@@ -352,7 +370,7 @@
}
catch (RuntimeException re)
{
- throw re;
+ throw re;
}
catch (Exception e)
{
@@ -383,9 +401,7 @@
Exception
{
Resource resource = ResourceFactory.getResource(resourceName, resourceType);
-
resource.setRuntimeServices(rsvc);
-
resource.setName(resourceName);
resource.setEncoding(encoding);
@@ -475,12 +491,11 @@
* @throws ParseErrorException if template cannot be parsed due to syntax (or other) error.
* @throws Exception if a problem in parse
*/
- protected void refreshResource(final Resource resource, final String encoding)
+ protected Resource refreshResource(Resource resource, final String encoding)
throws ResourceNotFoundException,
ParseErrorException,
Exception
{
-
/*
* The resource knows whether it needs to be checked
* or not, and the resource's loader can check to
@@ -489,52 +504,58 @@
* the input stream and parse it to make a new
* AST for the resource.
*/
- if (resource.requiresChecking())
+
+ /*
+ * touch() the resource to reset the counters
+ */
+ resource.touch();
+
+ if (resource.isSourceModified())
{
/*
- * touch() the resource to reset the counters
+ * now check encoding info. It's possible that the newly declared
+ * encoding is different than the encoding already in the resource
+ * this strikes me as bad...
*/
- resource.touch();
-
- if (resource.isSourceModified())
+ if (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), encoding))
{
- /*
- * now check encoding info. It's possible that the newly declared
- * encoding is different than the encoding already in the resource
- * this strikes me as bad...
- */
-
- if (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), encoding))
- {
- log.warn("Declared encoding for template '" +
+ log.warn("Declared encoding for template '" +
resource.getName() +
"' is different on reload. Old = '" +
resource.getEncoding() + "' New = '" + encoding);
- resource.setEncoding(encoding);
- }
-
- /*
- * read how old the resource is _before_
- * processing (=>reading) it
- */
- long howOldItWas = resource.getResourceLoader().getLastModified(resource);
+ resource.setEncoding(encoding);
+ }
- /*
- * read in the fresh stream and parse
- */
+ /*
+ * read how old the resource is _before_
+ * processing (=>reading) it
+ */
+ long howOldItWas = resource.getResourceLoader().getLastModified(resource);
- resource.process();
+ String resourceKey = resource.getType() + resource.getName();
- /*
- * now set the modification info and reset
- * the modification check counters
- */
+ /*
+ * we create a copy to avoid partially overwriting a
+ * template which may be in use in another thread
+ */
+
+ Resource newResource =
+ ResourceFactory.getResource(resource.getName(), resource.getType());
+
+ newResource.setRuntimeServices(rsvc);
+ newResource.setName(resource.getName());
+ newResource.setEncoding(resource.getEncoding());
+ newResource.setResourceLoader(resource.getResourceLoader());
+
+ newResource.process();
+ newResource.setLastModified(howOldItWas);
+ resource = newResource;
- resource.setLastModified(howOldItWas);
- }
+ globalCache.put(resourceKey, newResource);
}
+ return resource;
}
/**
Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java Fri Jul 25 11:21:31 2008
@@ -26,7 +26,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-
+import org.apache.commons.lang.text.StrBuilder;
import org.apache.velocity.runtime.log.Log;
/**
@@ -39,6 +39,7 @@
* @author <a href="mailto:szegedia@freemail.hu">Attila Szegedi</a>
* @author <a href="mailto:geirm@optonline.net">Geir Magnusson Jr.</a>
* @author <a href="mailto:henning@apache.org">Henning P. Schmiedehausen</a>
+ * @author Nathan Bubna
* @version $Id$
*/
public class ClassMap
@@ -72,9 +73,7 @@
log.debug("== Class: " + clazz);
}
- methodCache = new MethodCache(log);
-
- populateMethodCache();
+ methodCache = createMethodCache();
if (debugReflection && log.isDebugEnabled())
{
@@ -111,10 +110,11 @@
* are taken from all the public methods
* that our class, its parents and their implemented interfaces provide.
*/
- private void populateMethodCache()
+ private MethodCache createMethodCache()
{
+ MethodCache methodCache = new MethodCache(log);
//
- // Build a list of all elements in the class hierarchy. This one is bottom-first (i.e. we start
+ // Looks through 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.
@@ -126,70 +126,60 @@
// 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.
//
- List classesToReflect = new ArrayList();
-
// Ah, the miracles of Java for(;;) ...
for (Class classToReflect = getCachedClass(); classToReflect != null ; classToReflect = classToReflect.getSuperclass())
{
if (Modifier.isPublic(classToReflect.getModifiers()))
{
- classesToReflect.add(classToReflect);
- if (debugReflection && log.isDebugEnabled())
- {
- log.debug("Adding " + classToReflect + " for reflection");
- }
+ populateMethodCacheWith(methodCache, classToReflect);
}
Class [] interfaces = classToReflect.getInterfaces();
for (int i = 0; i < interfaces.length; i++)
{
if (Modifier.isPublic(interfaces[i].getModifiers()))
{
- classesToReflect.add(interfaces[i]);
- if (debugReflection && log.isDebugEnabled())
- {
- log.debug("Adding " + interfaces[i] + " for reflection");
- }
+ populateMethodCacheWith(methodCache, interfaces[i]);
}
}
}
+ // return the already initialized cache
+ return methodCache;
+ }
- for (Iterator it = classesToReflect.iterator(); it.hasNext(); )
+ private void populateMethodCacheWith(MethodCache methodCache, Class classToReflect)
+ {
+ if (debugReflection && log.isDebugEnabled())
{
- Class classToReflect = (Class) it.next();
- if (debugReflection && log.isDebugEnabled())
- {
- log.debug("Reflecting " + classToReflect);
- }
-
+ log.debug("Reflecting " + classToReflect);
+ }
- try
- {
- Method[] methods = classToReflect.getMethods();
+ try
+ {
+ Method[] methods = classToReflect.getDeclaredMethods();
- for (int i = 0; i < methods.length; i++)
+ for (int i = 0; i < methods.length; i++)
+ {
+ // Strictly spoken that check shouldn't be necessary
+ // because getMethods only returns public methods.
+ int modifiers = methods[i].getModifiers();
+ if (Modifier.isPublic(modifiers)) // && !)
{
- // Strictly spoken that check shouldn't be necessary
- // because getMethods only returns public methods.
- int modifiers = methods[i].getModifiers();
- if (Modifier.isPublic(modifiers)) // && !)
- {
- // Some of the interfaces contain abstract methods. That is fine, because the actual object must
- // implement them anyway (else it wouldn't be implementing the interface). If we find an abstract
- // method in a non-interface, we skip it, because we do want to make sure that no abstract methods end up in
- // the cache.
- if (classToReflect.isInterface() || !Modifier.isAbstract(modifiers))
- {
- methodCache.put(methods[i]);
- }
+ // Some of the interfaces contain abstract methods. That is fine, because the actual object must
+ // implement them anyway (else it wouldn't be implementing the interface). If we find an abstract
+ // method in a non-interface, we skip it, because we do want to make sure that no abstract methods end up in
+ // the cache.
+ if (classToReflect.isInterface() || !Modifier.isAbstract(modifiers))
+ {
+ methodCache.put(methods[i]);
}
}
}
- catch (SecurityException se) // Everybody feels better with...
+ }
+ catch (SecurityException se) // Everybody feels better with...
+ {
+ if (log.isDebugEnabled())
{
- if (log.isDebugEnabled())
- {
- log.debug("While accessing methods of " + classToReflect + ": ", se);
- }
+ log.debug("While accessing methods of " + classToReflect + ": ", se);
}
}
}
@@ -202,11 +192,9 @@
*/
private static final class MethodCache
{
- private static final class CacheMiss { }
-
- private static final CacheMiss CACHE_MISS = new CacheMiss();
+ private static final Object CACHE_MISS = new Object();
- private static final Object OBJECT = new Object();
+ private static final String NULL_ARG = new Object().getClass().getName();
private static final Map convertPrimitives = new HashMap();
@@ -230,6 +218,7 @@
* name and actual arguments used to find it.
*/
private final Map cache = new HashMap();
+ private final Map locks = new HashMap();
/** Map of methods that are searchable according to method parameters to find a match */
private final MethodMap methodMap = new MethodMap();
@@ -255,52 +244,55 @@
* @return A Method object representing the method to invoke or null.
* @throws MethodMap.AmbiguousException When more than one method is a match for the parameters.
*/
- public synchronized Method get(final String name, final Object [] params)
+ public Method get(final String name, final Object [] params)
throws MethodMap.AmbiguousException
{
- String methodKey = makeMethodKey(name, params);
+ String methodKey = getLock(makeMethodKey(name, params));
- Object cacheEntry = cache.get(methodKey);
-
- // We looked this up before and failed.
- if (cacheEntry == CACHE_MISS)
+ synchronized (methodKey)
{
- return null;
- }
+ Object cacheEntry = cache.get(methodKey);
- if (cacheEntry == null)
- {
- try
+ // We looked this up before and failed.
+ if (cacheEntry == CACHE_MISS)
{
- // That one is expensive...
- cacheEntry = methodMap.find(name, params);
+ return null;
}
- catch(MethodMap.AmbiguousException ae)
+
+ if (cacheEntry == null)
{
- /*
- * that's a miss :-)
- */
- cache.put(methodKey, CACHE_MISS);
- throw ae;
- }
+ try
+ {
+ // That one is expensive...
+ cacheEntry = methodMap.find(name, params);
+ }
+ catch(MethodMap.AmbiguousException ae)
+ {
+ /*
+ * that's a miss :-)
+ */
+ cache.put(methodKey, CACHE_MISS);
+ throw ae;
+ }
- cache.put(methodKey,
- (cacheEntry != null) ? cacheEntry : CACHE_MISS);
- }
+ cache.put(methodKey,
+ (cacheEntry != null) ? cacheEntry : CACHE_MISS);
+ }
- // Yes, this might just be null.
+ // Yes, this might just be null.
- return (Method) cacheEntry;
+ return (Method) cacheEntry;
+ }
}
- public synchronized void put(Method method)
+ private void put(Method method)
{
String methodKey = makeMethodKey(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.
+
+ // We don't overwrite methods because we fill the
+ // cache from defined class towards java.lang.Object
+ // and that would cause overridden methods to appear
+ // as if they were not overridden.
if (cache.get(methodKey) == null)
{
cache.put(methodKey, method);
@@ -312,6 +304,18 @@
}
}
+ private final String getLock(String key) {
+ synchronized (locks)
+ {
+ String lock = (String)locks.get(key);
+ if (lock == null)
+ {
+ return key;
+ }
+ return lock;
+ }
+ }
+
/**
* Make a methodKey for the given method using
* the concatenation of the name and the
@@ -323,10 +327,15 @@
private String makeMethodKey(final Method method)
{
Class[] parameterTypes = method.getParameterTypes();
+ int args = parameterTypes.length;
+ if (args == 0)
+ {
+ return method.getName();
+ }
- StringBuffer methodKey = new StringBuffer(method.getName());
+ StrBuilder methodKey = new StrBuilder((args+1)*16).append(method.getName());
- for (int j = 0; j < parameterTypes.length; j++)
+ for (int j = 0; j < args; j++)
{
/*
* If the argument type is primitive then we want
@@ -353,18 +362,25 @@
private String makeMethodKey(String method, Object[] params)
{
- StringBuffer methodKey = new StringBuffer().append(method);
+ int args = params.length;
+ if (args == 0)
+ {
+ return method;
+ }
- for (int j = 0; j < params.length; j++)
+ StrBuilder methodKey = new StrBuilder((args+1)*16).append(method);
+
+ for (int j = 0; j < args; j++)
{
Object arg = params[j];
-
if (arg == null)
{
- arg = OBJECT;
+ methodKey.append(NULL_ARG);
+ }
+ else
+ {
+ methodKey.append(arg.getClass().getName());
}
-
- methodKey.append(arg.getClass().getName());
}
return methodKey.toString();
Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java?rev=679872&r1=679871&r2=679872&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java Fri Jul 25 11:21:31 2008
@@ -21,7 +21,7 @@
import java.lang.reflect.Method;
import java.util.ArrayList;
-import java.util.Hashtable;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -45,7 +45,7 @@
/**
* Keep track of all methods with the same name.
*/
- Map methodByNameMap = new Hashtable();
+ Map methodByNameMap = new HashMap();
/**
* Add a method to a list of methods by name.
@@ -132,99 +132,99 @@
arg == null ? null : arg.getClass();
}
- return getMostSpecific(methodList, classes);
+ return getBestMatch(methodList, classes);
}
- /**
- * Simple distinguishable exception, used when
- * we run across ambiguous overloading. Caught
- * by the introspector.
- */
- public static class AmbiguousException extends RuntimeException
- {
- /**
- * Version Id for serializable
- */
- private static final long serialVersionUID = -2314636505414551663L;
- }
-
-
- private static Method getMostSpecific(List methods, Class[] classes)
- throws AmbiguousException
+ private static Method getBestMatch(List methods, Class[] args)
{
- LinkedList applicables = getApplicables(methods, classes);
-
- if(applicables.isEmpty())
+ List equivalentMatches = null;
+ Method bestMatch = null;
+ Class[] bestMatchTypes = null;
+ for (Iterator i = methods.iterator(); i.hasNext(); )
{
- return null;
- }
-
- if(applicables.size() == 1)
- {
- return (Method)applicables.getFirst();
- }
-
- /*
- * This list will contain the maximally specific methods. Hopefully at
- * the end of the below loop, the list will contain exactly one method,
- * (the most specific method) otherwise we have ambiguity.
- */
-
- LinkedList maximals = new LinkedList();
-
- for (Iterator applicable = applicables.iterator();
- applicable.hasNext();)
- {
- Method app = (Method) applicable.next();
- Class[] appArgs = app.getParameterTypes();
- boolean lessSpecific = false;
-
- for (Iterator maximal = maximals.iterator();
- !lessSpecific && maximal.hasNext();)
+ Method method = (Method)i.next();
+ if (isApplicable(method, args))
{
- Method max = (Method) maximal.next();
-
- switch(moreSpecific(appArgs, max.getParameterTypes()))
+ if (bestMatch == null)
{
- case MORE_SPECIFIC:
- {
- /*
- * This method is more specific than the previously
- * known maximally specific, so remove the old maximum.
- */
-
- maximal.remove();
- break;
- }
-
- case LESS_SPECIFIC:
+ bestMatch = method;
+ bestMatchTypes = method.getParameterTypes();
+ }
+ else
+ {
+ Class[] methodTypes = method.getParameterTypes();
+ switch (compare(methodTypes, bestMatchTypes))
{
- /*
- * This method is less specific than some of the
- * currently known maximally specific methods, so we
- * won't add it into the set of maximally specific
- * methods
- */
-
- lessSpecific = true;
- break;
+ case MORE_SPECIFIC:
+ if (equivalentMatches == null)
+ {
+ bestMatch = method;
+ bestMatchTypes = methodTypes;
+ }
+ else
+ {
+ // have to beat all other ambiguous ones...
+ int ambiguities = equivalentMatches.size();
+ for (int a=0; a < ambiguities; a++)
+ {
+ Method other = (Method)equivalentMatches.get(a);
+ switch (compare(methodTypes, other.getParameterTypes()))
+ {
+ case MORE_SPECIFIC:
+ // ...and thus replace them all...
+ bestMatch = method;
+ bestMatchTypes = methodTypes;
+ equivalentMatches = null;
+ ambiguities = 0;
+ break;
+
+ case INCOMPARABLE:
+ // ...join them...
+ equivalentMatches.add(method);
+ break;
+
+ case LESS_SPECIFIC:
+ // ...or just go away.
+ break;
+ }
+ }
+ }
+ break;
+
+ case INCOMPARABLE:
+ if (equivalentMatches == null)
+ {
+ equivalentMatches = new ArrayList(bestMatchTypes.length);
+ }
+ equivalentMatches.add(method);
+ break;
+
+ case LESS_SPECIFIC:
+ // do nothing
+ break;
}
}
}
-
- if(!lessSpecific)
- {
- maximals.addLast(app);
- }
}
-
- if(maximals.size() > 1)
+
+ if (equivalentMatches != null)
{
- // We have more than one maximally specific method
throw new AmbiguousException();
}
+ return bestMatch;
+ }
- return (Method)maximals.getFirst();
+ /**
+ * Simple distinguishable exception, used when
+ * we run across ambiguous overloading. Caught
+ * by the introspector.
+ */
+ public static class AmbiguousException extends RuntimeException
+ {
+ /**
+ * Version Id for serializable
+ */
+ private static final long serialVersionUID = -2314636505414551663L;
}
/**
@@ -235,7 +235,7 @@
* @return MORE_SPECIFIC if c1 is more specific than c2, LESS_SPECIFIC if
* c1 is less specific than c2, INCOMPARABLE if they are incomparable.
*/
- private static int moreSpecific(Class[] c1, Class[] c2)
+ private static int compare(Class[] c1, Class[] c2)
{
boolean c1MoreSpecific = false;
boolean c2MoreSpecific = false;
@@ -310,31 +310,6 @@
}
/**
- * Returns all methods that are applicable to actual argument types.
- * @param methods list of all candidate methods
- * @param classes the actual types of the arguments
- * @return a list that contains only applicable methods (number of
- * formal and actual arguments matches, and argument types are assignable
- * to formal types through a method invocation conversion).
- */
- private static LinkedList getApplicables(List methods, Class[] classes)
- {
- LinkedList list = new LinkedList();
-
- for (Iterator imethod = methods.iterator(); imethod.hasNext();)
- {
- Method method = (Method) imethod.next();
-
- if(isApplicable(method, classes))
- {
- list.add(method);
- }
-
- }
- return list;
- }
-
- /**
* Returns true if the supplied method is applicable to actual
* argument types.
*
Re: svn commit: r679872 - in /velocity/engine/trunk: ./ src/java/org/apache/velocity/runtime/ src/java/org/apache/velocity/runtime/directive/ src/java/org/apache/velocity/runtime/resource/ src/java/org/apache/velocity/util/introspection/
Posted by Nathan Bubna <nb...@apache.org>.
ok, that's fixed. i left the VelocimacroFactory changes, since i
forgot to commit those as part of r679871.
On Fri, Jul 25, 2008 at 11:24 AM, Nathan Bubna <nb...@apache.org> wrote:
> argh! only meant to commit pom.xml. backing out other changes now...
>
> On Fri, Jul 25, 2008 at 11:21 AM, <nb...@apache.org> wrote:
>> Author: nbubna
>> Date: Fri Jul 25 11:21:31 2008
>> New Revision: 679872
>>
>> URL: http://svn.apache.org/viewvc?rev=679872&view=rev
>> Log:
>> sync pom commons-lang version with build.properties
>>
>> Modified:
>> velocity/engine/trunk/pom.xml
>> velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java
>> velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
>> velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java
>> velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java
>> velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java
>> velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java
>>
>> Modified: velocity/engine/trunk/pom.xml
>> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/pom.xml?rev=679872&r1=679871&r2=679872&view=diff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: svn commit: r679872 - in /velocity/engine/trunk: ./ src/java/org/apache/velocity/runtime/ src/java/org/apache/velocity/runtime/directive/ src/java/org/apache/velocity/runtime/resource/ src/java/org/apache/velocity/util/introspection/
Posted by Nathan Bubna <nb...@apache.org>.
argh! only meant to commit pom.xml. backing out other changes now...
On Fri, Jul 25, 2008 at 11:21 AM, <nb...@apache.org> wrote:
> Author: nbubna
> Date: Fri Jul 25 11:21:31 2008
> New Revision: 679872
>
> URL: http://svn.apache.org/viewvc?rev=679872&view=rev
> Log:
> sync pom commons-lang version with build.properties
>
> Modified:
> velocity/engine/trunk/pom.xml
> velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java
> velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
> velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java
> velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java
> velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java
> velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java
>
> Modified: velocity/engine/trunk/pom.xml
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/pom.xml?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/pom.xml (original)
> +++ velocity/engine/trunk/pom.xml Fri Jul 25 11:21:31 2008
> @@ -138,7 +138,7 @@
> <dependency>
> <groupId>commons-lang</groupId>
> <artifactId>commons-lang</artifactId>
> - <version>2.1</version>
> + <version>2.4</version>
> </dependency>
> <dependency>
> <groupId>oro</groupId>
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java Fri Jul 25 11:21:31 2008
> @@ -94,17 +94,6 @@
> */
> private Map libModMap;
>
> - /*
> - * Map for holding macro execution information. An executing macro will
> - * be identified by the pair <tamplateName, macroName>.
> - */
> - private final Map templateMap;
> -
> - /*
> - * Private variable for holding the allowed max calling depth.
> - */
> - private int maxCallingDepth;
> -
> /**
> * C'tor for the VelociMacro factory.
> *
> @@ -122,96 +111,8 @@
> */
> libModMap = new HashMap();
> vmManager = new VelocimacroManager(rsvc);
> - templateMap = new HashMap();
> }
>
> - /**
> - * This method is called before a macro is rendered. This method
> - * checks whether a macro call is within the allowed calling depth.
> - * If the macro call exceeds the allowed calling depth it will throw
> - * an exception.
> - *
> - * @param macroName name of the macro
> - * @param templateName name of the template file containing the macro
> - * @throws MacroOverflowException if the number of macro calls exceeds the specified value
> - */
> - public void startMacroRendering(String macroName, String templateName)
> - throws MacroOverflowException
> - {
> - maxCallingDepth = rsvc.getInt(
> - RuntimeConstants.VM_MAX_DEPTH);
> -
> - /*
> - * If this property is set to 0 or minus value we do not keep track
> - * of the macro execution
> - */
> - if (maxCallingDepth > 0)
> - {
> - synchronized(templateMap)
> - {
> - Stack macroStack = (Stack)templateMap.get(templateName);
> - if (macroStack != null)
> - {
> - /*
> - * If the macro stack size is larger than or equal to
> - * maxCallingDepth allowed throw an exception
> - */
> - if (macroStack.size() >= maxCallingDepth)
> - {
> - log.error("Max calling depth exceded in Template:" +
> - templateName + "and Macro:" + macroName);
> -
> - String message = "Exceed maximum " + maxCallingDepth +
> - " macro calls. Call Stack:";
> - /*
> - * Construct the message from the stack
> - */
> - for (int i = 0; i < macroStack.size() - 1; i++)
> - {
> - message += macroStack.get(i) + "->";
> - }
> - message += macroStack.peek();
> -
> - /*
> - Clean up the template map
> - */
> - templateMap.remove(templateName);
> - throw new MacroOverflowException(message);
> - }
> - macroStack.push(macroName);
> - }
> - else
> - {
> - macroStack = new Stack();
> - macroStack.push(macroName);
> - templateMap.put(templateName, macroStack);
> - }
> - }
> - }
> - }
> -
> - /**
> - * This method is called when a macro finishes rendering. Clears the state
> - * saved about the macro call.
> - *
> - * @param macroName name of the macro
> - * @param templateName template name containg the macro
> - */
> - public void endMacroRendering(String macroName, String templateName)
> - {
> - String name = null;
> - Stack macroStack = (Stack)templateMap.get(templateName);
> - if (macroStack != null && macroStack.size() > 0)
> - {
> - macroStack.pop();
> - }
> - if (macroStack != null && macroStack.size() == 0)
> - {
> - templateMap.remove(templateName);
> - }
> - }
> -
> -
> /**
> * initialize the factory - setup all permissions
> * load all global libraries.
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java Fri Jul 25 11:21:31 2008
> @@ -19,6 +19,7 @@
> * under the License.
> */
>
> +import org.apache.commons.lang.text.StrBuilder;
> import org.apache.velocity.context.InternalContextAdapter;
> import org.apache.velocity.runtime.parser.node.Node;
> import org.apache.velocity.runtime.parser.Token;
> @@ -124,26 +125,35 @@
> this.context = context;
> this.node = node;
>
> - StringBuffer buffer = new StringBuffer();
> Token t = node.getFirstToken();
>
> - /**
> - * Retrieve the literal text
> - */
> - while (t != null && t != node.getLastToken())
> + if (t == node.getLastToken())
> {
> - buffer.append(t.image);
> - t = t.next;
> + literal = t.image;
> }
> -
> - if (t != null)
> + else
> {
> - buffer.append(t.image);
> + // guessing that most macros are much longer than
> + // the 32 char default capacity. let's guess 4x bigger :)
> + StrBuilder text = new StrBuilder(128);
> + /**
> + * Retrieve the literal text
> + */
> + while (t != null && t != node.getLastToken())
> + {
> + text.append(t.image);
> + t = t.next;
> + }
> + if (t != null)
> + {
> + text.append(t.image);
> + }
> +
> + /**
> + * Store the literal text
> + */
> + literal = text.toString();
> }
> - /**
> - * Store the literal text
> - */
> - literal = buffer.toString();
> }
>
> /**
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java Fri Jul 25 11:21:31 2008
> @@ -85,6 +85,11 @@
> protected Object data = null;
>
> /**
> + * Resource type (RESOURCE_TEMPLATE or RESOURCE_CONTENT)
> + */
> + protected int type;
> +
> + /**
> * Default constructor
> */
> public Resource()
> @@ -267,4 +272,20 @@
> {
> return data;
> }
> +
> + /**
> + * Sets the type of this Resource (RESOURCE_TEMPLATE or RESOURCE_CONTENT)
> + */
> + public void setType(int type)
> + {
> + this.type = type;
> + }
> +
> + /**
> + * @return type code of the Resource
> + */
> + public int getType()
> + {
> + return type;
> + }
> }
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java Fri Jul 25 11:21:31 2008
> @@ -99,11 +99,11 @@
> public synchronized void initialize(final RuntimeServices rsvc)
> throws Exception
> {
> - if (isInit)
> - {
> - log.error("Re-initialization of ResourceLoader attempted!");
> - return;
> - }
> + if (isInit)
> + {
> + log.error("Re-initialization of ResourceLoader attempted!");
> + return;
> + }
>
> ResourceLoader resourceLoader = null;
>
> @@ -259,6 +259,9 @@
> * Gets the named resource. Returned class type corresponds to specified type (i.e. <code>Template</code> to <code>
> * RESOURCE_TEMPLATE</code>).
> *
> + * This method is now unsynchronized which requires that ResourceCache
> + * implementations be thread safe (as the default is).
> + *
> * @param resourceName The name of the resource to retrieve.
> * @param resourceType The type of resource (<code>RESOURCE_TEMPLATE</code>, <code>RESOURCE_CONTENT</code>, etc.).
> * @param encoding The character encoding to use.
> @@ -269,7 +272,7 @@
> * @throws ParseErrorException if template cannot be parsed due to syntax (or other) error.
> * @throws Exception if a problem in parse
> */
> - public synchronized Resource getResource(final String resourceName, final int resourceType, final String encoding)
> + public Resource getResource(final String resourceName, final int resourceType, final String encoding)
> throws ResourceNotFoundException,
> ParseErrorException,
> Exception
> @@ -289,13 +292,29 @@
>
> if (resource != null)
> {
> - /*
> - * refresh the resource
> - */
> -
> try
> {
> - refreshResource(resource, encoding);
> + // avoids additional method call to refreshResource
> + if (resource.requiresChecking())
> + {
> + /*
> + * both loadResource() and refreshResource() now return
> + * a new Resource instance when they are called
> + * (put in the cache when appropriate) in order to allow
> + * several threads to parse the same template simultaneously.
> + * It is redundant work and will cause more garbage collection but the
> + * benefit is that it allows concurrent parsing and processing
> + * without race conditions when multiple requests try to
> + * refresh/load the same template at the same time.
> + *
> + * Another alternative is to limit template parsing/retrieval
> + * so that only one thread can parse each template at a time
> + * but that creates a scalability bottleneck.
> + *
> + * See VELOCITY-606, VELOCITY-595 and VELOCITY-24
> + */
> + resource = refreshResource(resource, encoding);
> + }
> }
> catch (ResourceNotFoundException rnfe)
> {
> @@ -316,7 +335,7 @@
> }
> catch (RuntimeException re)
> {
> - throw re;
> + throw re;
> }
> catch (Exception e)
> {
> @@ -330,8 +349,7 @@
> {
> /*
> * it's not in the cache, so load it.
> - */
> -
> + */
> resource = loadResource(resourceName, resourceType, encoding);
>
> if (resource.getResourceLoader().isCachingOn())
> @@ -352,7 +370,7 @@
> }
> catch (RuntimeException re)
> {
> - throw re;
> + throw re;
> }
> catch (Exception e)
> {
> @@ -383,9 +401,7 @@
> Exception
> {
> Resource resource = ResourceFactory.getResource(resourceName, resourceType);
> -
> resource.setRuntimeServices(rsvc);
> -
> resource.setName(resourceName);
> resource.setEncoding(encoding);
>
> @@ -475,12 +491,11 @@
> * @throws ParseErrorException if template cannot be parsed due to syntax (or other) error.
> * @throws Exception if a problem in parse
> */
> - protected void refreshResource(final Resource resource, final String encoding)
> + protected Resource refreshResource(Resource resource, final String encoding)
> throws ResourceNotFoundException,
> ParseErrorException,
> Exception
> {
> -
> /*
> * The resource knows whether it needs to be checked
> * or not, and the resource's loader can check to
> @@ -489,52 +504,58 @@
> * the input stream and parse it to make a new
> * AST for the resource.
> */
> - if (resource.requiresChecking())
> +
> + /*
> + * touch() the resource to reset the counters
> + */
> + resource.touch();
> +
> + if (resource.isSourceModified())
> {
> /*
> - * touch() the resource to reset the counters
> + * now check encoding info. It's possible that the newly declared
> + * encoding is different than the encoding already in the resource
> + * this strikes me as bad...
> */
>
> - resource.touch();
> -
> - if (resource.isSourceModified())
> + if (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), encoding))
> {
> - /*
> - * now check encoding info. It's possible that the newly declared
> - * encoding is different than the encoding already in the resource
> - * this strikes me as bad...
> - */
> -
> - if (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), encoding))
> - {
> - log.warn("Declared encoding for template '" +
> + log.warn("Declared encoding for template '" +
> resource.getName() +
> "' is different on reload. Old = '" +
> resource.getEncoding() + "' New = '" + encoding);
>
> - resource.setEncoding(encoding);
> - }
> -
> - /*
> - * read how old the resource is _before_
> - * processing (=>reading) it
> - */
> - long howOldItWas = resource.getResourceLoader().getLastModified(resource);
> + resource.setEncoding(encoding);
> + }
>
> - /*
> - * read in the fresh stream and parse
> - */
> + /*
> + * read how old the resource is _before_
> + * processing (=>reading) it
> + */
> + long howOldItWas = resource.getResourceLoader().getLastModified(resource);
>
> - resource.process();
> + String resourceKey = resource.getType() + resource.getName();
>
> - /*
> - * now set the modification info and reset
> - * the modification check counters
> - */
> + /*
> + * we create a copy to avoid partially overwriting a
> + * template which may be in use in another thread
> + */
> +
> + Resource newResource =
> + ResourceFactory.getResource(resource.getName(), resource.getType());
> +
> + newResource.setRuntimeServices(rsvc);
> + newResource.setName(resource.getName());
> + newResource.setEncoding(resource.getEncoding());
> + newResource.setResourceLoader(resource.getResourceLoader());
> +
> + newResource.process();
> + newResource.setLastModified(howOldItWas);
> + resource = newResource;
>
> - resource.setLastModified(howOldItWas);
> - }
> + globalCache.put(resourceKey, newResource);
> }
> + return resource;
> }
>
> /**
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java Fri Jul 25 11:21:31 2008
> @@ -26,7 +26,7 @@
> import java.util.Iterator;
> import java.util.List;
> import java.util.Map;
> -
> +import org.apache.commons.lang.text.StrBuilder;
> import org.apache.velocity.runtime.log.Log;
>
> /**
> @@ -39,6 +39,7 @@
> * @author <a href="mailto:szegedia@freemail.hu">Attila Szegedi</a>
> * @author <a href="mailto:geirm@optonline.net">Geir Magnusson Jr.</a>
> * @author <a href="mailto:henning@apache.org">Henning P. Schmiedehausen</a>
> + * @author Nathan Bubna
> * @version $Id$
> */
> public class ClassMap
> @@ -72,9 +73,7 @@
> log.debug("== Class: " + clazz);
> }
>
> - methodCache = new MethodCache(log);
> -
> - populateMethodCache();
> + methodCache = createMethodCache();
>
> if (debugReflection && log.isDebugEnabled())
> {
> @@ -111,10 +110,11 @@
> * are taken from all the public methods
> * that our class, its parents and their implemented interfaces provide.
> */
> - private void populateMethodCache()
> + private MethodCache createMethodCache()
> {
> + MethodCache methodCache = new MethodCache(log);
> //
> - // Build a list of all elements in the class hierarchy. This one is bottom-first (i.e. we start
> + // Looks through 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.
> @@ -126,70 +126,60 @@
> // 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.
> //
> - List classesToReflect = new ArrayList();
> -
> // Ah, the miracles of Java for(;;) ...
> for (Class classToReflect = getCachedClass(); classToReflect != null ; classToReflect = classToReflect.getSuperclass())
> {
> if (Modifier.isPublic(classToReflect.getModifiers()))
> {
> - classesToReflect.add(classToReflect);
> - if (debugReflection && log.isDebugEnabled())
> - {
> - log.debug("Adding " + classToReflect + " for reflection");
> - }
> + populateMethodCacheWith(methodCache, classToReflect);
> }
> Class [] interfaces = classToReflect.getInterfaces();
> for (int i = 0; i < interfaces.length; i++)
> {
> if (Modifier.isPublic(interfaces[i].getModifiers()))
> {
> - classesToReflect.add(interfaces[i]);
> - if (debugReflection && log.isDebugEnabled())
> - {
> - log.debug("Adding " + interfaces[i] + " for reflection");
> - }
> + populateMethodCacheWith(methodCache, interfaces[i]);
> }
> }
> }
> + // return the already initialized cache
> + return methodCache;
> + }
>
> - for (Iterator it = classesToReflect.iterator(); it.hasNext(); )
> + private void populateMethodCacheWith(MethodCache methodCache, Class classToReflect)
> + {
> + if (debugReflection && log.isDebugEnabled())
> {
> - Class classToReflect = (Class) it.next();
> - if (debugReflection && log.isDebugEnabled())
> - {
> - log.debug("Reflecting " + classToReflect);
> - }
> -
> + log.debug("Reflecting " + classToReflect);
> + }
>
> - try
> - {
> - Method[] methods = classToReflect.getMethods();
> + try
> + {
> + Method[] methods = classToReflect.getDeclaredMethods();
>
> - for (int i = 0; i < methods.length; i++)
> + for (int i = 0; i < methods.length; i++)
> + {
> + // Strictly spoken that check shouldn't be necessary
> + // because getMethods only returns public methods.
> + int modifiers = methods[i].getModifiers();
> + if (Modifier.isPublic(modifiers)) // && !)
> {
> - // Strictly spoken that check shouldn't be necessary
> - // because getMethods only returns public methods.
> - int modifiers = methods[i].getModifiers();
> - if (Modifier.isPublic(modifiers)) // && !)
> - {
> - // Some of the interfaces contain abstract methods. That is fine, because the actual object must
> - // implement them anyway (else it wouldn't be implementing the interface). If we find an abstract
> - // method in a non-interface, we skip it, because we do want to make sure that no abstract methods end up in
> - // the cache.
> - if (classToReflect.isInterface() || !Modifier.isAbstract(modifiers))
> - {
> - methodCache.put(methods[i]);
> - }
> + // Some of the interfaces contain abstract methods. That is fine, because the actual object must
> + // implement them anyway (else it wouldn't be implementing the interface). If we find an abstract
> + // method in a non-interface, we skip it, because we do want to make sure that no abstract methods end up in
> + // the cache.
> + if (classToReflect.isInterface() || !Modifier.isAbstract(modifiers))
> + {
> + methodCache.put(methods[i]);
> }
> }
> }
> - catch (SecurityException se) // Everybody feels better with...
> + }
> + catch (SecurityException se) // Everybody feels better with...
> + {
> + if (log.isDebugEnabled())
> {
> - if (log.isDebugEnabled())
> - {
> - log.debug("While accessing methods of " + classToReflect + ": ", se);
> - }
> + log.debug("While accessing methods of " + classToReflect + ": ", se);
> }
> }
> }
> @@ -202,11 +192,9 @@
> */
> private static final class MethodCache
> {
> - private static final class CacheMiss { }
> -
> - private static final CacheMiss CACHE_MISS = new CacheMiss();
> + private static final Object CACHE_MISS = new Object();
>
> - private static final Object OBJECT = new Object();
> + private static final String NULL_ARG = new Object().getClass().getName();
>
> private static final Map convertPrimitives = new HashMap();
>
> @@ -230,6 +218,7 @@
> * name and actual arguments used to find it.
> */
> private final Map cache = new HashMap();
> + private final Map locks = new HashMap();
>
> /** Map of methods that are searchable according to method parameters to find a match */
> private final MethodMap methodMap = new MethodMap();
> @@ -255,52 +244,55 @@
> * @return A Method object representing the method to invoke or null.
> * @throws MethodMap.AmbiguousException When more than one method is a match for the parameters.
> */
> - public synchronized Method get(final String name, final Object [] params)
> + public Method get(final String name, final Object [] params)
> throws MethodMap.AmbiguousException
> {
> - String methodKey = makeMethodKey(name, params);
> + String methodKey = getLock(makeMethodKey(name, params));
>
> - Object cacheEntry = cache.get(methodKey);
> -
> - // We looked this up before and failed.
> - if (cacheEntry == CACHE_MISS)
> + synchronized (methodKey)
> {
> - return null;
> - }
> + Object cacheEntry = cache.get(methodKey);
>
> - if (cacheEntry == null)
> - {
> - try
> + // We looked this up before and failed.
> + if (cacheEntry == CACHE_MISS)
> {
> - // That one is expensive...
> - cacheEntry = methodMap.find(name, params);
> + return null;
> }
> - catch(MethodMap.AmbiguousException ae)
> +
> + if (cacheEntry == null)
> {
> - /*
> - * that's a miss :-)
> - */
> - cache.put(methodKey, CACHE_MISS);
> - throw ae;
> - }
> + try
> + {
> + // That one is expensive...
> + cacheEntry = methodMap.find(name, params);
> + }
> + catch(MethodMap.AmbiguousException ae)
> + {
> + /*
> + * that's a miss :-)
> + */
> + cache.put(methodKey, CACHE_MISS);
> + throw ae;
> + }
>
> - cache.put(methodKey,
> - (cacheEntry != null) ? cacheEntry : CACHE_MISS);
> - }
> + cache.put(methodKey,
> + (cacheEntry != null) ? cacheEntry : CACHE_MISS);
> + }
>
> - // Yes, this might just be null.
> + // Yes, this might just be null.
>
> - return (Method) cacheEntry;
> + return (Method) cacheEntry;
> + }
> }
>
> - public synchronized void put(Method method)
> + private void put(Method method)
> {
> String methodKey = makeMethodKey(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.
> +
> + // We don't overwrite methods because we fill the
> + // cache from defined class towards java.lang.Object
> + // and that would cause overridden methods to appear
> + // as if they were not overridden.
> if (cache.get(methodKey) == null)
> {
> cache.put(methodKey, method);
> @@ -312,6 +304,18 @@
> }
> }
>
> + private final String getLock(String key) {
> + synchronized (locks)
> + {
> + String lock = (String)locks.get(key);
> + if (lock == null)
> + {
> + return key;
> + }
> + return lock;
> + }
> + }
> +
> /**
> * Make a methodKey for the given method using
> * the concatenation of the name and the
> @@ -323,10 +327,15 @@
> private String makeMethodKey(final Method method)
> {
> Class[] parameterTypes = method.getParameterTypes();
> + int args = parameterTypes.length;
> + if (args == 0)
> + {
> + return method.getName();
> + }
>
> - StringBuffer methodKey = new StringBuffer(method.getName());
> + StrBuilder methodKey = new StrBuilder((args+1)*16).append(method.getName());
>
> - for (int j = 0; j < parameterTypes.length; j++)
> + for (int j = 0; j < args; j++)
> {
> /*
> * If the argument type is primitive then we want
> @@ -353,18 +362,25 @@
>
> private String makeMethodKey(String method, Object[] params)
> {
> - StringBuffer methodKey = new StringBuffer().append(method);
> + int args = params.length;
> + if (args == 0)
> + {
> + return method;
> + }
>
> - for (int j = 0; j < params.length; j++)
> + StrBuilder methodKey = new StrBuilder((args+1)*16).append(method);
> +
> + for (int j = 0; j < args; j++)
> {
> Object arg = params[j];
> -
> if (arg == null)
> {
> - arg = OBJECT;
> + methodKey.append(NULL_ARG);
> + }
> + else
> + {
> + methodKey.append(arg.getClass().getName());
> }
> -
> - methodKey.append(arg.getClass().getName());
> }
>
> return methodKey.toString();
>
> Modified: velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java
> URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java?rev=679872&r1=679871&r2=679872&view=diff
> ==============================================================================
> --- velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java (original)
> +++ velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java Fri Jul 25 11:21:31 2008
> @@ -21,7 +21,7 @@
>
> import java.lang.reflect.Method;
> import java.util.ArrayList;
> -import java.util.Hashtable;
> +import java.util.HashMap;
> import java.util.Iterator;
> import java.util.LinkedList;
> import java.util.List;
> @@ -45,7 +45,7 @@
> /**
> * Keep track of all methods with the same name.
> */
> - Map methodByNameMap = new Hashtable();
> + Map methodByNameMap = new HashMap();
>
> /**
> * Add a method to a list of methods by name.
> @@ -132,99 +132,99 @@
> arg == null ? null : arg.getClass();
> }
>
> - return getMostSpecific(methodList, classes);
> + return getBestMatch(methodList, classes);
> }
>
> - /**
> - * Simple distinguishable exception, used when
> - * we run across ambiguous overloading. Caught
> - * by the introspector.
> - */
> - public static class AmbiguousException extends RuntimeException
> - {
> - /**
> - * Version Id for serializable
> - */
> - private static final long serialVersionUID = -2314636505414551663L;
> - }
> -
> -
> - private static Method getMostSpecific(List methods, Class[] classes)
> - throws AmbiguousException
> + private static Method getBestMatch(List methods, Class[] args)
> {
> - LinkedList applicables = getApplicables(methods, classes);
> -
> - if(applicables.isEmpty())
> + List equivalentMatches = null;
> + Method bestMatch = null;
> + Class[] bestMatchTypes = null;
> + for (Iterator i = methods.iterator(); i.hasNext(); )
> {
> - return null;
> - }
> -
> - if(applicables.size() == 1)
> - {
> - return (Method)applicables.getFirst();
> - }
> -
> - /*
> - * This list will contain the maximally specific methods. Hopefully at
> - * the end of the below loop, the list will contain exactly one method,
> - * (the most specific method) otherwise we have ambiguity.
> - */
> -
> - LinkedList maximals = new LinkedList();
> -
> - for (Iterator applicable = applicables.iterator();
> - applicable.hasNext();)
> - {
> - Method app = (Method) applicable.next();
> - Class[] appArgs = app.getParameterTypes();
> - boolean lessSpecific = false;
> -
> - for (Iterator maximal = maximals.iterator();
> - !lessSpecific && maximal.hasNext();)
> + Method method = (Method)i.next();
> + if (isApplicable(method, args))
> {
> - Method max = (Method) maximal.next();
> -
> - switch(moreSpecific(appArgs, max.getParameterTypes()))
> + if (bestMatch == null)
> {
> - case MORE_SPECIFIC:
> - {
> - /*
> - * This method is more specific than the previously
> - * known maximally specific, so remove the old maximum.
> - */
> -
> - maximal.remove();
> - break;
> - }
> -
> - case LESS_SPECIFIC:
> + bestMatch = method;
> + bestMatchTypes = method.getParameterTypes();
> + }
> + else
> + {
> + Class[] methodTypes = method.getParameterTypes();
> + switch (compare(methodTypes, bestMatchTypes))
> {
> - /*
> - * This method is less specific than some of the
> - * currently known maximally specific methods, so we
> - * won't add it into the set of maximally specific
> - * methods
> - */
> -
> - lessSpecific = true;
> - break;
> + case MORE_SPECIFIC:
> + if (equivalentMatches == null)
> + {
> + bestMatch = method;
> + bestMatchTypes = methodTypes;
> + }
> + else
> + {
> + // have to beat all other ambiguous ones...
> + int ambiguities = equivalentMatches.size();
> + for (int a=0; a < ambiguities; a++)
> + {
> + Method other = (Method)equivalentMatches.get(a);
> + switch (compare(methodTypes, other.getParameterTypes()))
> + {
> + case MORE_SPECIFIC:
> + // ...and thus replace them all...
> + bestMatch = method;
> + bestMatchTypes = methodTypes;
> + equivalentMatches = null;
> + ambiguities = 0;
> + break;
> +
> + case INCOMPARABLE:
> + // ...join them...
> + equivalentMatches.add(method);
> + break;
> +
> + case LESS_SPECIFIC:
> + // ...or just go away.
> + break;
> + }
> + }
> + }
> + break;
> +
> + case INCOMPARABLE:
> + if (equivalentMatches == null)
> + {
> + equivalentMatches = new ArrayList(bestMatchTypes.length);
> + }
> + equivalentMatches.add(method);
> + break;
> +
> + case LESS_SPECIFIC:
> + // do nothing
> + break;
> }
> }
> }
> -
> - if(!lessSpecific)
> - {
> - maximals.addLast(app);
> - }
> }
> -
> - if(maximals.size() > 1)
> +
> + if (equivalentMatches != null)
> {
> - // We have more than one maximally specific method
> throw new AmbiguousException();
> }
> + return bestMatch;
> + }
>
> - return (Method)maximals.getFirst();
> + /**
> + * Simple distinguishable exception, used when
> + * we run across ambiguous overloading. Caught
> + * by the introspector.
> + */
> + public static class AmbiguousException extends RuntimeException
> + {
> + /**
> + * Version Id for serializable
> + */
> + private static final long serialVersionUID = -2314636505414551663L;
> }
>
> /**
> @@ -235,7 +235,7 @@
> * @return MORE_SPECIFIC if c1 is more specific than c2, LESS_SPECIFIC if
> * c1 is less specific than c2, INCOMPARABLE if they are incomparable.
> */
> - private static int moreSpecific(Class[] c1, Class[] c2)
> + private static int compare(Class[] c1, Class[] c2)
> {
> boolean c1MoreSpecific = false;
> boolean c2MoreSpecific = false;
> @@ -310,31 +310,6 @@
> }
>
> /**
> - * Returns all methods that are applicable to actual argument types.
> - * @param methods list of all candidate methods
> - * @param classes the actual types of the arguments
> - * @return a list that contains only applicable methods (number of
> - * formal and actual arguments matches, and argument types are assignable
> - * to formal types through a method invocation conversion).
> - */
> - private static LinkedList getApplicables(List methods, Class[] classes)
> - {
> - LinkedList list = new LinkedList();
> -
> - for (Iterator imethod = methods.iterator(); imethod.hasNext();)
> - {
> - Method method = (Method) imethod.next();
> -
> - if(isApplicable(method, classes))
> - {
> - list.add(method);
> - }
> -
> - }
> - return list;
> - }
> -
> - /**
> * Returns true if the supplied method is applicable to actual
> * argument types.
> *
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org