velocity-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nbu...@apache.org
Subject 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/
Date Fri, 25 Jul 2008 18:21:31 GMT
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.
      * 



Mime
View raw message