You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by br...@apache.org on 2005/05/01 16:05:54 UTC

svn commit: r165507 - in /jakarta/commons/proper/jelly/trunk: src/java/org/apache/commons/jelly/JellyContext.java src/java/org/apache/commons/jelly/impl/TagScript.java src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java xdocs/changes.xml

Author: brett
Date: Sun May  1 07:05:54 2005
New Revision: 165507

URL: http://svn.apache.org/viewcvs?rev=165507&view=rev
Log:
Submitted By:	Hans Glide
Simplify tag caching

Modified:
    jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/JellyContext.java
    jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/impl/TagScript.java
    jakarta/commons/proper/jelly/trunk/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java
    jakarta/commons/proper/jelly/trunk/xdocs/changes.xml

Modified: jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/JellyContext.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/JellyContext.java?rev=165507&r1=165506&r2=165507&view=diff
==============================================================================
--- jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/JellyContext.java (original)
+++ jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/JellyContext.java Sun May  1 07:05:54 2005
@@ -92,20 +92,6 @@
     /** Should we export tag libraries to our parents context */
     private boolean exportLibraries = true;
 
-    /** Maps a Thread to its local Script data cache. It's 
-     * like a ThreadLocal, but it reclaims memory better
-     * when the JellyCointext goes out of scope.
-     * This isn't a ThreadLocal because of the typical usage scenario of
-     * JellyContext. ThreadLocal is meant to be sued as a static variable,
-     * we were using it as a local variable.
-     * {@link #setThreadLocalScriptData(Script,Object)}
-      */
-    private Map threadLocalScriptData = Collections.synchronizedMap(new WeakHashMap());
-    // THINKME: Script objects are like Object (for equals and hashCode) I think.
-    //          It should be asy to optimize hash-map distribution, e.g. by
-    //          shifting the hashcode return value (presuming Object.hashcode()
-    //          is something like an address)
-
     /**
      * Create a new context with the currentURL set to the rootURL
      */
@@ -390,91 +376,18 @@
         return createChildContext();
     }
     
-
-    /** Gets the Script data item that may have previously been stored
-     * by the script, in this context, for the current thread.
-     *  
-     * @return the tag associated with the current context and thread
-      */
-    public Object getThreadScriptData(Script script) {
-        if( script == null )
-            return null;
-        Tag tag = (Tag) getThreadScriptDataMap().get(script);
-		if( tag == null && getParent() != null) {
-			return getParent().getThreadScriptData(script);
-		} else {
-			return tag;
-		}
-    }
-	
-	/** Gets a per-thread (thread local) Map of data for use by
-     * Scripts.
-     * @return the thread local Map of Script data */
-	public Map getThreadScriptDataMap() {
-        Map rv;
-        Thread t = Thread.currentThread();
-        Map data = (Map) threadLocalScriptData.get(t);
-        if (data == null) {
-            rv = new HashMap();
-            threadLocalScriptData.put(t, rv);
-        } else {
-            rv = data;
-        }
-		return rv;
-	}
-    
-    /** Stores an object that lasts for the life of this context
-     * and is local to the current thread. This method is
-     * mainly intended to store Tag instances. However, any
-     * Script that wants to cache data can use this
-     * method.
-      */
-    public void setThreadScriptData(Script script, Object data) {
-        getThreadScriptDataMap().put(script,data);
-    }
-    
-    /** Clears variables set by Tags (basically, variables set in a Jelly script)
-     * and data stored by {@link Script} instances.
+    /** Clears variables set by Tags.
      * @see #clearVariables()
-     * @see #clearThreadScriptData()
-     * @see #clearScriptData()
       */
     public void clear() {
-        clearScriptData();
         clearVariables();
     }
     
     /** Clears variables set by Tags (variables set while running a Jelly script)
      * @see #clear()
-     * @see #clearThreadScriptData()
-     * @see #clearScriptData()
      */
-    public void clearVariables() {
+    protected void clearVariables() {
         variables.clear();
-    }
-    
-    /** Clears data cached by {@link Script} instances, 
-     * for this context, <strong>for the current thread</strong>.
-     * The data cleared could be cached Tag instances or other data
-     * saved by Script classes.
-     * @see #clear()
-     * @see #clearVariables()
-     * @see #clearScriptData()
-     */
-    public void clearThreadScriptData() {
-        getThreadScriptDataMap().clear();
-    }
-    
-    /** Clears data cached by {@link Script} instances, 
-     * for this context, <strong>for all threads</strong>. 
-     * The data cleared could be cached Tag instances or other data
-     * saved by Script classes.
-     * @see #clear()
-     * @see #clearThreadScriptData()
-     * @see #clearVariables()
-     */
-    public void clearScriptData() {
-        threadLocalScriptData.clear();
     }
     
     /** Registers the given tag library against the given namespace URI.

Modified: jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/impl/TagScript.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/impl/TagScript.java?rev=165507&r1=165506&r2=165507&view=diff
==============================================================================
--- jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/impl/TagScript.java (original)
+++ jakarta/commons/proper/jelly/trunk/src/java/org/apache/commons/jelly/impl/TagScript.java Sun May  1 07:05:54 2005
@@ -19,9 +19,11 @@
 import java.lang.reflect.InvocationTargetException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.Collections;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.WeakHashMap;
 
 import org.apache.commons.beanutils.ConvertingWrapDynaBean;
 import org.apache.commons.beanutils.ConvertUtils;
@@ -104,6 +106,10 @@
     
     /** the url of the script when parsed */
     private URL scriptURL = null;
+    
+    /** A synchronized WeakHashMap from the current Thread (key) to a Tag object (value).
+     */
+    private Map threadLocalTagCache = Collections.synchronizedMap(new WeakHashMap());
 
     /**
      * @return a new TagScript based on whether
@@ -286,11 +292,12 @@
      * @return the tag to be evaluated, creating it lazily if required.
      */
     public Tag getTag(JellyContext context) throws JellyException {
-        Tag tag = (Tag) context.getThreadScriptData(this);
+        Thread t = Thread.currentThread();
+        Tag tag = (Tag) threadLocalTagCache.get(t);
         if ( tag == null ) {
             tag = createTag();
             if ( tag != null ) {
-                context.setThreadScriptData(this,tag);
+                threadLocalTagCache.put(t,tag);
                 configureTag(tag,context);
             }
         }
@@ -514,7 +521,8 @@
      * when a StaticTag is switched with a DynamicTag
      */
     protected void setTag(Tag tag, JellyContext context) {
-        context.setThreadScriptData(this,tag);
+        Thread t = Thread.currentThread();
+        threadLocalTagCache.put(t,tag);
     }
 
     /**

Modified: jakarta/commons/proper/jelly/trunk/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/jelly/trunk/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java?rev=165507&r1=165506&r2=165507&view=diff
==============================================================================
--- jakarta/commons/proper/jelly/trunk/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java (original)
+++ jakarta/commons/proper/jelly/trunk/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java Sun May  1 07:05:54 2005
@@ -130,7 +130,6 @@
                 rt.runFinalization();
                 rt.gc();
                 long middle = rt.totalMemory() - rt.freeMemory();
-                log.info("TagHolderMap has " + jc.getThreadScriptDataMap().size() + " entries.");
                 log.info("Memory test after " + i + " runs: "
                         + (middle - start));
             }

Modified: jakarta/commons/proper/jelly/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/jelly/trunk/xdocs/changes.xml?rev=165507&r1=165506&r2=165507&view=diff
==============================================================================
--- jakarta/commons/proper/jelly/trunk/xdocs/changes.xml (original)
+++ jakarta/commons/proper/jelly/trunk/xdocs/changes.xml Sun May  1 07:05:54 2005
@@ -25,6 +25,7 @@
   </properties>
   <body>
     <release version="1.0-RC2" date="in CVS">
+      <action dev="brett" type="fix" due-to="Hans Gilde">Improve tag caching to improve memory consumption</action>
       <action dev="dion" type="fix" issue="JELLY-196">SwitchTag can not be reused if default encountered.</action>
       <action dev="polx" type="add" issue="JELLY-191">FileTag now allows appending to the file.</action>
       <action dev="polx" type="fix" issue="JELLY-85">Removed instance-based ThreadLocal substituting a JellyContext-based tag-caching.</action>



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org