commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brett Porter <br...@apache.org>
Subject Re: [jelly] a memory leak fix without a tag cache
Date Thu, 27 Jan 2005 12:40:57 GMT
This didn't apply for me:
1) It says Index: at line 124 is invalid
2) if I trim to just JellyContext, one of the hunks fails

How did you generate it? IT is also in dos format, so I assume it was 
winCVS or something?

Can you try maven scm:cvs-create-patch?

Thanks,
Brett

Hans Gilde wrote:

> Here’s a patch for HEAD with a thread-safe cache that’s got no 
> JellyContext tag cache, no memory leak and no dumb WeakReferences. And 
> it even passes the unit tests.
>
> It’s so simple that it was invisible; it’s less than 10 lines of 
> change from the pre-RC1 TagScript. Sigh.
>
> Anyway, I didn’t want to commit it because we already have consensus 
> on the tag cache.
>
> Could everyone test with this one too? If it works, I’ll commit it.
>
> Hans
>
>------------------------------------------------------------------------
>
>Index: src/java/org/apache/commons/jelly/JellyContext.java
>===================================================================
>RCS file: /home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/JellyContext.java,v
>retrieving revision 1.66
>diff -u -r1.66 JellyContext.java
>--- src/java/org/apache/commons/jelly/JellyContext.java	27 Jan 2005 05:52:27 -0000	1.66
>+++ src/java/org/apache/commons/jelly/JellyContext.java	27 Jan 2005 06:11:11 -0000
>@@ -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,93 +376,20 @@
>         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.
>      * This should be called before the parser is used.
>      */
>Index: src/java/org/apache/commons/jelly/impl/TagScript.java
>===================================================================
>RCS file: /home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/TagScript.java,v
>retrieving revision 1.50
>diff -u -r1.50 TagScript.java
>--- src/java/org/apache/commons/jelly/impl/TagScript.java	20 Jan 2005 05:18:56 -0000	1.50
>+++ src/java/org/apache/commons/jelly/impl/TagScript.java	27 Jan 2005 06:11:11 -0000
>@@ -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,8 @@
>     
>     /** the url of the script when parsed */
>     private URL scriptURL = null;
>+    
>+    private Map threadLocalTagCache = Collections.synchronizedMap(new WeakHashMap());
> 
>     /**
>      * @return a new TagScript based on whether
>@@ -286,11 +290,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 +519,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);
>     }
> 
>     /**
>Index: src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java
>===================================================================
>RCS file: /home/cvs/jakarta-commons/jelly/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java,v
>retrieving revision 1.3
>diff -u -r1.3 BaseMemoryLeakTest.java
>--- src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java	20 Jan 2005 05:19:04
-0000	1.3
>+++ src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java	27 Jan 2005 06:11:11
-0000
>@@ -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));
>             }
>
>  
>
>------------------------------------------------------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>


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


Mime
View raw message